feat: update provider status when provider emits events (#309)

* refactor: move registry singleton to the registry module Signed-off-by: Federico Bond <federicobond@gmail.com> * refactor: make openfeature.provider.registry a private module Signed-off-by: Federico Bond <federicobond@gmail.com> * feat: update provider status when provider emits events Signed-off-by: Federico Bond <federicobond@gmail.com> * refactor: avoid duplicate code Signed-off-by: Federico Bond <federicobond@gmail.com> * fix: fix provider event dispatch on initialize/shutdown Signed-off-by: Federico Bond <federicobond@gmail.com> * refactor: rename default_registry to provider_registry Signed-off-by: Federico Bond <federicobond@gmail.com> --------- Signed-off-by: Federico Bond <federicobond@gmail.com>
open-feature · Apr 7, 2024 · 9966c14 · 9966c14
1 parent faf02a9
commit 9966c14
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 33 deletions.
diff --git a/openfeature/api.py b/openfeature/api.py
@@ -10,15 +10,13 @@
 from openfeature.exception import GeneralError
 from openfeature.hook import Hook
 from openfeature.provider import FeatureProvider
+from openfeature.provider._registry import provider_registry
 from openfeature.provider.metadata import Metadata
-from openfeature.provider.registry import ProviderRegistry
 
 _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)
+        provider_registry.set_default_provider(provider)
     else:
-        _provider_registry.set_provider(domain, provider)
+        provider_registry.set_provider(domain, provider)
 
 
 def clear_providers() -> None:
-    _provider_registry.clear_providers()
+    provider_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 provider_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()
+    provider_registry.shutdown()
 
 
 def add_handler(event: ProviderEvent, handler: EventHandler) -> None:

diff --git 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 provider_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 provider_registry.get_provider(self.domain)
 
     def get_provider_status(self) -> ProviderStatus:
-        return api._provider_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 → openfeature/provider/_registry.py b/openfeature/provider/registry.py → openfeature/provider/_registry.py
@@ -74,31 +74,68 @@ 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)
-            else:
-                self._set_provider_status(provider, ProviderStatus.ERROR)
+            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:
             if hasattr(provider, "shutdown"):
                 provider.shutdown()
-            self._set_provider_status(provider, ProviderStatus.NOT_READY)
-        except Exception:
-            self._set_provider_status(provider, ProviderStatus.FATAL)
+            self._provider_status[provider] = ProviderStatus.NOT_READY
+        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
+
+
+provider_registry = ProviderRegistry()
diff --git 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 provider_registry
+
+        provider_registry.dispatch_event(self, event, details)
diff --git a/tests/test_api.py b/tests/test_api.py
@@ -18,9 +18,9 @@
 )
 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
+from openfeature.provider import FeatureProvider, Metadata, ProviderStatus
 from openfeature.provider.no_op_provider import NoOpProvider
 
 
@@ -303,13 +303,57 @@ 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()
+    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