Skip to content

update watch api: include schema changes#133

Merged
miparnisari merged 3 commits into
mainfrom
update-watch-api-include-schema-changes
Mar 18, 2025
Merged

update watch api: include schema changes#133
miparnisari merged 3 commits into
mainfrom
update-watch-api-include-schema-changes

Conversation

@miparnisari

Copy link
Copy Markdown
Contributor
@miparnisari miparnisari force-pushed the update-watch-api-include-schema-changes branch from f39a93f to d90061c Compare March 17, 2025 23:36
@miparnisari miparnisari force-pushed the update-watch-api-include-schema-changes branch 2 times, most recently from 180b035 to e52b636 Compare March 18, 2025 18:59
@miparnisari miparnisari force-pushed the update-watch-api-include-schema-changes branch from e52b636 to 70284c8 Compare March 18, 2025 19:07
@miparnisari miparnisari marked this pull request as ready for review March 18, 2025 19:13

@tstirrat15 tstirrat15 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.

LGTM

@josephschorr josephschorr 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

@miparnisari miparnisari merged commit 3fb638d into main Mar 18, 2025
@miparnisari miparnisari deleted the update-watch-api-include-schema-changes branch March 18, 2025 19:50
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 18, 2025
// last watch response.
repeated ReflectionSchemaDiff schema_updates = 4;

// is_checkpoint, if true, indicates that a revision was reached.

@vroldanbet vroldanbet Mar 19, 2025

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.

@miparnisari I think we should clarify this: "Reaching a revision" does not tell the client what guarantees they get from a checkpoint.

I would frame it as "a checkpoint indicates the server guarantees the client will not observe any changes at a revision below or equal to the revision in this response"

What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! Added to #135

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

Labels

None yet

4 participants