feat: address migration gaps - #9487
Conversation
- This is a deferral, in line with #9443 which introduced the breaking change. There's no point in writing v1/ metadata until transport can place v1/.
- Fix C2 item from migration gaps review
- Fix D1 item from migration gaps review
- Fix C1 item from migration gaps review - Pin dependents to the published version, not the bumped one, so a package released later in the same run does not need a version PyPI lacks
- Fix A2 item from migration gaps review, private folders only - Code quality fixes
- Fix A3 item from migration gaps review, floor mechanism only - Every floor starts at 0, so no peer is refused; protocol 1 has never shipped, so 0 is the only correct value today.
- Fix A3 item from migration gaps review, peer gate policy - A peer with UNKNOWN version is still skipped, because nothing can be negotiated without its version
- Fix A2b item from migration gaps review, completing A2 - A folder this client owns is reused after an upgrade, becasuse a peer that has not upgraded still looks for the old name - Delete _filter_patch_compatible, which has no caller left.
- B2: version the crypto key file; refuse an unknown later version because a private key cannot be rebuilt
- B4: version the persisted caches; reset on an unknown later version, because the client rebuilds them. On-disk format becomes {"version", "entries"}
- D2: freeze the VersionInfo V1 field set; every field V2 adds needs a default.
- A4: delete the two unused version exception classes.
- Fix B3 item from migration gaps review - Stamp the format version under a reserved _meta key, so older clients that treat every top-level key as a peer email skip it safely - Log and skip an unknown peer state instead of dropping the peer in silence; the writer keeps other entries, so the record is not erased on Drive
- Fix B1 item from migration gaps review; A2a already fixed the folder half - A later client can reshape a field while the object still parses, which gives a wrong restore silently. Ever load site already falls back to downloading all events, so refusing costs one slow cold start.
- Fix C3 case 1 from migration gaps review - The existing tests assert the negotiated version only; removing the protocol-0 codec fails this test and leaves those passing
- Fix A5 item from migration gaps review; the entry named the wrong pair, the different is between the two dataset methods, not jobs vs datasets.
- Fix A1 from migration gaps review. The dataset transport dropped the protocol version: the sender flattened every file and the receiver rebuilt a flat path, so a v1 dataset arrived with metadata that pointed at a directory that was not there. - create_dataset now writes one copy for each layout in the audience, and each copy gets its own collection. Every collection is shared with the whole audience, so a peer that upgrades later moves to the newer layout with no action by the owner. - The collection folder name carries the version as a v<n> infix before the separator. A client that predates multi-copy searches for the separator and so never lists a layout it cannot read. A protocol-0 name is unchanged, byte for byte. - Private data goes up with the copy that owns it. The copies hold separate private directories, so one upload of the newest left the others local only and a cold start did not restore them. - The watcher keeps the newest readable layout for each dataset, and warns and skips the rest. It keeps a local copy when the owner still publishes the dataset but in no layout this client reads, because that copy is the last one this client could read. - Login writes the remote version file. Only test helpers wrote it before, so the remote file kept the version that first created it. The mismatch check then prompted at every login, and a peer negotiated a job or dataset protocol version from a stale number. Closes the login item of A3. - The login mismatch prompt keeps local and remote data by default and repairs on the next sync. A full wipe is an explicit second choice. delete_unversioned_state is gone with the old first choice. A run with no terminal takes the keep-everything default instead of blocking.
…_router send messages
feat: Migrations/datasets , Migrations/Connection Router
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
pjwerneck
changed the title
| if status == CompatibilityStatus.INCOMPATIBLE: | ||
| # A different client version does not refuse the peer. What each side |
There was a problem hiding this comment.
for A3 I think we should refuse by default, but it should be possible for the user to override this. I think in general this should often be the case
There was a problem hiding this comment.
I also wonder if this is a good idea. I think we should be more clear about what compatibility status means. If two peers have a incompatible syft-client version, we should skip syncing for them
There was a problem hiding this comment.
The override already exists. ignore_peer_version per call and force_ignore_peer_version on the config. The skip message already says Use ignore_peer_version=True to override.
Refusing on a client version difference by default is what deliberately tried to avoid. The live test on real Google Drive confirmed the current policy works. A 0.1.117 DS exchanged a job and read a dataset from a 0.2.0 DO. Under refuse-by-default, every rollout window breaks that.
What exactly is the problem you see? Do you prefer it to fail by default and be explicitly overrided by the user?
| _VersionedFolder = tuple[int, int, int, str, str] | ||
|
|
||
|
|
||
| def _sorted_by_version(folders: list[tuple[str, str]]) -> list[tuple[str, str]]: |
There was a problem hiding this comment.
I wonder if this is the way we should do it. We should be able to know a peers version from their version.json, we dont need multiple sources of truth, we should probably be able to decide based on that instead of this information. For instance, what happens if a peer downgrades?
There was a problem hiding this comment.
Peer capability comes solely from version.json, but it lives inside the folder we are trying to find, so it's a chicken-and-egg situation. The folder name is a recognizable address. It's not a source of truth for the negotiation.
Downgrade cases are handled depending on the direction and other details. I will confirm in the code and add another comment later.
| }, | ||
| ) | ||
|
|
||
| def negotiate_protocol_version( |
There was a problem hiding this comment.
This sounds like we are actively communicating with the peer, which is not what's happening. Something like infer_protocol_version_to_use makes more sense to me.
| # Format of the crypto key file. Raise it when the layout of the file changes, | ||
| # and add a read path for every earlier version. A file with no version was | ||
| # written before the field, and is version 0. | ||
| CRYPTO_KEYS_VERSION = 1 |
There was a problem hiding this comment.
wrt crypto keys version and persisted dict, I think the assumption is that they are backwards compatible. Until we have migrations for them we should probably have a test that we can read old versions?
There was a problem hiding this comment.
The tests exist in:
- tests/unit/test_crypto_keys_version.py
- tests/unit/test_persisted_dict_version.py
| # Return the newest protocol version written (richest layout). | ||
| return created[max(created, key=int)] | ||
|
|
||
| def create_all( |
There was a problem hiding this comment.
W.r.t. datasets I would like to balance writing multiple versions (heavy) against missing a version. It's quite easy to write an additional version later on version mismatch, and it should be possible for the user to modify this easily from the top-level API.
| f"folder(s) on Drive (keeping the one with your data) and retry." | ||
| ) | ||
|
|
||
| def _find_or_adopt_versioned_folder( |
There was a problem hiding this comment.
how do we know this data is compatible?
There was a problem hiding this comment.
Adoption renames a folder and does nothing else. Compatibility is enforced per file, downstream. A checkpoint from a newer client is refused, a cache with unknown version starts empty, a versioned object loads through the registry with the frozen protocol history and the drift check, and so on.
| """Handle a client major/minor mismatch at login. | ||
|
|
||
| The default is to keep local and remote data. Folder adopt, refuse-later | ||
| checks, and cache reset repair state on the next sync. A full wipe is an |
There was a problem hiding this comment.
Re: Flow 2.7 — login with a version mismatch
If the answer is "keep everything and repair on the next sync", we should probably just call sync() right there, and print that we are doing it.
| choices=["bumped", "published"], | ||
| default="bumped", |
There was a problem hiding this comment.
Re: Flow 2.8 — scripts/bump_version.py --dependents published
I wonder if we need the old version (bumped), we can probably remove that
| execute_with_retries( | ||
| self.drive_service.files().update( | ||
| fileId=folder_id, body={"name": current_name} | ||
| ) |
There was a problem hiding this comment.
if we do _find_or_adopt_versioned_folder and we upgrade, how do we know that the renamed folder is in the right format?
| return [json.loads(uncompress_data(raw))["version"] for raw in raw_list] | ||
|
|
||
|
|
||
| def test_a_v2_proposal_for_a_protocol0_peer_downgrades_on_the_wire( |
There was a problem hiding this comment.
we probably need a migration test with encryption on as well
There was a problem hiding this comment.
Agree. Will add it.
4 participants
Summary
Closes most of the migration gaps that block safe upgrades: private Drive folders are adopted by rename, P2P folders are found regardless of the version in the name, and durable surfaces (checkpoints, rolling state, crypto keys, peers JSON, local caches) carry or refuse versions instead of failing silently.
Adds a per-protocol minimum-supported floor (shipped inert at "0"), stops refusing peers on client semver mismatch, and hardens release/CI (export → publish → tag → bump, client migration tests in CI, numeric version ordering). Job protocol skew delivery is covered end-to-end; dataset transport multi-copy (A1) and a few process/policy items remain open.
Note
The base branch is
stephen/syft-client-migrations, notdev. This branch assumes that branch will be merged first.Changes
Done
create_datasetwrites one copy for each protocolversion in the audience. Each copy gets its own Drive collection, and every
collection reaches the whole audience. A peer that upgrades later therefore
moves to the newer layout with no action by the owner. The collection name
holds the version as a
v<n>infix before the separator. A client thatpredates this change therefore never lists a layout it cannot read. A
protocol-0 name does not change. The client uploads the private data of a
copy with that copy. A watcher keeps the newest readable layout for each
dataset, and warns and skips the rest.
_find_or_adopt_versioned_folderrenames the folderof the highest earlier version and keeps the data. The personal datasite
folder, the checkpoints folder, and the rolling-state folder all use it.
_find_p2p_folder_idfinds a folder whatever versionits name holds.
_filter_patch_compatibleis gone.MIN_SUPPORTED_*_PROTOCOL_VERSION. Thevalue travels as
min_supported_version, andnegotiate_protocol_versionenforces it for jobs and for datasets.
(
get_peer_compatibility_status).force_ignore_protocol_versionis gone.deletion is an explicit second choice. A run with no terminal takes the
keep-data answer, so a notebook or a scheduled run does not stop. Login
writes the version file on both sides. A second login therefore shows no
prompt, and a peer reads a current version.
ClientVersionMismatchErrorandProtocolVersionMismatchErrorare gone. The inert semverPROTOCOL_VERSIONfield stays on the wire.
logs a warning. See
test_unknown_peer_forced_path.py.client, and downloads all events instead. A local load that fails reports the
failure.
CRYPTO_KEYS_VERSION. A later version isrefused, because a private key cannot be rebuilt.
SYFT_peers.jsoncarries a version under_meta. The client logs anunknown peer state and skips it, and the rewrite of the raw map keeps the
entry.
PersistedDictwrites{"version", "entries"}. An old flat filereads as version 0, and a later version resets to empty.
--dependents publishedpins the published version, andprotocol_bump_missingcompares against the newest released protocol.just test-client-migrations, and again after a release.reads (
test_job_protocol_skew_delivery.py). A dataset reaches an audienceof mixed protocol versions (
test_dataset_multicopy_delivery.py)._version_ordercompares a version as a number, so"10"is after"2".VersionInfofields(
test_version_info_fields.py).live peer-schema dict.
Open items
One decision on A3 clears the first group. One release
cycle clears the second.
1. Waits on a decision (A3)
A peer can speak a protocol below the floor of a package. The client can refuse
it, warn about it, or go read-only. That question is open, and three items wait
on the answer. Every floor is
"0"today, so the mechanism refuses nothing andno user sees a difference.
target_protocol_versions_for_peerstakesmin(ours, theirs)and applies no floor. A1 makes this path live. Thecorrect action for a peer below the floor is part of the same decision.
because the correct result is not defined without the policy. A manual test
covers this ground now.
2. Closed by the next release
The first two items block the release. Do them before it. No code change on
this branch can close any of the three, because each one needs a release that
happened.
syft-migrationis not on PyPI, and no CD workflow releases it.syft-client, syft-job, and syft-dataset all depend on it with no pin. The
monorepo resolves the dependency from the workspace, so a local install
works. An install of the next release fails.
versions: syft-client 0.1.117, syft-job 0.1.39, syft-dataset 0.1.20. The
release job publishes the version that is on the branch, and PyPI refuses a
version that exists.
syft_client_install_sourceandattestation_token. The published wheel doesnot define these fields, because PyPI received it 12 days before the fields
arrived. A correct file needs the published wheel as the source.
3. Needs a rehearsal by a person
VersionInfohas a real migration pair. Both items close when one persondoes a full object version increase on a branch that never merges. That
person records each point where the tools say nothing. A synthetic V2 must not
merge: a released V2 freezes its schema and owes a migration forever.
4. Open by decision
5. Permanent, and not work
PROTOCOL_VERSIONfield (A4). It carries no meaning, and itmust stay on the wire, because a supported older client parses it.
6. A rare failure, kept loud on purpose
_expect_one, which raises andnames both folders. An automatic merge can put the data of a user out of
reach, so the client stops instead. Folder adoption makes this case rare.
7. Not measured
file_hashesreplays the event stream. The resetworks. The cost for a large datasite is not measured.
Testing
Asana task
https://app.asana.com/1/1185126988600652/project/1216249688888494/task/1217005273770094?focus=true