-
Couldn't load subscription status.
- Fork 46
chore: Add persistence store support for FDv2 #357
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
base: mk/sdk-1419/fdv2-datasystem-impl
Are you sure you want to change the base?
Changes from all commits
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,13 +1,18 @@ | ||
| import time | ||
| from copy import copy | ||
| from typing import Callable, Optional | ||
|
|
||
| from ldclient.impl.datasystem.store import Store | ||
| from ldclient.impl.listeners import Listeners | ||
| from ldclient.impl.rwlock import ReadWriteLock | ||
| from ldclient.interfaces import ( | ||
| DataSourceErrorInfo, | ||
| DataSourceState, | ||
| DataSourceStatus, | ||
| DataSourceStatusProvider | ||
| DataSourceStatusProvider, | ||
| DataStoreStatus, | ||
| DataStoreStatusProvider, | ||
| FeatureStore | ||
| ) | ||
|
|
||
|
|
||
|
|
@@ -55,3 +60,50 @@ def add_listener(self, listener: Callable[[DataSourceStatus], None]): | |
|
|
||
| def remove_listener(self, listener: Callable[[DataSourceStatus], None]): | ||
| self.__listeners.remove(listener) | ||
|
|
||
|
|
||
| class DataStoreStatusProviderImpl(DataStoreStatusProvider): | ||
| def __init__(self, store: Optional[FeatureStore], listeners: Listeners): | ||
| self.__store = store | ||
| self.__listeners = listeners | ||
|
|
||
| self.__lock = ReadWriteLock() | ||
| self.__status = DataStoreStatus(True, False) | ||
|
|
||
| def update_status(self, status: DataStoreStatus): | ||
| """ | ||
| update_status is called from the data store to push a status update. | ||
| """ | ||
| self.__lock.lock() | ||
| modified = False | ||
|
|
||
| if self.__status != status: | ||
| self.__status = status | ||
| modified = True | ||
|
|
||
| self.__lock.unlock() | ||
|
|
||
| if modified: | ||
| self.__listeners.notify(status) | ||
|
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. What is the threading situation like for this dispatch? Is the caller thread always the same thread? 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. Yes, unfortunately that's the way the current listeners work in python. The notify method itself catches all exceptions, which is why we aren't doing it here. 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. Ok. Well it is good in a sense, because it helps prevent out of order handling from unfortunate thread context switching. If it was multi-threaded, then I would be concerned we would need some async queue type situation. |
||
|
|
||
| @property | ||
| def status(self) -> DataStoreStatus: | ||
| self.__lock.rlock() | ||
| status = copy(self.__status) | ||
| self.__lock.runlock() | ||
|
|
||
| return status | ||
|
|
||
| def is_monitoring_enabled(self) -> bool: | ||
| if self.__store is None: | ||
| return False | ||
| if hasattr(self.__store, "is_monitoring_enabled") is False: | ||
| return False | ||
|
|
||
| return self.__store.is_monitoring_enabled() # type: ignore | ||
|
|
||
| def add_listener(self, listener: Callable[[DataStoreStatus], None]): | ||
| self.__listeners.add(listener) | ||
|
|
||
| def remove_listener(self, listener: Callable[[DataStoreStatus], None]): | ||
| self.__listeners.remove(listener) | ||
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.
Is there a reason this has to use lock/unlock vs with?
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.
This is the LD provided RW lock. There is no context usage for this class.