Skip to content

Implement Channel builder#2675

Open
nathanielford wants to merge 14 commits into
grpc:masterfrom
nathanielford:implement/channel-builder
Open

Implement Channel builder#2675
nathanielford wants to merge 14 commits into
grpc:masterfrom
nathanielford:implement/channel-builder

Conversation

@nathanielford

Copy link
Copy Markdown
Contributor

Introduces a type-builder Channel builder for creating new Channels, using an idiomatic pattern for adding in configuration options.

Motivation

In order to flexibly and ergonomically provide for channel configuration on construction, the type builder is introduced. This allows users to construct channels in a manner similar to:

let channel = Channel::builder("https://api.darkcave.com:443")
    .credential(my_creds)
    .runtime(my_runtime)
    .override_authority("arrow.wumpus.api")
    // ... any other options.
    .build();  // No error is returned here

Solution

The solution uses a 'type builder' so that users can build up their channel configuration over various calls, and the compiler can catch type issues. This also allows us to encapsulate various ergonomics (such as default selection of runtime). This PR implements the critical values at this time and defers additional optional values not being used until a later point. It also leaves work around the internals of the PersistentChannel/ActiveChannel for a future PR.

Because the public API is being altered, this requires a version bump.

@nathanielford nathanielford force-pushed the implement/channel-builder branch 3 times, most recently from 1014b27 to 821aa43 Compare June 11, 2026 19:50
@nathanielford nathanielford force-pushed the implement/channel-builder branch from 821aa43 to c0e1055 Compare June 11, 2026 20:01
@nathanielford nathanielford requested a review from arjan-bal June 11, 2026 20:08
@nathanielford nathanielford marked this pull request as ready for review June 11, 2026 20:08
…nChannelCredential impl adjustments should handle the type issue when passing DynChannelCredentials internally
@arjan-bal arjan-bal self-assigned this Jun 17, 2026
runtime: GrpcRuntime,
security_opts: SecurityOpts,
authority: String,
pub struct MissingOpt;

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.

Since users shouldn't need to instantiate this struct, we should add zero-sized private field to prevent this.

pub struct MissingOpt(())

security_opts: SecurityOpts,
authority: String,
pub struct MissingOpt;
pub struct PresentOpt<T>(pub T);

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.

Does the inner T need to be pub here? Since the struct is only instantiated by the gRPC crate, I think pub struct PresentOpt<T>(T) should work.

Comment on lines +186 to +188
// TODO(nford): This field is currently unused. When/if it is needed ensure the type is appropriate.
// - Channels reference SecurityOpts, which hold the value in an Authority object.
// - All other references currently use "ignored" as the authority.

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.

Across languages, there are a few ways to set the channel authority. In order of precedence (highest to lowest), they are:

  1. User-specified authority in the channel options.
  2. The authority from the name resolver builder.
  3. The endpoint from a channel target formatted as scheme://[authority]/endpoint.

I believe our existing implementation already follows this behavior. I'd suggest we keep the authority handling logic as-is in this PR. Let me know if you have questions about the authority overriding.

@arjan-bal

Copy link
Copy Markdown
Contributor

As discussed offline, I've raised #2694 with only the changes to the trait bounds for the credentials. I'll wait for Doug to review it.

@arjan-bal arjan-bal removed their assignment Jun 17, 2026
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