Skip to content

Conversation

@coolreader18
Copy link
Collaborator

@coolreader18 coolreader18 commented Mar 14, 2025

Description of Changes

From the comment I put in the code:

set the patch version of newly-created metadata files to 0 [which means that patch number of existing metadata files won't be updated past 0 either] -- v1.0.0 set cmp.patch = Some(file_version.patch) when checking version compatibility, meaning it won't be forwards-compatible with a database claiming to be created on v1.0.1, even though that should work. This can be changed once we release v1.1.0, since we don't care about its DBs being backwards-compatible with v1.0.0 anyway.

Expected complexity level and risk

2 - it's slightly tricky to figure out the right thing to do to maintain compatibility while disallowing incompatibility, but I'm confident this is it :)

Testing

  • Added unit tests to make sure it all works, including that v1.0.0 will be compatible with v1.0.1

@bfops bfops added the release-any To be landed in any release window label Mar 17, 2025
@jdetter jdetter added release-1.0.1 and removed release-any To be landed in any release window labels Mar 17, 2025
@bfops bfops self-requested a review March 17, 2025 19:43
@bfops
Copy link
Collaborator

bfops commented Mar 18, 2025

Here is the end-to-end testing I did:

  1. Start a server on the local branch
  2. Publish a module
  3. Run 1.0.0 spacetime start

When my local branch is master, the 1.0.0 spacetime start fails; when my local branch is this PR, spacetime start starts up happily.

Copy link
Collaborator

@bfops bfops left a comment

Choose a reason for hiding this comment

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

Seems good to me!

@coolreader18 coolreader18 added this pull request to the merge queue Mar 19, 2025
Merged via the queue into master with commit 16c5f0e Mar 19, 2025
21 of 22 checks passed
"metadata.toml indicates that this database is from an incompatible \
version of SpacetimeDB. please run a migration before proceeding."
);
meta = existing_meta.check_compatibility_and_update(meta)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

ah I realize now post-merge that we call this on existing_meta with meta as a param, rather than the other way around. This means we're preserving the client_connection_id from existing_meta that we weren't preserving before. Any chance there are negative side-effects of that?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants