Skip to content

Add a compas_pb migrate command for older serialized data - #85

Merged
gonzalocasas merged 2 commits into
mainfrom
feat/migrate-cli
Aug 10, 2026
Merged

gonzalocasas merged 2 commits into
mainfrom
feat/migrate-cli

Conversation

@gonzalocasas

@gonzalocasas gonzalocasas commented Aug 5, 2026

Copy link
Copy Markdown
Member

Small CLI tool to migrate bytes from older version to latest (using uvx)

What type of change is this?

  • Bug fix in a backwards-compatible manner.
  • New feature in a backwards-compatible manner.
  • Breaking change: bug fix or new feature that involve incompatible API changes.
  • Other (e.g. doc update, configuration, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • I ran all tests on my computer and it's all green (i.e. invoke test).
  • I ran lint on my computer and there are no errors (i.e. invoke lint).
  • I added new functions/classes and made them available on a second-level import, e.g. compas.datastructures.Mesh.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate)

The wire-version check is a hard gate, so data written by an incompatible version is refused
outright. That was the right call -- field numbers are reused across format revisions, so an
old blob read by a new build can silently misparse into plausible but wrong geometry -- but it
left users with archived blobs no way forward beyond hand-rolling a two-environment dance.

Both formats cannot be read in one process: the two builds generate protobuf descriptors from
the same file path, and only one can register. So `migrate` decodes the blob in an ephemeral
`uv` environment holding the version that wrote it, bridges through COMPAS JSON, and re-encodes
with the current build. Nothing is installed into the caller's environment.

The source version is read by scanning the top-level framing rather than by parsing the
message. `ParseFromString` does recover the tag, but it eagerly misparses the whole payload to
get one field, which is the exact hazard the gate exists to prevent. Scanning field 2 and
skipping field 1 by its length prefix cannot fail on payload contents. Blobs written before
v0.4.1 carry no version tag at all, hence `--from-version`.

Verified against blobs written by real 0.5.0 and 0.4.10 installs: geometry, datastructures,
attributes and explicitly set guids all survive. Two caveats are inherent to the old data and
are documented rather than fixed -- float32 precision is not recovered, and integral floats
come back as ints because that is how the old reader unwrapped protobuf `Value`.

The test that spawns `uv` is marked `network` so it can be deselected in CI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@gonzalocasas
gonzalocasas requested review from WeiTing1991, chenkasirer and ericgozzi and a lite review from Copilot and removed request for Copilot August 5, 2026 13:25
`compas_pb migrate` shells out to uv to read data written by older versions, so the migration
test fails on a runner without it. compas-actions.build v5 takes a `management_tool` input,
which installs uv before running `invoke test`.

Verified the full suite, including the migration test, on Python 3.9 -- the oldest cell in the
matrix -- so the ephemeral environment resolves compas_pb 0.5.0 there too.

Note that v5 skips the build entirely on macOS with Python 3.9, so that cell no longer runs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 5, 2026 13:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a compas_pb migrate CLI command (and supporting Python helpers) to re-encode wire-incompatible serialized data produced by older compas_pb versions into the current wire format by decoding in an ephemeral uv environment and bridging through COMPAS JSON.

Changes:

  • Added a new CLI module (compas_pb.cli) providing compas_pb migrate, plus detect_wire_version() / migrate_bytes() for programmatic use.
  • Updated the incompatible-wire-format error message to point users to the new migration workflow.
  • Added documentation, changelog entry, and tests (including a network-marked end-to-end migration test), and wired the CLI into packaging/CI.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/test_cli.py Adds CLI/migration tests, including a network-marked full migration path.
src/compas_pb/core.py Updates wire-incompatibility error message to reference the migrate command.
src/compas_pb/cli.py Introduces the new CLI and migration implementation using uv run.
pyproject.toml Registers compas_pb console script and adds a network pytest marker.
mkdocs.yml Adds the migration guide to the docs navigation.
docs/migration.md Documents why migration is needed and how to use compas_pb migrate.
CHANGELOG.md Records the new CLI feature, Python helpers, and doc addition.
.github/workflows/release.yml Updates CI action version and configures uv as management tool.
.github/workflows/build.yml Updates CI action version and configures uv as management tool.

Comment thread src/compas_pb/cli.py
Comment on lines +127 to +131
if source_version is None:
source_version = detect_wire_version(blob)
if source_version is None:
raise ValueError("this blob carries no version tag, so the version that wrote it cannot be detected; pass --from-version explicitly")

Comment thread src/compas_pb/cli.py
Comment on lines +143 to +147
def _cmd_migrate(args) -> int:
blob = sys.stdin.buffer.read() if args.input == "-" else open(args.input, "rb").read()
if not blob:
print("error: no input data", file=sys.stderr)
return 1
Comment thread pyproject.toml
Comment on lines +116 to +118
markers = [
"network: needs network access to fetch an older compas_pb into an ephemeral environment",
]
Comment thread src/compas_pb/cli.py
Comment on lines +62 to +74
if wire_type == 0:
_, pos = _read_varint(blob, pos)
elif wire_type == 1:
pos += 8
elif wire_type == 5:
pos += 4
elif wire_type == 2:
length, pos = _read_varint(blob, pos)
if field_no == 2:
return blob[pos : pos + length].decode("utf-8")
pos += length
else:
raise ValueError("unsupported protobuf wire type {}; not a compas_pb message".format(wire_type))

@WeiTing1991 WeiTing1991 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, Thank you!!!

@gonzalocasas
gonzalocasas merged commit bc4d976 into main Aug 10, 2026
18 checks passed
@gonzalocasas
gonzalocasas deleted the feat/migrate-cli branch August 10, 2026 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants