Skip to content
This repository was archived by the owner on Mar 10, 2025. It is now read-only.

Conversation

@nathanielc
Copy link
Contributor

With this change it is now possible to update a model definition in backwards compatible ways. Additionally when creating or updating model instance documents a modelVersion header value can be supplied to indicate that the instance uses the updated version of the model. Instances will always use the init event of a model (i.e. its first version) unless explicitly changed to a new version using the modelVersion header.

With this change it is now possible to update a model definition in
backwards compatible ways. Additionally when creating or updating model
instance documents a modelVersion header value can be supplied to
indicate that the instance uses the updated version of the model.
Instances will always use the init event of a model (i.e. its first
version) unless explicitly changed to a new version using the
modelVersion header.
// CBOR encoding doesn't support undefined values
const header =
params.shouldIndex == null ? undefined : { shouldIndex: params.shouldIndex }
const header: DocumentDataEventHeader = {}

Choose a reason for hiding this comment

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

Does it matter that we were sending an undefined header before if shouldIndex == null? We'll always pass the header into the function now, and it might be an empty map.

Choose a reason for hiding this comment

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

Or should we just update createDataEventPayload to ignore both an undefined header and an empty one?

Choose a reason for hiding this comment

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

Or does it not matter if header is empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I am 90% sure it doesn't matter but better safe than sorry. I'll change this to not set a header if shouldIndex or modelVersion are not set.

* @returns The `CommitID` for the stream.
*/
getCurrentID(streamID: string): CommitID {
return new CommitID(2, streamID)

Choose a reason for hiding this comment

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

What does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

constructs a CommitID from just a stream id. Therefore the commit referenced will be the init commit

Copy link

@smrz2001 smrz2001 left a comment

Choose a reason for hiding this comment

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

:shipit:

@nathanielc nathanielc merged commit 7a6929a into main Feb 27, 2025
10 checks passed
@nathanielc nathanielc deleted the feat/model-updates branch February 27, 2025 20:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants