Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-39335: [C#] Support creating FlightClient with Grpc.Core.Channel #39348

Merged
merged 1 commit into from
Dec 22, 2023
Merged

GH-39335: [C#] Support creating FlightClient with Grpc.Core.Channel #39348

merged 1 commit into from
Dec 22, 2023

Conversation

johnnyg
Copy link
Contributor

@johnnyg johnnyg commented Dec 22, 2023

as well as Grpc.Net.Client.GrpcChannel by changing our constructor arg to Grpc.Core.ChannelBase which both classes inherit from.

Rationale for this change

What changes are included in this PR?

Changing the constructor of C#'s Flight Client to take in a ChannelBase which allows for multiple implementations of gRPC channels to be passed in.

Are these changes tested?

Existing tests already cover the use but have also manually tested in a separate app (

Are there any user-facing changes?

No as we're just changing the constructor to take in a parent/base class instead.

as well as Grpc.Net.Client.GrpcChannel by changing our constructor arg
to Grpc.Core.ChannelBase which both classes inherit from.
Copy link

⚠️ GitHub issue #39335 has been automatically assigned in GitHub to PR creator.

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Dec 22, 2023
@CurtHagenlocher CurtHagenlocher merged commit 87971df into apache:main Dec 22, 2023
9 checks passed
@CurtHagenlocher CurtHagenlocher removed the awaiting committer review Awaiting committer review label Dec 22, 2023
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 87971df.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

@johnnyg johnnyg deleted the gh39335 branch January 18, 2024 01:58
clayburn pushed a commit to clayburn/arrow that referenced this pull request Jan 23, 2024
…nnel (apache#39348)

as well as Grpc.Net.Client.GrpcChannel by changing our constructor arg to Grpc.Core.ChannelBase which both classes inherit from.

### Rationale for this change

### What changes are included in this PR?

Changing the constructor of C#'s Flight Client to take in a ChannelBase which allows for multiple implementations of gRPC channels to be passed in.

### Are these changes tested?

Existing tests already cover the use but have also manually tested in a separate app (

### Are there any user-facing changes?

No as we're just changing the constructor to take in a parent/base class instead.
* Closes: apache#39335

Authored-by: John Garland <johnnybg@gmail.com>
Signed-off-by: Curt Hagenlocher <curt@hagenlocher.org>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…nnel (apache#39348)

as well as Grpc.Net.Client.GrpcChannel by changing our constructor arg to Grpc.Core.ChannelBase which both classes inherit from.

### Rationale for this change

### What changes are included in this PR?

Changing the constructor of C#'s Flight Client to take in a ChannelBase which allows for multiple implementations of gRPC channels to be passed in.

### Are these changes tested?

Existing tests already cover the use but have also manually tested in a separate app (

### Are there any user-facing changes?

No as we're just changing the constructor to take in a parent/base class instead.
* Closes: apache#39335

Authored-by: John Garland <johnnybg@gmail.com>
Signed-off-by: Curt Hagenlocher <curt@hagenlocher.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[C#] Support creating FlightClient with Grpc.Core.Channel
2 participants