Skip to content

feat(internal/sidekick/rust): initialize bidi streaming RPCs eagerly - #7221

Merged
suzmue merged 2 commits into
googleapis:mainfrom
suzmue:eager-initialization
Aug 7, 2026
Merged

feat(internal/sidekick/rust): initialize bidi streaming RPCs eagerly#7221
suzmue merged 2 commits into
googleapis:mainfrom
suzmue:eager-initialization

Conversation

@suzmue

@suzmue suzmue commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Initialize bidirectional streaming RPCs eagerly on send instead of deferring transport dialing until the first message is written to the channel in a background task.

Stream builders now support field setters and with_request to configure the initial request message, and send initiates the stream immediately, surfacing connection errors to the caller.

This change is intended to be the minimal change needed to move the initialization logic and does not address other TODOs remaining in the transport implementation. This includes configuration for the channel values, which may mean we will not use RequestBuilder directly in the final implementation.

For #6835

Initialize bidirectional streaming RPCs eagerly on send instead of
deferring transport dialing until the first message is written to the
channel in a background task.

Stream builders now support field setters and with_request to configure
the initial request message, and send initiates the stream immediately,
surfacing connection errors to the caller.
@suzmue
suzmue requested a review from a team as a code owner August 6, 2026 22:05

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request refactors the Rust template code for bidirectional streaming RPCs. It renames the builder's build method to send, which now accepts an initial request and returns a Result. The stub, tracing, and transport layers are updated to propagate this request and return a Result. In the transport layer, the connection setup is now awaited directly, allowing stream initiation errors to be returned immediately to the caller. I have no feedback to provide as there are no review comments.

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

In Swift we started writing unit tests to verify the mustache template output matches expectations.... For example:

got := extractBlock(t, string(contentsB), "/// Initialize from an integer value.", "\n }")
want := `/// Initialize from an integer value.
///
/// If the value is unknown, this initializes to ` + "[`unknownIntValue`](doc:Kind/unknownIntValue(_:))." + `
public init(intValue: Int) {
switch intValue {
case 0: self = .test
case 1: self = .otherTest
default: self = .unknownIntValue(intValue)
}
}`
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("mismatch (-want +got):\n%s", diff)
}

I think these are less annoying than golden file tests, and will avoid somebody accidentally changing the template to return the wrong thing.

The downside is that they trigger errors if you change spaces or indentation in the mustache templates.

Consider starting a few such tests with this PR.

…tion

Add table-driven unit tests for Rust bidirectional streaming code
generation, verifying the mustache template output across builder,
stub, dynamic stub, tracing, and transport files.

For googleapis#6835
@suzmue
suzmue enabled auto-merge (squash) August 7, 2026 16:54
@suzmue
suzmue merged commit cbfa368 into googleapis:main Aug 7, 2026
40 checks passed
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.

2 participants