Skip to content
This repository has been archived by the owner on Jun 20, 2018. It is now read-only.

Adding an "exclude" condition causes undefined behaviour #105

Open
robgolding opened this issue Dec 7, 2015 · 1 comment
Open

Adding an "exclude" condition causes undefined behaviour #105

robgolding opened this issue Dec 7, 2015 · 1 comment

Comments

@robgolding
Copy link
Contributor

This is a pretty serious bug introduced by #53.


Since this commit, the order of the conditions is now important, and since this library is backed by modeldict, that order can not be guaranteed. Say you have multiple conditions over multiple fields:

If the conditions come out in the order above, then the condition set is always true. If they come out in the opposite order, the condition set is only true if the IP address is 127.0.0.1.

What we're seeing is that the conditions are sometimes respected correctly (resulting in the expected behaviour), but the vast majority of the time the switch is just enabled. It's almost impossible to reproduce by running the code in a shell, because the order of a dictionary changes based on how the memory is laid out, and that's unique for each process.

Furthermore, I don't think the "default" should ever be that a switch is on. If a switch is in selective mode and has a single exclude condition, then that switch should not be enabled for anyone. You should have to selectively opt-in groups users to the switch, then opt-out individuals if necessary.

I don't expect there's any chance of the default behaviour being changed again, so I'll just update our fork to revert this change, but I wanted to raise this in case anyone comes across this crazy bug in the future!

@jlward
Copy link

jlward commented Dec 14, 2015

I just ran into this updating from v0.10.8 (the version right before the bug)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants