From ee0296fb3ade17cae6ae32928b241cbd76d8cd9c Mon Sep 17 00:00:00 2001 From: Federico Bond Date: Thu, 28 Mar 2024 15:11:05 +1100 Subject: [PATCH 1/6] refactor: move registry singleton to the registry module Signed-off-by: Federico Bond --- openfeature/api.py | 14 ++++++-------- openfeature/client.py | 5 +++-- openfeature/provider/registry.py | 3 +++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/openfeature/api.py b/openfeature/api.py index 4460cc70..8f789a8c 100644 --- a/openfeature/api.py +++ b/openfeature/api.py @@ -11,14 +11,12 @@ from openfeature.hook import Hook from openfeature.provider import FeatureProvider from openfeature.provider.metadata import Metadata -from openfeature.provider.registry import ProviderRegistry +from openfeature.provider.registry import default_registry _evaluation_context = EvaluationContext() _hooks: typing.List[Hook] = [] -_provider_registry: ProviderRegistry = ProviderRegistry() - def get_client( domain: typing.Optional[str] = None, version: typing.Optional[str] = None @@ -30,18 +28,18 @@ def set_provider( provider: FeatureProvider, domain: typing.Optional[str] = None ) -> None: if domain is None: - _provider_registry.set_default_provider(provider) + default_registry.set_default_provider(provider) else: - _provider_registry.set_provider(domain, provider) + default_registry.set_provider(domain, provider) def clear_providers() -> None: - _provider_registry.clear_providers() + default_registry.clear_providers() _event_support.clear() def get_provider_metadata(domain: typing.Optional[str] = None) -> Metadata: - return _provider_registry.get_provider(domain).get_metadata() + return default_registry.get_provider(domain).get_metadata() def get_evaluation_context() -> EvaluationContext: @@ -72,7 +70,7 @@ def get_hooks() -> typing.List[Hook]: def shutdown() -> None: - _provider_registry.shutdown() + default_registry.shutdown() def add_handler(event: ProviderEvent, handler: EventHandler) -> None: diff --git a/openfeature/client.py b/openfeature/client.py index 31028828..077ca329 100644 --- a/openfeature/client.py +++ b/openfeature/client.py @@ -28,6 +28,7 @@ error_hooks, ) from openfeature.provider import FeatureProvider, ProviderStatus +from openfeature.provider.registry import default_registry logger = logging.getLogger("openfeature") @@ -82,10 +83,10 @@ def __init__( @property def provider(self) -> FeatureProvider: - return api._provider_registry.get_provider(self.domain) + return default_registry.get_provider(self.domain) def get_provider_status(self) -> ProviderStatus: - return api._provider_registry.get_provider_status(self.provider) + return default_registry.get_provider_status(self.provider) def get_metadata(self) -> ClientMetadata: return ClientMetadata(domain=self.domain) diff --git a/openfeature/provider/registry.py b/openfeature/provider/registry.py index 8d764465..d5c02ba7 100644 --- a/openfeature/provider/registry.py +++ b/openfeature/provider/registry.py @@ -102,3 +102,6 @@ def _set_provider_status( if event := ProviderEvent.from_provider_status(status): run_handlers_for_provider(provider, event, ProviderEventDetails()) + + +default_registry = ProviderRegistry() From caf3c2eb534620c4bd63d44e5500c2721295febf Mon Sep 17 00:00:00 2001 From: Federico Bond Date: Thu, 28 Mar 2024 15:12:44 +1100 Subject: [PATCH 2/6] refactor: make openfeature.provider.registry a private module Signed-off-by: Federico Bond --- openfeature/api.py | 2 +- openfeature/client.py | 2 +- openfeature/provider/{registry.py => _registry.py} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename openfeature/provider/{registry.py => _registry.py} (100%) diff --git a/openfeature/api.py b/openfeature/api.py index 8f789a8c..5d7f9988 100644 --- a/openfeature/api.py +++ b/openfeature/api.py @@ -10,8 +10,8 @@ from openfeature.exception import GeneralError from openfeature.hook import Hook from openfeature.provider import FeatureProvider +from openfeature.provider._registry import default_registry from openfeature.provider.metadata import Metadata -from openfeature.provider.registry import default_registry _evaluation_context = EvaluationContext() diff --git a/openfeature/client.py b/openfeature/client.py index 077ca329..543e67c6 100644 --- a/openfeature/client.py +++ b/openfeature/client.py @@ -28,7 +28,7 @@ error_hooks, ) from openfeature.provider import FeatureProvider, ProviderStatus -from openfeature.provider.registry import default_registry +from openfeature.provider._registry import default_registry logger = logging.getLogger("openfeature") diff --git a/openfeature/provider/registry.py b/openfeature/provider/_registry.py similarity index 100% rename from openfeature/provider/registry.py rename to openfeature/provider/_registry.py From a1f8cae4f20a92677746cc2703685f3a13a5a54a Mon Sep 17 00:00:00 2001 From: Federico Bond Date: Thu, 28 Mar 2024 15:35:14 +1100 Subject: [PATCH 3/6] feat: update provider status when provider emits events Signed-off-by: Federico Bond --- openfeature/provider/_registry.py | 68 +++++++++++++++++++++++++------ openfeature/provider/provider.py | 5 ++- tests/test_api.py | 31 +++++++++++++- 3 files changed, 89 insertions(+), 15 deletions(-) diff --git a/openfeature/provider/_registry.py b/openfeature/provider/_registry.py index d5c02ba7..4b8ac25f 100644 --- a/openfeature/provider/_registry.py +++ b/openfeature/provider/_registry.py @@ -74,34 +74,78 @@ def _initialize_provider(self, provider: FeatureProvider) -> None: try: if hasattr(provider, "initialize"): provider.initialize(self._get_evaluation_context()) - self._set_provider_status(provider, ProviderStatus.READY) + self.dispatch_event( + provider, ProviderEvent.PROVIDER_READY, ProviderEventDetails() + ) except Exception as err: if ( isinstance(err, OpenFeatureError) and err.error_code == ErrorCode.PROVIDER_FATAL ): - self._set_provider_status(provider, ProviderStatus.FATAL) + self.dispatch_event( + provider, + ProviderEvent.PROVIDER_ERROR, + ProviderEventDetails( + message=f"Provider initialization failed: {err}", + error_code=ErrorCode.PROVIDER_FATAL, + ), + ) else: - self._set_provider_status(provider, ProviderStatus.ERROR) + self.dispatch_event( + provider, + ProviderEvent.PROVIDER_ERROR, + ProviderEventDetails( + message=f"Provider initialization failed: {err}", + error_code=ErrorCode.GENERAL, + ), + ) def _shutdown_provider(self, provider: FeatureProvider) -> None: try: if hasattr(provider, "shutdown"): provider.shutdown() - self._set_provider_status(provider, ProviderStatus.NOT_READY) - except Exception: - self._set_provider_status(provider, ProviderStatus.FATAL) + self.dispatch_event( + provider, ProviderEvent.PROVIDER_READY, ProviderEventDetails() + ) + except Exception as err: + self.dispatch_event( + provider, + ProviderEvent.PROVIDER_ERROR, + ProviderEventDetails( + message=f"Provider shutdown failed: {err}", + error_code=ErrorCode.PROVIDER_FATAL, + ), + ) def get_provider_status(self, provider: FeatureProvider) -> ProviderStatus: return self._provider_status.get(provider, ProviderStatus.NOT_READY) - def _set_provider_status( - self, provider: FeatureProvider, status: ProviderStatus + def dispatch_event( + self, + provider: FeatureProvider, + event: ProviderEvent, + details: ProviderEventDetails, ) -> None: - self._provider_status[provider] = status - - if event := ProviderEvent.from_provider_status(status): - run_handlers_for_provider(provider, event, ProviderEventDetails()) + self._update_provider_status(provider, event, details) + run_handlers_for_provider(provider, event, details) + + def _update_provider_status( + self, + provider: FeatureProvider, + event: ProviderEvent, + details: ProviderEventDetails, + ) -> None: + if event == ProviderEvent.PROVIDER_READY: + self._provider_status[provider] = ProviderStatus.READY + elif event == ProviderEvent.PROVIDER_STALE: + self._provider_status[provider] = ProviderStatus.STALE + elif event == ProviderEvent.PROVIDER_ERROR: + status = ( + ProviderStatus.FATAL + if details.error_code == ErrorCode.PROVIDER_FATAL + else ProviderStatus.ERROR + ) + self._provider_status[provider] = status default_registry = ProviderRegistry() diff --git a/openfeature/provider/provider.py b/openfeature/provider/provider.py index 2e5da576..bb233e2c 100644 --- a/openfeature/provider/provider.py +++ b/openfeature/provider/provider.py @@ -1,7 +1,6 @@ import typing from abc import abstractmethod -from openfeature._event_support import run_handlers_for_provider from openfeature.evaluation_context import EvaluationContext from openfeature.event import ProviderEvent, ProviderEventDetails from openfeature.flag_evaluation import FlagResolutionDetails @@ -84,4 +83,6 @@ def emit_provider_stale(self, details: ProviderEventDetails) -> None: self.emit(ProviderEvent.PROVIDER_STALE, details) def emit(self, event: ProviderEvent, details: ProviderEventDetails) -> None: - run_handlers_for_provider(self, event, details) + from openfeature.provider._registry import default_registry + + default_registry.dispatch_event(self, event, details) diff --git a/tests/test_api.py b/tests/test_api.py index 5bb9c91a..bad1fffe 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -20,7 +20,7 @@ from openfeature.event import EventDetails, ProviderEvent, ProviderEventDetails from openfeature.exception import ErrorCode, GeneralError from openfeature.hook import Hook -from openfeature.provider import FeatureProvider, Metadata +from openfeature.provider import FeatureProvider, Metadata, ProviderStatus from openfeature.provider.no_op_provider import NoOpProvider @@ -313,3 +313,32 @@ def test_provider_ready_handlers_run_if_provider_initialize_function_terminates_ # Then spy.provider_ready.assert_called_once() + + +def test_provider_status_is_updated_after_provider_emits_event(): + # Given + provider = NoOpProvider() + set_provider(provider) + client = get_client() + + # When + provider.emit_provider_error(ProviderEventDetails(error_code=ErrorCode.GENERAL)) + # Then + assert client.get_provider_status() == ProviderStatus.ERROR + + # When + provider.emit_provider_error( + ProviderEventDetails(error_code=ErrorCode.PROVIDER_FATAL) + ) + # Then + assert client.get_provider_status() == ProviderStatus.FATAL + + # When + provider.emit_provider_stale(ProviderEventDetails()) + # Then + assert client.get_provider_status() == ProviderStatus.STALE + + # When + provider.emit_provider_ready(ProviderEventDetails()) + # Then + assert client.get_provider_status() == ProviderStatus.READY From b8e2fcca107b9c37e78ae44ebd7e9fa146c612c4 Mon Sep 17 00:00:00 2001 From: Federico Bond Date: Fri, 5 Apr 2024 17:52:24 +1100 Subject: [PATCH 4/6] refactor: avoid duplicate code Signed-off-by: Federico Bond --- openfeature/provider/_registry.py | 34 ++++++++++++------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/openfeature/provider/_registry.py b/openfeature/provider/_registry.py index 4b8ac25f..74375935 100644 --- a/openfeature/provider/_registry.py +++ b/openfeature/provider/_registry.py @@ -78,27 +78,19 @@ def _initialize_provider(self, provider: FeatureProvider) -> None: provider, ProviderEvent.PROVIDER_READY, ProviderEventDetails() ) except Exception as err: - if ( - isinstance(err, OpenFeatureError) - and err.error_code == ErrorCode.PROVIDER_FATAL - ): - self.dispatch_event( - provider, - ProviderEvent.PROVIDER_ERROR, - ProviderEventDetails( - message=f"Provider initialization failed: {err}", - error_code=ErrorCode.PROVIDER_FATAL, - ), - ) - else: - self.dispatch_event( - provider, - ProviderEvent.PROVIDER_ERROR, - ProviderEventDetails( - message=f"Provider initialization failed: {err}", - error_code=ErrorCode.GENERAL, - ), - ) + error_code = ( + err.error_code + if isinstance(err, OpenFeatureError) + else ErrorCode.GENERAL + ) + self.dispatch_event( + provider, + ProviderEvent.PROVIDER_ERROR, + ProviderEventDetails( + message=f"Provider initialization failed: {err}", + error_code=error_code, + ), + ) def _shutdown_provider(self, provider: FeatureProvider) -> None: try: From 2dc911fcd147016115e924c209c024527ee0b52c Mon Sep 17 00:00:00 2001 From: Federico Bond Date: Fri, 5 Apr 2024 18:55:34 +1100 Subject: [PATCH 5/6] fix: fix provider event dispatch on initialize/shutdown Signed-off-by: Federico Bond --- openfeature/provider/_registry.py | 4 +--- tests/test_api.py | 21 ++++++++++++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/openfeature/provider/_registry.py b/openfeature/provider/_registry.py index 74375935..60d3168f 100644 --- a/openfeature/provider/_registry.py +++ b/openfeature/provider/_registry.py @@ -96,9 +96,7 @@ def _shutdown_provider(self, provider: FeatureProvider) -> None: try: if hasattr(provider, "shutdown"): provider.shutdown() - self.dispatch_event( - provider, ProviderEvent.PROVIDER_READY, ProviderEventDetails() - ) + self._provider_status[provider] = ProviderStatus.NOT_READY except Exception as err: self.dispatch_event( provider, diff --git a/tests/test_api.py b/tests/test_api.py index bad1fffe..aaea26b8 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -18,7 +18,7 @@ ) from openfeature.evaluation_context import EvaluationContext from openfeature.event import EventDetails, ProviderEvent, ProviderEventDetails -from openfeature.exception import ErrorCode, GeneralError +from openfeature.exception import ErrorCode, GeneralError, ProviderFatalError from openfeature.hook import Hook from openfeature.provider import FeatureProvider, Metadata, ProviderStatus from openfeature.provider.no_op_provider import NoOpProvider @@ -303,18 +303,33 @@ def test_handlers_attached_to_provider_already_in_associated_state_should_run_im def test_provider_ready_handlers_run_if_provider_initialize_function_terminates_normally(): # Given provider = NoOpProvider() - set_provider(provider) spy = MagicMock() add_handler(ProviderEvent.PROVIDER_READY, spy.provider_ready) + spy.reset_mock() # reset the mock to avoid counting the immediate call on subscribe # When - provider.initialize(get_evaluation_context()) + set_provider(provider) # Then spy.provider_ready.assert_called_once() +def test_provider_error_handlers_run_if_provider_initialize_function_terminates_abnormally(): + # Given + provider = MagicMock(spec=FeatureProvider) + provider.initialize.side_effect = ProviderFatalError() + + spy = MagicMock() + add_handler(ProviderEvent.PROVIDER_ERROR, spy.provider_error) + + # When + set_provider(provider) + + # Then + spy.provider_error.assert_called_once() + + def test_provider_status_is_updated_after_provider_emits_event(): # Given provider = NoOpProvider() From 9aa8a75dcc9c92d983aec9219ab17b24663c4140 Mon Sep 17 00:00:00 2001 From: Federico Bond Date: Sun, 7 Apr 2024 22:52:35 +1000 Subject: [PATCH 6/6] refactor: rename default_registry to provider_registry Signed-off-by: Federico Bond --- openfeature/api.py | 12 ++++++------ openfeature/client.py | 6 +++--- openfeature/provider/_registry.py | 2 +- openfeature/provider/provider.py | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/openfeature/api.py b/openfeature/api.py index 5d7f9988..cbff4b62 100644 --- a/openfeature/api.py +++ b/openfeature/api.py @@ -10,7 +10,7 @@ from openfeature.exception import GeneralError from openfeature.hook import Hook from openfeature.provider import FeatureProvider -from openfeature.provider._registry import default_registry +from openfeature.provider._registry import provider_registry from openfeature.provider.metadata import Metadata _evaluation_context = EvaluationContext() @@ -28,18 +28,18 @@ def set_provider( provider: FeatureProvider, domain: typing.Optional[str] = None ) -> None: if domain is None: - default_registry.set_default_provider(provider) + provider_registry.set_default_provider(provider) else: - default_registry.set_provider(domain, provider) + provider_registry.set_provider(domain, provider) def clear_providers() -> None: - default_registry.clear_providers() + provider_registry.clear_providers() _event_support.clear() def get_provider_metadata(domain: typing.Optional[str] = None) -> Metadata: - return default_registry.get_provider(domain).get_metadata() + return provider_registry.get_provider(domain).get_metadata() def get_evaluation_context() -> EvaluationContext: @@ -70,7 +70,7 @@ def get_hooks() -> typing.List[Hook]: def shutdown() -> None: - default_registry.shutdown() + provider_registry.shutdown() def add_handler(event: ProviderEvent, handler: EventHandler) -> None: diff --git a/openfeature/client.py b/openfeature/client.py index 543e67c6..c2203ca1 100644 --- a/openfeature/client.py +++ b/openfeature/client.py @@ -28,7 +28,7 @@ error_hooks, ) from openfeature.provider import FeatureProvider, ProviderStatus -from openfeature.provider._registry import default_registry +from openfeature.provider._registry import provider_registry logger = logging.getLogger("openfeature") @@ -83,10 +83,10 @@ def __init__( @property def provider(self) -> FeatureProvider: - return default_registry.get_provider(self.domain) + return provider_registry.get_provider(self.domain) def get_provider_status(self) -> ProviderStatus: - return default_registry.get_provider_status(self.provider) + return provider_registry.get_provider_status(self.provider) def get_metadata(self) -> ClientMetadata: return ClientMetadata(domain=self.domain) diff --git a/openfeature/provider/_registry.py b/openfeature/provider/_registry.py index 60d3168f..902a204e 100644 --- a/openfeature/provider/_registry.py +++ b/openfeature/provider/_registry.py @@ -138,4 +138,4 @@ def _update_provider_status( self._provider_status[provider] = status -default_registry = ProviderRegistry() +provider_registry = ProviderRegistry() diff --git a/openfeature/provider/provider.py b/openfeature/provider/provider.py index bb233e2c..debc1d56 100644 --- a/openfeature/provider/provider.py +++ b/openfeature/provider/provider.py @@ -83,6 +83,6 @@ def emit_provider_stale(self, details: ProviderEventDetails) -> None: self.emit(ProviderEvent.PROVIDER_STALE, details) def emit(self, event: ProviderEvent, details: ProviderEventDetails) -> None: - from openfeature.provider._registry import default_registry + from openfeature.provider._registry import provider_registry - default_registry.dispatch_event(self, event, details) + provider_registry.dispatch_event(self, event, details)