feat: Add async migrations - #470
Conversation
abe7dac to
c735e0e
Compare
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c735e0e. Configure here.
c735e0e to
ec7746a
Compare
…ic base Drop _MigrationConfigBase and give MigrationConfig and AsyncMigrationConfig their own bodies. The shared base saved only a few trivial passthrough properties while adding generic indirection, and full duplication matches how Config/AsyncConfig are handled. MigrationConfig reverts to its pre-async standalone form.
Give MigratorBuilder and AsyncMigratorBuilder their own read_execution_order/ track_latency/track_errors methods instead of sharing a base for three trivial property setters. MigratorBuilder and types.py revert to their pre-async form.
| @@ -1,8 +1,15 @@ | |||
| # async_migrator is import-cheap (asyncio stdlib only, no aiohttp), so it is | |||
|
|
||
| return authoritative_result | ||
|
|
||
| async def __write_both(self, authoritative: AsyncExecutor, nonauthoritative: AsyncExecutor, tracker: OpTracker) -> Tuple[OperationResult, Optional[OperationResult]]: |
There was a problem hiding this comment.
Do we have a standard approach we are using for task cancellation? I am concerned about cancelling between the writes potentially. I am not familiar enough with the ecosystem to know if we can just say not to do that.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ 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 d62ed3c. Configure here.
| nonauthoritative_result = await nonauthoritative.run() | ||
| tracker.invoked(nonauthoritative.origin) | ||
|
|
||
| return authoritative_result, nonauthoritative_result |
There was a problem hiding this comment.
Dual-write cancellation leaves origins inconsistent
Medium Severity
__write_both awaits the authoritative write, then the nonauthoritative write, with no protection against task cancellation between those awaits. In dual-write stages, a cancel after the first write commits can skip the second origin and also skip track_migration_op, leaving origins out of sync. This is a new asyncio risk relative to the sync migrator.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d62ed3c. Configure here.
2 participants
Adds the async migration surface for the async Python SDK —
AsyncMigrator,AsyncMigratorBuilder,AsyncMigrationConfig, and theAsyncMigratorFntype — inldclient/migrations/async_migrator.py, eagerly exported fromldclient/migrations/__init__.py(import-cheap: asyncio stdlib only, no aiohttp).Also carries the migration config/builder dedup shared by the sync and async surfaces:
types.pygains_MigrationConfigBase[_MigratorFnT](Generic) and_MigratorBuilderBase(fluent-setter mixin), andmigrator.py'sMigratorBuilderis re-based onto that mixin. These are behavior-preserving refactors of already-released sync code.The public async classes carry experimental
.. caution::blocks.Self-contained: depends only on already-merged foundation work. The
AsyncLDClient/Contextreferences areTYPE_CHECKING-only (annotations kept lazy viafrom __future__ import annotations); no runtime import of any not-yet-merged async module.No CHANGELOG or version changes (handled at release).
Tracked internally: SDK-2767
Note
Medium Risk
New experimental API on the migration/data-path; wrong stage or dual-write behavior could affect customer backends, though it mirrors proven sync logic and is gated with caution docs.
Overview
Adds an experimental async migration API for the async Python SDK, parallel to the existing sync
Migrator.New public surface in
async_migrator.py:AsyncMigrator/AsyncMigratorImpl,AsyncMigratorBuilder,AsyncMigrationConfig, andAsyncMigratorFn(coroutine callbacks returningResult).read/writeawaitAsyncLDClient.migration_variation, run old/new origins per migration stage (including parallel/serial/random dual-read viaasyncio.gather), optional read consistency comparison, and calltrack_migration_opsynchronously (not awaited).AsyncMigratorBuildervalidates read/write config onbuild()and returns an error string when missing.ldclient/migrations/__init__.pyeagerly exports the async types (stdlibasyncioonly, documented as import-cheap). Broad pytest coverage intest_async_migrator.pymirrors sync migrator behavior (payloads, invoked/latency/error/consistency telemetry, exceptions, execution order).Reviewed by Cursor Bugbot for commit d62ed3c. Bugbot is set up for automated code reviews on this repo. Configure here.