feat(flags): add Unleash feature flagging integration#3888
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #3888 +/- ##
==========================================
+ Coverage 79.83% 79.87% +0.03%
==========================================
Files 139 140 +1
Lines 15419 15451 +32
Branches 2623 2624 +1
==========================================
+ Hits 12310 12341 +31
- Misses 2235 2237 +2
+ Partials 874 873 -1
|
…eeded. Ref tests back to 1 file
|
Hey @aliu39 ! Thanks for the integration. This will be reviewed after the holiday season. |
|
Todo:
|
cmanallen
left a comment
cmanallen
left a comment
There was a problem hiding this comment.
Mutating class attributes in the __init__ method violates encapsulation. There are a whole category of problems which come from this. I've removed it and replaced with by mutating the UnleashClient class directly. I noticed in the tests you were testing for isolation between instances. While that's interesting it isn't the goal. We want to inject ourselves into every instance or none. This follows the behavior of our other integrations.
Also I've reverted the changes in conftest.py. There was nothing wrong with them. They are just out of scope for this unit of work. Feel free to do those in a follow up PR.
cmanallen
left a comment
cmanallen
left a comment
There was a problem hiding this comment.
Okay we're all good. We're going to keep this setup.
antonpirker
left a comment
antonpirker
left a comment
There was a problem hiding this comment.
Looks good to me.
Nice integrations, and GREAT test suite! Thanks for that!
Follow-up to #3888 The original PR patched 2 methods used for evaluating feature flags, `is_enabled` (simple toggle on/off) and `get_variant` (returns a dict of metadata, see https://docs.getunleash.io/reference/sdks/python#getting-a-variant). We want to remove all `get_variant` code since we only support boolean flag evals atm. It seems like the main usecase for variants is reading payloads (non-bool) for A/B/multivariate testing. This could lead to a lot of extraneous flags, so until it is requested and/or we support non-bool values, let's not patch this method.
3 participants
Adds an integration for tracking flag evaluations from Unleash customers.
Implementation
Unleash has no native support for evaluation hooks/listeners, unless the user opts in for each flag. Therefore we decided to patch the
is_enabledandget_variantmethods on theUnleashClientclass. The methods are wrapped and the only side effect is writing to Sentry scope, so users shouldn't see any change in behavior.We patch these methods on the UnleashClient class template.
We patch oneUnleashClientinstance instead of the whole class. The reasons for this are described in- #3895It's also safer to not modify the unleash import.References
Documentation PR