fix!: Use None rather than zero for indefinite initialization waiting

README.md

@@ -39,7 +39,7 @@ This matrix mirrors the [feature matrix of the OpenFeature SDK for Python](https
 | ✅      | Domains                         | Domains bind clients to providers in the OpenFeature SDK; a separate provider instance may be registered per domain.                                                                                                       |
 | ✅      | Eventing                        | LaunchDarkly data source status changes are emitted as `PROVIDER_READY`, `PROVIDER_STALE` and `PROVIDER_ERROR`, and only when the status changes; flag changes as `PROVIDER_CONFIGURATION_CHANGED` with the changed flag key.                              |
 | ✅      | Tracking                        | `track` sends a LaunchDarkly custom event for the evaluation context, with the tracking event value and remaining details attached.                                                                                        |
-| ⚠️      | Initialization                  | `initialize` reports whether the LaunchDarkly client became ready. The optional `start_wait` parameter bounds initialization; zero applies no timeout and waits until the data source becomes valid or permanently fails. Since LaunchDarkly Python SDK 9.18.0 a rejected SDK key is retried rather than failing permanently, so zero can wait indefinitely. Provider gap: zero should not wait, [#64](https://github.com/launchdarkly/openfeature-python-server/pull/64). |
+| ✅      | Initialization                  | `initialize` reports whether the LaunchDarkly client became ready. The optional `start_wait` parameter bounds initialization; `None` waits until the data source becomes valid or permanently fails. Since LaunchDarkly Python SDK 9.18.0 a rejected SDK key is retried rather than failing permanently, so `None` can wait indefinitely.                        |
 | ✅      | Shutdown                        | `shutdown` closes the LaunchDarkly client; a closed client cannot be restarted, so a new provider instance is required afterward.                                                                                          |
 | ✅      | Transaction Context Propagation | Provided by the OpenFeature SDK, which merges the transaction context into the evaluation context before the provider is called; no provider support is required.                                                          |
 | ✅      | Extending                       | The underlying LaunchDarkly client is available through the `client` property.                                                                                                                                            |
@@ -70,7 +70,7 @@ api.set_provider(openfeature_provider)
 # Refer to OpenFeature documentation for getting a client and performing evaluations.
 ```
 
-The optional `start_wait` parameter is the number of seconds to wait for a successful connection to LaunchDarkly, matching the same parameter of the LaunchDarkly SDK's `LDClient`, and defaulting to the same five seconds. A positive value bounds the whole of initialization: the provider constructor blocks for up to that long, and OpenFeature initialization then completes immediately, reporting a failed initialization if the client did not become ready in time. Zero does not block the constructor at all, and initialization then waits without a deadline for the data source to become valid or to fail permanently. Since LaunchDarkly Python SDK 9.18.0 a rejected SDK key is retried rather than treated as a permanent failure, so with zero, initialization does not complete while the key is rejected.
+The optional `start_wait` parameter is the number of seconds to wait for a successful connection to LaunchDarkly, matching the same parameter of the LaunchDarkly SDK's `LDClient`, and defaulting to the same five seconds. A positive value bounds the whole of initialization: the provider constructor blocks for up to that long, and OpenFeature initialization then completes immediately, reporting a failed initialization if the client did not become ready in time. Zero waits nowhere, so initialization fails unless the client is already ready, and the application learns when the client becomes usable from provider events. `None` requests waiting indefinitely: the constructor does not block, and initialization waits without a deadline for the data source to become valid or to fail permanently. Since LaunchDarkly Python SDK 9.18.0 a rejected SDK key is retried rather than treated as a permanent failure, so with `None`, initialization does not complete while the key is rejected.
 
 `api.set_provider` registers the provider without waiting for it to initialize, so the first evaluations may use fallback values while the LaunchDarkly client is still connecting. Use `api.set_provider_and_wait` instead when the application needs registration to block until initialization has succeeded or failed.
 

ld_openfeature/provider.py

@@ -10,7 +10,7 @@
 from openfeature.flag_evaluation import FlagResolutionDetails, FlagType, FlagValueType, Reason
 from openfeature.hook import Hook
 from openfeature.provider.metadata import Metadata
-from openfeature.provider import AbstractProvider
+from openfeature.provider import AbstractProvider, ProviderStatus
 from openfeature.event import ProviderEventDetails
 from openfeature.track import TrackingEventDetails
 
@@ -26,26 +26,28 @@
 
 
 class LaunchDarklyProvider(AbstractProvider):
-    def __init__(self, config: Config, start_wait: float = 5):
+    def __init__(self, config: Config, start_wait: Optional[float] = 5):
         """
         Create a provider backed by a LaunchDarkly client.
 
         :param config: The LaunchDarkly client configuration.
         :param start_wait: The number of seconds to wait for a successful connection to LaunchDarkly, matching
             the same parameter of :class:`ldclient.LDClient`. A positive value bounds the whole of initialization:
             this constructor blocks for up to that long, and ``initialize`` then completes immediately, reporting
-            a failed initialization if the client did not become ready in time. Zero does not block this
-            constructor at all, and ``initialize`` then waits without a deadline for the data source to become
-            valid or to fail permanently.
+            a failed initialization if the client did not become ready in time. Zero waits nowhere, so
+            ``initialize`` reports a failed initialization unless the client is already ready. ``None`` waits
+            only in ``initialize``, without a deadline, until the data source becomes valid or fails permanently.
         """
-        self.__client = LDClient(config.with_wrapper_information(WRAPPER_NAME, VERSION), start_wait)
+        self.__client = LDClient(config.with_wrapper_information(WRAPPER_NAME, VERSION),
+                                 0 if start_wait is None else start_wait)
         self.__start_wait = start_wait
+        self.__initialization_complete = False
+        self.__status_lock = threading.Lock()
+        self.__provider_status = ProviderStatus.NOT_READY
 
         self.__context_converter = EvaluationContextConverter()
         self.__details_converter = ResolutionDetailsConverter()
 
-        self.__status_lock = threading.Lock()
-        self.__last_emitted_state: Optional[DataSourceState] = None
 
     @property
     def client(self) -> LDClient:
@@ -56,39 +58,40 @@ def client(self) -> LDClient:
         """
         return self.__client
 
-    def __is_new_status(self, state: DataSourceState) -> bool:
+    def __set_status(self, status: ProviderStatus) -> bool:
         """
-        Report whether a state changes the provider status. Several data source states can map to the same provider
-        status, and a repeated status is not a change an application can act on.
+        Record the provider status and report whether it changed, and whether the change should be emitted.
+
+        The OpenFeature client emits an event for the status change which completes initialization, so that
+        change is not emitted here. Changes after initialization has completed, including after it has failed,
+        are emitted.
         """
         with self.__status_lock:
-            if state == self.__last_emitted_state:
+            if status == self.__provider_status:
                 return False
-
-            self.__last_emitted_state = state
-            return True
+            self.__provider_status = status
+            return self.__initialization_complete
 
     def __handle_data_source_status(self, status: DataSourceStatus):
         state = status.state
         if state == DataSourceState.INITIALIZING:
             return
-
-        if not self.__is_new_status(state):
-            return
-
-        if state == DataSourceState.VALID:
-            self.emit_provider_ready(ProviderEventDetails())
+        elif state == DataSourceState.VALID:
+            if self.__set_status(ProviderStatus.READY):
+                self.emit_provider_ready(ProviderEventDetails())
         elif state == DataSourceState.OFF:
             error_message = self.__get_message(status,
                                                "the provider has encountered a permanent error or has been shutdown")
             # This is not reported as a fatal error. A fatal provider prevents the OpenFeature client
             # from evaluating flags at all, but the LaunchDarkly client can keep evaluating the flag
             # data it already has.
-            self.emit_provider_error(ProviderEventDetails(error_code=ErrorCode.GENERAL,
-                                                          message=error_message))
+            if self.__set_status(ProviderStatus.ERROR):
+                self.emit_provider_error(ProviderEventDetails(error_code=ErrorCode.GENERAL,
+                                                              message=error_message))
         elif state == DataSourceState.INTERRUPTED:
             error_message = self.__get_message(status, "encountered an unknown error")
-            self.emit_provider_stale(ProviderEventDetails(message=error_message))
+            if self.__set_status(ProviderStatus.STALE):
+                self.emit_provider_stale(ProviderEventDetails(message=error_message))
 
         # For now treat an unknown state as no change.
 
@@ -105,7 +108,11 @@ def ready_handler(status: DataSourceStatus):
             elif status.state == DataSourceState.OFF:
                 ready_event.set()
 
-        # We listen just to handle the ready event. We do not emit events because the client emits them for us.
+        # We listen for status changes before checking the current state, so that a change in between is not
+        # missed. Emission is suppressed until initialization has completed, so the listeners stay registered
+        # even when initialization fails and the client later recovers.
+        self.__client.data_source_status_provider.add_listener(self.__handle_data_source_status)
+        self.__client.flag_tracker.add_listener(self.__handle_flag_change)
         self.__client.data_source_status_provider.add_listener(ready_handler)
 
         # Check for conditions that may have happened before we added the listener.
@@ -116,17 +123,22 @@ def ready_handler(status: DataSourceStatus):
             ready_event.set()
 
         # With a start wait the client constructor has already waited, so the outcome is whatever it is now.
-        if self.__start_wait <= 0:
+        if self.__start_wait is None:
             ready_event.wait()
 
         self.__client.data_source_status_provider.remove_listener(ready_handler)
 
-        if not self.__client.is_initialized():
-            raise ProviderNotReadyError(error_message="launchdarkly client initialization failed")
+        # The OpenFeature client reports the outcome of initialization itself, so the provider records the
+        # resulting status without emitting an event for it. The outcome is read and recorded, and initialization
+        # is completed, under the lock, so a concurrent status change either precedes the outcome it is included
+        # in or is emitted.
+        with self.__status_lock:
+            initialized = self.__client.is_initialized()
+            self.__provider_status = ProviderStatus.READY if initialized else ProviderStatus.ERROR
+            self.__initialization_complete = True
 
-        # Listen to new status events and emit them.
-        self.__client.data_source_status_provider.add_listener(self.__handle_data_source_status)
-        self.__client.flag_tracker.add_listener(self.__handle_flag_change)
+        if not initialized:
+            raise ProviderNotReadyError(error_message="launchdarkly client initialization failed")
 
     def shutdown(self):
         self.__client.data_source_status_provider.remove_listener(self.__handle_data_source_status)

tests/test_data_sources.py

@@ -91,6 +91,34 @@ def initialized(self):
         return False
 
 
+class DelayedReadyDataSource(UpdateProcessor):
+    def __init__(self, config: Config, store, ready: threading.Event):
+        self._data_source_update_sink: Optional[DataSourceUpdateSink] = config.data_source_update_sink
+        self._ready = ready
+        self._initialized = False
+
+    def start(self):
+        if self._data_source_update_sink is None:
+            return
+
+        def data_source_ready():
+            self._data_source_update_sink.init({FEATURES: {}})
+            self._initialized = True
+            self._data_source_update_sink.update_status(DataSourceState.VALID, None)
+            self._ready.set()
+
+        threading.Timer(0.1, data_source_ready).start()
+
+    def stop(self):
+        pass
+
+    def is_alive(self):
+        return True
+
+    def initialized(self):
+        return self._initialized
+
+
 class InitializedThenFailingDataSource(UpdateProcessor):
     def __init__(self, config: Config, store, ready: threading.Event):
         self._data_source_update_sink: Optional[DataSourceUpdateSink] = config.data_source_update_sink

tests/test_provider.py

@@ -16,7 +16,7 @@
 from openfeature import api
 
 from ld_openfeature import LaunchDarklyProvider, Config
-from tests.test_data_sources import FailingDataSource, InitializedThenFailingDataSource, NeverReadyDataSource, RepeatedlyInterruptedDataSource, StaleDataSource, UpdatingDataSource, DelayedFailingDataSource
+from tests.test_data_sources import FailingDataSource, InitializedThenFailingDataSource, NeverReadyDataSource, RepeatedlyInterruptedDataSource, StaleDataSource, UpdatingDataSource, DelayedFailingDataSource, DelayedReadyDataSource
 from ld_openfeature.version import VERSION
 
 
@@ -73,6 +73,63 @@ def test_initialization_fails_without_waiting_again_with_positive_start_wait():
     provider.shutdown()
 
 
+def test_zero_start_wait_does_not_wait_for_the_data_source():
+    started = time.time()
+    provider = LaunchDarklyProvider(
+        Config("", update_processor_class=NeverReadyDataSource, send_events=False),
+        start_wait=0,
+    )
+
+    with pytest.raises(ProviderNotReadyError):
+        provider.initialize(EvaluationContext("user-key"))
+
+    assert time.time() - started < 0.25
+    provider.shutdown()
+
+
+def test_no_start_wait_waits_for_the_data_source_during_initialization():
+    started = time.time()
+    provider = LaunchDarklyProvider(
+        Config("", update_processor_class=DelayedReadyDataSource, send_events=False),
+        start_wait=None,
+    )
+    construction_finished = time.time()
+
+    provider.initialize(EvaluationContext("user-key"))
+
+    assert construction_finished - started < 0.1
+    assert time.time() - started >= 0.1
+    provider.shutdown()
+
+
+def test_ready_is_reported_after_initialization_failed():
+    thread_event = threading.Event()
+    emission_count = 0
+
+    def handle_status(details: EventDetails):
+        nonlocal emission_count
+        if details.provider_name == 'launchdarkly-openfeature-server':
+            emission_count += 1
+            thread_event.set()
+
+    api.add_handler(ProviderEvent.PROVIDER_READY, handle_status)
+
+    provider = LaunchDarklyProvider(
+        Config("", update_processor_class=DelayedReadyDataSource, send_events=False),
+        start_wait=0,
+    )
+
+    # Initialization fails because the data source is not ready yet, and the provider reports the later
+    # transition to a valid data source.
+    api.set_provider(provider)
+
+    assert thread_event.wait(timeout=5)
+    time.sleep(0.1)
+    assert emission_count == 1
+
+    api.shutdown()
+
+
 def test_provider_identifies_itself_as_the_wrapper(provider: LaunchDarklyProvider, config: Config):
     assert provider.client._config.wrapper_name == "open-feature-python-server"
     assert provider.client._config.wrapper_version == VERSION