| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@anxkhn Thanks for the PR! Do you think it's good to call out in the PR / release notes that this only helps installs that have not already run v0071, users who already migrated already lost those intervals and still need a re-backfill (no recovery path in this PR). |
Sorry, something went wrong.
The v0071 intervals migration compared a bare dev version string against `used_dev_versions`, which only ever holds `(name, dev_version)` tuples. A string is never an element of a set of 2-tuples, so `dev_version not in used_dev_versions` was always true and the guard collapsed to `if is_dev: continue`. Every dev interval whose version was still live (it had already passed the `used_versions` check) was dropped on upgrade, forcing a re-backfill of dev environments. Compare the correctly-typed `(name, dev_version)` key so only genuinely unused dev intervals are removed. Add a regression test that runs the migration against an in-memory DuckDB state with a still-live dev interval and asserts it survives. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
|
@anxkhn Wanted to follow up on my prev comment! |
Sorry, something went wrong.
|
good call. called that out in the pr description. this only helps installs that have not already run v0071; already-migrated dbs still need a manual repair. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Note: This only helps installs that have not already run v0071. Users who already migrated still have the dropped intervals and need a re-backfill / manual repair. There is no recovery path in this PR.
The v0071 intervals migration drops every still-live dev interval on upgrade
because of a type mismatch in a set-membership check.
In _migrate_intervals, used_dev_versions is a Set[Tuple[str, str]] and its
only insertion adds a (name, dev_version) 2-tuple:
But the guard that decides whether to drop a dev interval compares a bare
dev_version string against that set:
A str is never an element of a set of 2-tuples, so dev_version not in used_dev_versions is always True and the condition collapses to
if is_dev: continue. Every dev interval that had already passed the preceding
used_versions check (i.e. is still live) is discarded, forcing a re-backfill of
dev environments after the upgrade.
This changes the check to compare the correctly-typed (name, dev_version) key,
matching the tuples stored in used_dev_versions, so only genuinely unused dev
intervals are removed.
Test Plan
tests/core/state_sync/test_state_sync.py. It builds an in-memory DuckDB state
at schema v70 (intervals table without the dev_version column) with a live
prod snapshot/interval and a live dev snapshot/interval, runs the v0071
migrate_schemas + migrate_rows, and asserts both intervals survive.
{'prod_int'} != {'dev_int', 'prod_int'}) and passes with it.
to a live snapshot) is still correctly removed, so the fix is not over-broad.
tests/core/state_sync/test_state_sync.py migration/interval subset passes.
Checklist