Add new constraint operators #192
Conversation
UnleashClient/__init__.py
Outdated
| context.update(self.unleash_static_context) | ||
|
|
||
| # Update context with optional values | ||
| if 'currentTime' not in context.keys(): | ||
| context.update({'currentTime': datetime.now()}) |
There was a problem hiding this comment.
I might need to steal this to the Node.SDK.
The only reason I kept this in the operator itself is that it does give an additional cost for every isEnabled call to create currentTime instance, even if it is mostly not needed (which will be 99% of the cases). On the other hand, datetime.now() probably has an insignificant cost?
There was a problem hiding this comment.
tldr after a few rabbit holes: Yes for modern CPUs.
I was mainly putting it there since it seemed that it might be handy for other uses (on reflection though, not 100% sure what those could be). I'd be OK with moving this into the Constraint check.
Do you think you'd use currentTime in other features?
There was a problem hiding this comment.
not sure yet. I do not see a big issue moving it out to the context creation later if we find use for it. I would prefer us to have it in the constraint for now.
There was a problem hiding this comment.
I've moved this down into the Constraint.apply() function: https://github.com/Unleash/unleash-client-python/pull/192/files#diff-5a34048a72d9dbcc1cf64fe80a13119b0b40ee924c5cfc90a0352c30e7036ac4R162
| self.context_name: str = constraint_dict['contextName'] | ||
| self.operator: ConstraintOperators = ConstraintOperators(constraint_dict['operator'].upper()) | ||
| self.values = constraint_dict['values'] if 'values' in constraint_dict.keys() else [] | ||
| self.value = constraint_dict['value'] if 'value' in constraint_dict.keys() else [] |
There was a problem hiding this comment.
else [] seems a bit weird for a string?
There was a problem hiding this comment.
Whoops, that's an embarassing C/P error. Def. should be None!
Description
Supports new operators outlined in #191 .
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: