-
Notifications
You must be signed in to change notification settings - Fork 10.6k
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
C# Improving Channel API #2756
Conversation
Hm, looks flaky. |
In tests, for channels where you set the channel argument |
{ | ||
using (var channel = new Channel("127.0.0.1", Credentials.Insecure)) | ||
{ | ||
Assert.IsTrue(channel.Target.Contains("127.0.0.1")); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
34c612d
to
541768e
Compare
Why is grpc.default_authority important? Looks like none of gRPC implementations is setting it. |
MIchael is OOO, I will discuss the grpc.default_authority with him separately. Please review the rest of the code. |
dfea902
to
9fb1d20
Compare
Didn't see anything that jumps out to me. LGTM. |
-- 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