feat/river schema v2 compatibility#106
Merged
blast-hardcheese merged 193 commits intomainfrom Apr 8, 2025
Merged
Conversation
a59a4df to
dadffb8
Compare
masad-frost
approved these changes
Nov 6, 2024
Member
masad-frost
left a comment
There was a problem hiding this comment.
Do you wanna set up a different base branch in case we need to send a patch against main?
dadffb8 to
4c0e071
Compare
Contributor
Author
If we need to patch, we should do feature patches off previous release tags. Since we're reserving 1.x for GA, I can follow the 0.200.0 strategy in this repo for schema-v2 releases. Doing that though would suggest that we do at least a halfway decent pass at the v2 server codegen as well though. Given what we discussed supporting v1/v2 using a naive shim doesn't seem too bad, probably can get that done this week. Wdyt? |
b71a809 to
717cc68
Compare
aa49136 to
747f7e1
Compare
cbrewster
reviewed
Mar 26, 2025
jackyzha0
reviewed
Mar 27, 2025
masad-frost
reviewed
Mar 27, 2025
Discovered that it was overloaded, written to by multiple different sources with different semantics.
bb52361 to
7123e82
Compare
8589484 to
ffc10ce
Compare
ffc10ce to
4820ef8
Compare
Contributor
Author
|
Five remaining flakes in babel: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
River Schema v2 compatibility so we can start getting rid of the compatibility layer.
What changed
Client
Significantly restructured the
river.v2.*.asyncio.Lock's have been removedsend_messagewhen called concurrently, so we'd end up withseqbound and incremented but then messages would become out-of-order by the time thews.sendmethod returned. Now we just atomically append to adequeas we push onto the send buffer, and when we get a confirmation from the ws library we move the message over to an ack buffer (also adeque) waiting for incoming messages from the server to allow us to drain.Codegen
initis now required for all method types,inputis now optional. This simplifiesrpcandstreamcodegen.initwas being used ininputposition and vice versa.f"'{foo}'"encoding forf"{repr(foo)}", giving greater safety post-generation (to avoid situations wheref"'{None}'"gets rendered as"None".f"{repr(None)}"would render asNonewhich would fail typechecks.)Test plan
Manually ran codegen against v2 generated schemas, everything typechecks.