Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add migration metadata to observation schema #280

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

EvanHahn
Copy link
Contributor

@EvanHahn EvanHahn commented Dec 17, 2024

This adds a new field, migrationMetadata, which has two sub-fields:

  • originalDocument: the original observation, in case we ever need it
  • hypercore: metadata about the observation in the old Hypercore

I considered adding this to the Common schema, but I don't think it makes sense there because we won't necessarily be migrating all types, like CoreOwnership.

This adds a new field, `migrationMetadata`, which has two sub-fields:

- `originalDocument`: the original observation, in case we ever need it
- `hypercore`: metadata about the observation in the old Hypercore

I considered adding this to the `Common` schema, but I don't think it
makes sense there because we won't necessarily be migrating all types,
like `CoreOwnership`.
@EvanHahn EvanHahn changed the title feat: add migration metadata to common schema feat: add migration metadata to observation schema Dec 18, 2024
@EvanHahn EvanHahn marked this pull request as ready for review December 18, 2024 17:27
@EvanHahn EvanHahn merged commit 6097ff2 into main Dec 18, 2024
6 checks passed
@EvanHahn EvanHahn deleted the migration-metadata branch December 18, 2024 18:01
EvanHahn added a commit to digidem/comapeo-core that referenced this pull request Dec 19, 2024
This new version adds a new property to the Observation table,
`migrationMetadata`. See [`@comapeo/schema` PR 280][0] for details.

[0]: digidem/comapeo-schema#280
@gmaclennan
Copy link
Member

I've reverted this because I think it contains bugs. It seems like the original observation is stored in the protobuf as a Buffer, in JSONSchema it is a hex-encoded buffer, but the decoding logic is trying to stringify the Buffer. This will lead to corrupt data when editing observations.

The "raw" format of migrated observations is JSON (this is what is stored in the MLEF), so I'm not sure where we would get the buffer from in the first place.

Rather than fix this, which would take a while, I want to revert this quickly so that we don't introduce these breaking changes in any build of comapeo-core.

I wonder whether we should store the original doc as JSON / struct on the protobuf. Also, I wonder whether for the case of migrated observation, we should not store it for edits, e.g. when links: [] is not an empty array.

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