-
Notifications
You must be signed in to change notification settings - Fork 44
Add Twisted support for LDD mode #55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
de7d3b0
2134d2c
e13c40d
3bad5e8
c5dca8b
a01754c
4b5a72b
03278c6
e6e98a6
9ce49fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| include requirements.txt | ||
| include README.txt | ||
| include test-requirements.txt | ||
| include twisted-requirements.txt | ||
| include redis-requirements.txt |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,24 +10,24 @@ def __init__(self): | |
| self._initialized = False | ||
| self._features = {} | ||
|
|
||
| def get(self, key): | ||
| def get(self, key, callback): | ||
| try: | ||
| self._lock.rlock() | ||
| f = self._features.get(key) | ||
| if f is None: | ||
| log.debug("Attempted to get missing feature: " + str(key) + " Returning None") | ||
| return None | ||
| return callback(None) | ||
| if 'deleted' in f and f['deleted']: | ||
| log.debug("Attempted to get deleted feature: " + str(key) + " Returning None") | ||
| return None | ||
| return f | ||
| return callback(None) | ||
| return callback(f) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm probably missing something, but is this class used by both Twisted and non-Twisted? The interface docs below state
So shouldn't this execute And if I'm understanding this correctly, then I think you can give the callback parameter a default value (of a no-op function) to lessen the impact of the API changes. |
||
| finally: | ||
| self._lock.runlock() | ||
|
|
||
| def all(self): | ||
| def all(self, callback): | ||
| try: | ||
| self._lock.rlock() | ||
| return dict((k, f) for k, f in self._features.items() if ('deleted' not in f) or not f['deleted']) | ||
| return callback(dict((k, f) for k, f in self._features.items() if ('deleted' not in f) or not f['deleted'])) | ||
| finally: | ||
| self._lock.runlock() | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,22 +8,23 @@ class FeatureStore(object): | |
| __metaclass__ = ABCMeta | ||
|
|
||
| @abstractmethod | ||
| def get(self, key): | ||
| def get(self, key, callback): | ||
| """ | ||
| Gets the data for a feature flag for evaluation | ||
|
|
||
| :param key: The feature flag key | ||
| Gets a feature and calls the callback with the feature data to return the result | ||
| :param key: The feature key | ||
| :type key: str | ||
| :return: The feature flag data | ||
| :rtype: dict | ||
| :param callback: The function that accepts the feature data and returns the feature value | ||
| :type callback: Function that processes the feature flag once received. | ||
| :return: The result of executing callback. | ||
| """ | ||
|
|
||
| @abstractmethod | ||
| def all(self): | ||
| def all(self, callback): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the callback parameter should be documented |
||
| """ | ||
| Returns all feature flags and their data | ||
|
|
||
| :rtype: dict[str, dict] | ||
| :param callback: The function that accepts the feature data and returns the feature value | ||
| :type callback: Function that processes the feature flags once received. | ||
| :rtype: The result of executing callback. | ||
| """ | ||
|
|
||
| @abstractmethod | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically this is an interface change that should require us to change the version to 3.0..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work in the non-twisted world?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes- Locally I ran the integration harness against the redis and in-memory feature store