Skip to content

feat: Add async FDv2 data system - #486

Open
jsonbailey wants to merge 6 commits into
mainfrom
jb/sdk-60/async-fdv2-datasystem
Open

feat: Add async FDv2 data system#486
jsonbailey wants to merge 6 commits into
mainfrom
jb/sdk-60/async-fdv2-datasystem

Conversation

@jsonbailey

@jsonbailey jsonbailey commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Overview

Part of the async Python SDK work (epic SDK-60). Adds the async FDv2 data system (coordinator) and wires it into the async client. Targets main (its predecessor, #485 async FDv2 data sources, has merged).

This is experimental and should not be considered production-ready.

What this PR adds

  • impl/datasystem/async_fdv2.pyAsyncFDv2, the async data system that coordinates the async initializers and synchronizers, mirrors the sync FDv2 fallback/recovery behavior, and exposes the async data source status and flag tracking. Includes AsyncFeatureStoreClientWrapper for persistent-store availability polling.
  • async_client.py wiring: _make_data_system builds AsyncFDv2; _wire_data_source_sessions shares the client's aiohttp session into the async data source builders so they reuse the connection pool.

Async readiness gate (awaitable end-to-end)

Readiness and availability are now awaitable so a persistent store populated by another process (warm start / daemon) is recognized:

  • AsyncDataSystem.data_availability and AsyncLDClient.is_initialized() are coroutines; the eval path awaits data_availability() directly.
  • AsyncFeatureStore gains an abstract async is_initialized() (with $inited caching + monotonic latch in AsyncCachingStoreWrapper, surfaced via AsyncStore.is_ready()). The getattr duck-type is dropped, so a custom store that omits it fails at construction rather than silently serving defaults.
  • The gate catches a persistent-store error and degrades to DEFAULTS rather than raising, so variation() never throws on a store outage. (Sync counterpart: fix: Do not propagate persistent-store errors from the sync FDv2 warm-start check #506.)

Wrapper hardening

  • is_monitoring_enabled() delegates to the store's own opt-in (matching sync), so a store that cannot report availability is not polled and left stuck unavailable.
  • close() is idempotent, bounds the availability-poller stop with a timeout, and logs errors from the inner store close.
  • AsyncStore.close() logs and swallows close errors instead of returning them.

Shared refactor

  • impl/datasystem/fdv2_common.py gains module-level fallback_condition / recovery_condition, shared by the sync and async data systems.

Testing

  • LD_SKIP_DATABASE_TESTS=1 uv run pytest ldclient/testing/impl/datasystem/ and ldclient/testing/test_async_client.py — green.
  • make lint (mypy, isort, pycodestyle) — clean.

Tracked internally: SDK-2870


Note

Overview
Adds async Flag Delivery v2 support by introducing AsyncFDv2, wiring it into AsyncLDClient when datasystem_config is set (replacing the previous NotImplementedError), and sharing the client's aiohttp session with async polling/streaming data source builders via _wire_data_source_sessions.

Readiness and availability are now async so persistent stores populated by another process can be detected: AsyncLDClient.is_initialized(), AsyncDataSystem.data_availability(), and a new AsyncFeatureStore.is_initialized() (with caching/latching in AsyncCachingStoreWrapper and AsyncStore.is_ready()). Contract-test harnesses await these checks accordingly.

Sync FDv2 is refactored to share wiring through _FDv2Base and module-level fallback_condition / recovery_condition in fdv2_common.py. AsyncFDv2 mirrors initializer/synchronizer coordination, FDv1 fallback, persistent-store outage recovery, and adds AsyncFeatureStoreClientWrapper for availability polling.

AsyncStore.close() logs and swallows close errors instead of returning them. Large new test suites cover AsyncFDv2, async persistence, and readiness gating.

Reviewed by Cursor Bugbot for commit 111bb37. Bugbot is set up for automated code reviews on this repo. Configure here.

@jsonbailey
jsonbailey force-pushed the jb/sdk-60/async-fdv2-datasystem branch 2 times, most recently from 027941e to b911aa7 Compare August 13, 2026 22:20
@jsonbailey
jsonbailey force-pushed the jb/sdk-60/async-fdv2-datasystem branch from b911aa7 to 35dd8be Compare August 13, 2026 22:32
@jsonbailey
jsonbailey marked this pull request as ready for review August 14, 2026 12:59
@jsonbailey
jsonbailey requested a review from a team as a code owner August 14, 2026 12:59
Comment thread ldclient/impl/datasystem/async_fdv2.py Outdated
@jsonbailey
jsonbailey force-pushed the jb/sdk-60/async-fdv2-datasystem branch from 35dd8be to c88b6aa Compare August 14, 2026 14:22
Comment thread ldclient/impl/datasystem/async_fdv2.py Outdated
@jsonbailey
jsonbailey force-pushed the jb/sdk-60/async-fdv2-datasystem branch from c88b6aa to 1c996d6 Compare August 14, 2026 14:58
Comment thread ldclient/impl/datasystem/async_fdv2.py
@jsonbailey
jsonbailey force-pushed the jb/sdk-60/async-fdv2-datasystem branch from 1c996d6 to 9f6ce38 Compare August 14, 2026 15:05
Comment thread ldclient/impl/datasystem/async_fdv2.py
@jsonbailey
jsonbailey marked this pull request as draft August 14, 2026 19:18
Base automatically changed from jb/sdk-60/async-fdv2-sources to main August 14, 2026 20:17
@jsonbailey
jsonbailey force-pushed the jb/sdk-60/async-fdv2-datasystem branch 2 times, most recently from cde1b0b to ef7c261 Compare August 14, 2026 20:20
@jsonbailey
jsonbailey force-pushed the jb/sdk-60/async-fdv2-datasystem branch 3 times, most recently from 22a3bab to 4b20c9a Compare August 26, 2026 13:54
@jsonbailey
jsonbailey marked this pull request as ready for review August 26, 2026 22:19
@jsonbailey
jsonbailey force-pushed the jb/sdk-60/async-fdv2-datasystem branch from 4b20c9a to 0db5afa Compare August 26, 2026 22:19

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0db5afa. Configure here.

Comment thread ldclient/async_client.py Outdated
Comment thread ldclient/impl/datasystem/async_fdv2.py Outdated
# Apply the basis to the store
await self._store.apply(basis.change_set, basis.persist)

# Set ready event if and only if a selector is defined for the changeset

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are changing the spec and the logic around this. I think it's fine to leave for now, but just making sure it's on your radar.

Comment thread ldclient/impl/datasystem/async_fdv2.py
Comment thread ldclient/async_client.py Outdated
Comment thread ldclient/impl/datasystem/async_fdv1.py Outdated
Comment thread ldclient/async_client.py Outdated
Comment thread ldclient/impl/datasystem/async_fdv2.py
Comment thread ldclient/impl/datasystem/async_fdv2.py Outdated
Comment thread ldclient/impl/datasystem/async_fdv2.py Outdated
Comment thread ldclient/impl/datasystem/async_fdv2.py
Comment thread ldclient/impl/datasystem/async_fdv2.py
Addresses review feedback on the interim warm-start refresh. The async
readiness path is now awaitable at every layer instead of a private
two-step refresh feeding a synchronous gate:

- AsyncDataSystem.data_availability and AsyncLDClient.is_initialized()
  become coroutines; the eval path awaits data_availability() directly.
- AsyncFeatureStore gains an abstract async is_initialized(); the
  getattr duck-type is dropped, so a custom store that omits it fails
  at construction instead of silently serving DEFAULTS forever.
- The warm-start store error is caught inside data_availability and
  degrades to DEFAULTS, so evaluation never propagates a store error.
- Removes the interim refresh_availability, cold-path gating, and
  start-time refresh, which are subsumed by the awaitable gate.
- Adds a per-iteration stop-event check in the synchronizer loop so a
  perpetually-ready queue cannot starve the stop signal.

The sync data system is unchanged.
jsonbailey added a commit that referenced this pull request Aug 27, 2026
AsyncDataSystem.data_availability and AsyncLDClient.is_initialized()
become coroutines; AsyncFeatureStore gains an abstract async
is_initialized() and the getattr duck-type is dropped; the warm-start
store error is caught inside data_availability and degrades to DEFAULTS;
the interim refresh_availability, cold-path gating, and start-time
refresh are removed. Extracted to PR #486. Sync unchanged.
…n close()

- is_monitoring_enabled now delegates to the store's own opt-in, matching
  the sync wrapper. A store that cannot report availability is no longer
  polled, so it is never marked unavailable with no path back to recovery.
- close() does nothing on a later call, bounds the poller stop with a
  timeout so a wedged poller cannot hang shutdown, and logs an error from
  the inner store's close instead of letting it propagate.
…g them

AsyncStore.close() previously returned the close error as
Optional[Exception], which the only caller (AsyncFDv2.stop) discarded, so
a failed close was silently lost. It now logs a warning and returns None.
Closing happens at shutdown, where there is no caller left to react to the
error, so logging is the useful outcome.
Comment thread ldclient/impl/datasystem/async_fdv2.py Outdated
availability is not polled, so it is never marked unavailable with no
path back to recovery.
"""
return callable(getattr(self._store, "is_available", None))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we checking for the store is_available elsewhere now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The recovery poller still calls is_available, in _check_availability. This method now decides only whether to monitor. It uses the store's own opt-in (is_monitoring_enabled), which is true when the core provides is_available.


Does nothing on a later call, so closing more than once is safe.
"""
if self._closed:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could get us into another situation where we never close since the boolean is set before any of the actual closing happens... this might be fine if we think that calling the stop/close functions are our best effort.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and this is best-effort by design. We set the flag first so a second close() call does nothing. Both teardown steps have their own guards, so neither one raises. Only task cancellation can stop the close early, which means an abnormal shutdown. If you want a retry after a failed close, we would need a lock so callers wait for the first close — more than a shutdown path needs, but I can add it if you prefer.

Comment thread ldclient/async_client.py
return self._config.offline

def is_initialized(self) -> bool:
async def is_initialized(self) -> bool:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this is a problem, but does python do truthy so when is_initialized() returns a future then if client.is_initialized() will always test true?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_initialized() is now a coroutine, so if client.is_initialized() is always true unless you await it. This applies to every async method here, such as variation and all_flags_state. The fix is to await the call. The docstring now states that it is a coroutine, and all internal calls await it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants