feat!: make set_provider non-blocking, add set_provider_and_wait#595
feat!: make set_provider non-blocking, add set_provider_and_wait#595jonathannorris wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #595 +/- ##
==========================================
+ Coverage 98.35% 98.41% +0.06%
==========================================
Files 45 45
Lines 2183 2276 +93
==========================================
+ Hits 2147 2240 +93
Misses 36 36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces asynchronous provider initialization to the OpenFeature Python SDK. The set_provider method is now non-blocking by default, delegating initialization to a background thread, while a new set_provider_and_wait method has been added for cases requiring blocking behavior. The documentation and existing tests have been updated to reflect these changes. Feedback from the review highlights critical thread-safety concerns, specifically a race condition in the provider registry that could lead to redundant initializations and the need for synchronization when updating shared state from background threads.
There was a problem hiding this comment.
Pull request overview
This PR updates the provider registration API to make set_provider() non-blocking by default (initialization runs asynchronously), and introduces set_provider_and_wait() for callers/tests that need to block until provider initialization completes or fails. This aligns the Python SDK behavior with the OpenFeature spec guidance and other SDKs.
Changes:
- Make provider initialization asynchronous by default via a background daemon thread.
- Add
set_provider_and_wait()(blocking) and update tests/BDD steps to use it where readiness is required. - Update README examples to document the new semantics and correct a few API usage examples.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
openfeature/provider/_registry.py |
Adds async initialization path (wait_for_init flag) and refactors initialization into _run_initialize. |
openfeature/api.py |
Exposes set_provider_and_wait() and adds it to __all__. |
tests/test_api.py |
Updates existing tests to block where needed; adds new tests for non-blocking semantics. |
tests/provider/test_registry.py |
Updates registry tests for the new wait_for_init behavior; adds new async-init tests. |
tests/conftest.py |
Switches fixture initialization to set_provider_and_wait to ensure readiness. |
tests/features/steps/steps.py |
Updates behave steps to use set_provider_and_wait. |
tests/features/steps/metadata_steps.py |
Updates behave steps to use set_provider_and_wait. |
README.md |
Documents non-blocking set_provider() and adds example for set_provider_and_wait(); adjusts several snippets for API correctness. |
Comments suppressed due to low confidence (2)
openfeature/provider/_registry.py:130
- With async initialization,
_shutdown_providercan run before_run_initializehas ever set an entry in_provider_status.del self._provider_status[provider]will then raiseKeyError, which is caught and reported as a shutdown failure (and also leaves status cleanup inconsistent). Consider usingpop(..., None)and/or inserting an initial NOT_READY status when initialization starts so shutdown/clear is safe while init is in-flight.
def _shutdown_provider(self, provider: FeatureProvider) -> None:
try:
if hasattr(provider, "shutdown"):
provider.shutdown()
del self._provider_status[provider]
except Exception as err:
openfeature/provider/_registry.py:108
_initialize_providernow spawns a background thread which always callsself.dispatch_event(...PROVIDER_READY/ERROR...)afterinitialize()finishes. If the provider is replaced/cleared/shutdown while init is still running, this thread can still update_provider_statusand run global/client handlers after the provider has been detached, causing late/incorrect events and state resurrection. Consider tracking init threads/futures and canceling/ignoring completion for providers no longer registered (e.g., generation token or per-provider state guarded by a lock).
def _initialize_provider(
self, provider: FeatureProvider, wait_for_init: bool = False
) -> None:
provider.attach(self.dispatch_event)
if wait_for_init:
self._run_initialize(provider, raise_on_error=True)
else:
thread = threading.Thread(
target=self._run_initialize,
args=(provider,),
kwargs={"raise_on_error": False},
daemon=True,
)
thread.start()
def _run_initialize(
self, provider: FeatureProvider, raise_on_error: bool = False
) -> None:
try:
if hasattr(provider, "initialize"):
provider.initialize(self._get_evaluation_context())
self.dispatch_event(
provider, ProviderEvent.PROVIDER_READY, ProviderEventDetails()
)
except Exception as err:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
There's a related PR (#567) worth flagging. It takes a purely additive approach — This PR makes |
cd994d6 to
86f8b63
Compare
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
5f4f5fd to
3dd16c1
Compare
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Summary
set_provider()now returns immediately; provider initialization runs in a background thread (daemon)set_provider_and_wait()that blocks until initialization completes or re-raises on failureset_provider_and_waitwhere init completion is required; adds 8 new tests covering non-blocking semantics explicitlyMotivation
Closes #594. Spec Requirement 1.1.2.4 implies
set_provider()should be non-blocking, with a separate variant for callers who need to wait. Python was the only SDK whereset_provider()blocked by default. The gunicorn DoS scenario in the issue is real — a provider that hangs during init causes gunicorn to kill the worker with a crypticWORKER TIMEOUT.All other SDKs (Go, Java, JS/TS, Ruby) have the same split: a fire-and-forget default and a
*AndWaitvariant. This implementation follows Ruby's approach since it's also synchronous — await_for_initflag in the registry routes to eitherThread(daemon=True).start()or an inline call.Behavior change
set_provider()— non-blocking (default):initialize()completesPROVIDER_NOT_READYPROVIDER_ERRORevent; they are not propagated to the callerset_provider_and_wait()— blocking:initialize()completes or raisesREADY(or in error state) before the call returnsNotes
This is a breaking behavioral change for callers who relied on
set_provider()blocking until ready. Tagging asfeat!so release-please bumps the minor version (pre-1.0, so 0.9.x → 0.10.0 perbump-minor-pre-major). Getting this in before 1.0 is the right call to avoid carrying it as a breaking change post-stable.