Uh oh!
There was an error while loading. Please reload this page.
feat!: make set_provider non-blocking, add set_provider_and_wait - #595
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #595 +/- ##
==========================================
- Coverage 98.35% 98.28% -0.07%
==========================================
Files 45 45 Lines 2183 2386 +203 ==========================================
+ Hits 2147 2345 +198 - Misses 36 41 +5
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
jonathannorris
commented
May 20, 2026
There's a related PR (#567) worth flagging. It takes a purely additive approach — This PR makes |
cd994d6 to
86f8b63CompareSigned-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
5f4f5fd to
3dd16c1CompareSigned-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert
left a comment
There was a problem hiding this comment.
Pulled locally and added a few fixes:
- closed the race where concurrent
set_providercalls with the same instance could double-initialize - prevented a late
PROVIDER_READY/PROVIDER_ERRORfrom a background init from clobbering the status of a provider that was already replaced or shut down_ - eliminated a
KeyErrorin shutdownby usingpopnotdel - made old-provider shutdown async so a hanging shutdown() can't block set_provider (otherwise the DoS this PR fixes is only half-closed).
I added tests that seemed reasonable for these. Python is not my strong-suit so please take a look @jonathannorris .
commit is here.
I think this resolves all the review feedback by bots and @dd-oleksii .
toddbaert
commented
May 25, 2026
Leaving this open for a bit because it's a pretty fundamental change, but I will get it merged this week. Please weigh in if you have objections. |
jonathannorris
commented
May 25, 2026
We found an issue with the stale check @toddbaert added in The fix is to check active registration instead: # current (insufficient)ifprovidernotinself._provider_status:
return# correctifproviderisnotself._default_providerandprovidernotinself._providers.values():
returnWill push a fix. |
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
There was a problem hiding this comment.
It looks like there is a race condition on shutdown.
Now that old-provider shutdown runs in the background, re-registering the same provider instance while its previous shutdown is still finishing lets the stale shutdown wipe out the fresh registration's event wiring and status.
Proposed Fix (or something like):
- Add a per-registration generation token (a counter bumped each time a provider is registered).
- Have each background shutdown capture the token it was started for.
- Before the shutdown does its final
detach()and status pop, re-check that the provider's current token still matches. - If the provider has been re-registered since (newer token), skip the cleanup instead of clobbering the new setup.
Good call. This race is valid; repro added as |
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Uh oh!
There was an error while loading. Please reload this page.
…n-feature#595) * feat!: make set_provider non-blocking, add set_provider_and_wait Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fix: ruff format signature collapse in api.py Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fix: use threading.Event in error event test to avoid flaky busy-wait Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fixup: pr feedback and additional checks Signed-off-by: Todd Baert <todd.baert@dynatrace.com> * fix: check active registration in stale-init guard, not _provider_status Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fixup: edge shutdown race Signed-off-by: Todd Baert <todd.baert@dynatrace.com> --------- Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: gruebel <anton.gruebel@gmail.com>
* test(e2e): add Behave step definitions for context merging Add the missing E2E step definitions so the contextMerging.feature scenarios from the OpenFeature spec run against python-sdk. Fixes#500 Changes: - Bump spec submodule to 130df3eb so contextMerging.feature is copied in during the `poe e2e` task. - Add tests/features/environment.py with a before_scenario hook that resets provider/hook/API-context/transaction-context state, so scenarios cannot leak state between features. - Add tests/features/steps/context_merging_steps.py: - RetrievableContextProvider captures the merged EvaluationContext it receives, so assertions can inspect what the SDK merged. - Step definitions for all scenarios in contextMerging.feature: single-level insert, multi-level insert, and per-key overwrite precedence across API / Transaction / Client / Invocation / Before Hooks. - Client-level context is set via direct attribute assignment on OpenFeatureClient.context (no new setter), since merging already honors client.context (openfeature/client.py:422-429). Runs clean: 4 features / 50 scenarios / 233 steps. Signed-off-by: gruebel <anton.gruebel@gmail.com> * docs: fix inaccuracies in README code examples (#592) Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * fix: correctly reset api state on shutdown (#589) correctly reset api state on shutdown Signed-off-by: gruebel <anton.gruebel@gmail.com> * chore(main): release 0.9.0 (#555) Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * chore(deps): update pre-commit hook tox-dev/pyproject-fmt to v2.21.2 (#601) * chore(deps): update pre-commit hook tox-dev/pyproject-fmt to v2.21.2 * upper bound toml-fmt-common till fixed Signed-off-by: gruebel <anton.gruebel@gmail.com> --------- Signed-off-by: gruebel <anton.gruebel@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <anton.gruebel@gmail.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * chore(deps): update astral-sh/setup-uv action to v8 (#603) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * chore(deps): update codecov/codecov-action action to v6 (#604) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * chore(deps): update googleapis/release-please-action action to v5 (#605) * chore(deps): update googleapis/release-please-action action to v5 * fix config Signed-off-by: gruebel <anton.gruebel@gmail.com> --------- Signed-off-by: gruebel <anton.gruebel@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <anton.gruebel@gmail.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * chore(deps): update pre-commit hook pre-commit/mirrors-mypy to v2 (#606) * chore(deps): update pre-commit hook pre-commit/mirrors-mypy to v2 * update config Signed-off-by: gruebel <anton.gruebel@gmail.com> --------- Signed-off-by: gruebel <anton.gruebel@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <anton.gruebel@gmail.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * chore(deps): update dependency prek to >=0.4.3,<0.5.0 (#607) * chore(deps): update dependency prek to >=0.4.3,<0.5.0 * adjust CI Signed-off-by: gruebel <anton.gruebel@gmail.com> --------- Signed-off-by: gruebel <anton.gruebel@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: gruebel <anton.gruebel@gmail.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * feat!: make set_provider non-blocking, add set_provider_and_wait (#595) * feat!: make set_provider non-blocking, add set_provider_and_wait Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fix: ruff format signature collapse in api.py Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fix: use threading.Event in error event test to avoid flaky busy-wait Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fixup: pr feedback and additional checks Signed-off-by: Todd Baert <todd.baert@dynatrace.com> * fix: check active registration in stale-init guard, not _provider_status Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * fixup: edge shutdown race Signed-off-by: Todd Baert <todd.baert@dynatrace.com> --------- Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * fix: isolate provider event handler dispatch (#599) * Isolate provider event handlers Signed-off-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> * Address event handler review feedback Signed-off-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> * test: cover event dispatch noop path Signed-off-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> * fixup: drain executor at exit and relax non-blocking test timing margin Signed-off-by: Todd Baert <todd.baert@dynatrace.com> --------- Signed-off-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * test: fix flaky event handler test (#609) fix flaky event handler test Signed-off-by: gruebel <anton.gruebel@gmail.com> * chore(deps): update pre-commit hook astral-sh/ruff-pre-commit to v0.15.15 (#608) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * chore(main): release 0.10.0 (#602) * chore(main): release 0.10.0 Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com> * docs: clarify non-blocking set_provider behavior in changelog Signed-off-by: Todd Baert <todd.baert@dynatrace.com> --------- Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: gruebel <anton.gruebel@gmail.com> * fix CR comments Signed-off-by: gruebel <anton.gruebel@gmail.com> * cleanup Signed-off-by: gruebel <anton.gruebel@gmail.com> --------- Signed-off-by: gruebel <anton.gruebel@gmail.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.com> Co-authored-by: Jonathan Norris <jonathan.norris@dynatrace.com> Co-authored-by: Anton Grübel <anton.gruebel@gmail.com> Co-authored-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: Nguyen Cat Luong <pkiphone.anhluong@gmail.com> Co-authored-by: Lucas-FManager <265058144+Lucas-FManager@users.noreply.github.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.Relates to: #96
Fixes: #594