Repository navigation
Persist the DMS version in resources.json serial - #7046
shreyas-goenka wants to merge 12 commits into
Conversation
Integration test reportCommit: d2383c8
Top 6 slowest tests (at least 2 minutes):
|
0934179 to
1b37103
Compare
|
|
||
| # The service owns the version and the file persists no serial, so take it from the deployment. | ||
| # Rebuilt in header order, since the file has no serial key to overwrite in place. | ||
| # The file's serial is a copy of the DMS version to ensure outdated resource state is not read. |
There was a problem hiding this comment.
We should mention that it's serial of post-migration and it's not updated further?
There was a problem hiding this comment.
That'll be a followup PR. Right now it's still just the serial that's being updated every time.
| "state_version": 3, | ||
| "cli_version": "[CLI_VERSION]", | ||
| "lineage": "[UUID]", | ||
| "serial": 2, |
There was a problem hiding this comment.
Can we also extract serial from resources.json? To show that it's one before the "serial" as returnd by print_state.py
There was a problem hiding this comment.
We already do that. print_state.py --no-dms prints the local state. The remote state is not updated on bundle destroy that's why it's missing
| // VersionID is the DMS counterpart of Data.Serial - same meaning, but owned by the service | ||
| // rather than the file, so it lives outside Data and is never written to resources.json. | ||
| // VersionID is the service-owned counter for recorded deployments. A copy is stored in | ||
| // resources.json as Serial to ensure outdated resource state is not read. |
There was a problem hiding this comment.
That's not correct, right? It's only a copy first time, then number in resources.json stays behind.
There was a problem hiding this comment.
Not yet. In a followup.
| {name: "legacy missing serial", omitSerial: true, deploymentID: "123", lastVersionID: 7}, | ||
| {name: "legacy zero serial", deploymentID: "123", lastVersionID: 7}, | ||
| {name: "cached serial behind service", cachedSerial: 2, deploymentID: "123", lastVersionID: 7}, | ||
| {name: "cached serial ahead of service", cachedSerial: 99, deploymentID: "123", lastVersionID: 7}, |
There was a problem hiding this comment.
should not this error? resources.json with higher serial but dms feature set means there were multiple migrations and we're not using correct deployment.
There was a problem hiding this comment.
Do we need to error here? The remote is the source of truth. If for any reason local has a higher serial, it's corrupted. Even if we error all we could direct the user to do is to delete the corrupted local state file.
WDYT? Unless we should guide users here that you might want to delete the remote state?
In any case this is a rare enough edge case that would never happen.
There was a problem hiding this comment.
tbf, there is a scenario where the user might want to destroy their existing state? MAYBE? Lets just error - seems like the safest thing to do.
There was a problem hiding this comment.
Added an error
| lastVersionID int | ||
| wantErr string | ||
| }{ | ||
| {name: "legacy missing serial", omitSerial: true, deploymentID: "123", lastVersionID: 7}, |
There was a problem hiding this comment.
nit: field-per-line is more readable.
This branch has not been deployed
3 participants
Changes
Persist the known DMS version ID in the existing
serialfield ofresources.json. Resource contents still live in DMS.The serial in this file is a cache, not a new source of truth:
OpenandGetSerial()continue to use the service version. Legacy files with an omitted or zero serial remain supported. A claimed version is cached even if it is in progress or fails; unsuccessful version creation does not advance it.Tests
New acceptance test that asserts that even if there's an outdated local state, the serial from remote is read and DMS takes priority.