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

C# Improving Channel API #2756

Merged
merged 7 commits into from
Aug 4, 2015
Merged

Conversation

jtattermusch
Copy link
Contributor

-- add channel state API
-- start using default host when calling grpc_channel_create_call
-- add some tests
-- fix some style nits
-- exposes Target property for channel with the right contents.

Fixes #2508
Fixes #2637

@jtattermusch jtattermusch changed the title C# Channel State API C# Improving Channel API Aug 2, 2015
@jtattermusch
Copy link
Contributor Author

Hm, looks flaky.

@murgatroid99
Copy link
Member

In tests, for channels where you set the channel argument grpc.ssl_target_name_override, make sure you also set grpc.default_authority.

{
using (var channel = new Channel("127.0.0.1", Credentials.Insecure))
{
Assert.IsTrue(channel.Target.Contains("127.0.0.1"));
Copy link
Member

Choose a reason for hiding this comment

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

The actual value of the channel target is not guaranteed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems it's working reliably on my machine and in our test environment. Do you still suggest removing?

@jtattermusch
Copy link
Contributor Author

pull request #2779 and #2778 should address the flakiness. Rebased this PR to contain them.

@jtattermusch
Copy link
Contributor Author

Why is grpc.default_authority important? Looks like none of gRPC implementations is setting it.

@jtattermusch
Copy link
Contributor Author

MIchael is OOO, I will discuss the grpc.default_authority with him separately. Please review the rest of the code.

@stanley-cheung
Copy link
Contributor

Didn't see anything that jumps out to me. LGTM.

jtattermusch added a commit that referenced this pull request Aug 4, 2015
@jtattermusch jtattermusch merged commit 8956e60 into grpc:master Aug 4, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use NULL for default host in grpc_channel_call_create() in C# Implement connectivity API for C#
4 participants