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

ManagedChannel getState/notifyChanged API docs #3762

Closed
dcow opened this issue Nov 16, 2017 · 5 comments
Closed

ManagedChannel getState/notifyChanged API docs #3762

dcow opened this issue Nov 16, 2017 · 5 comments
Milestone

Comments

@dcow
Copy link
Contributor

dcow commented Nov 16, 2017

I'm using gRPC 1.7 (on Android) and notice the docs for ManagedChannel still say this functionality is unimplemented (and the base implementation throws as it should). However, the MannagedChannelImpl (which OkHttpChannelImpleBuilder uses) has this implemented. Is this an oversight or on purpose? It caused me to dive into the issues (culminating in #2292) to figure out if this was actually supported yet (I've determined it is). Should the docs be updated to something a little more helpful?

@carl-mastrangelo
Copy link
Contributor

We don't own all implementations of ManagedChannel, so we cannot be certain all implementations do. The only known implementation, ManagedChannelImpl, does implement it. Using ManagedChannelBuilders written by the gRPC team will implement these methods.

@dcow
Copy link
Contributor Author

dcow commented Nov 16, 2017

Understood, but the docs still say it's not implemented and have links to closed issues ( #28 and #2292 ).

* <p><strong>Warning</strong>: this API is not yet implemented by the gRPC library (<a
* href="https://github.com/grpc/grpc-java/issues/2292" target="_blank">issue on github</a>).

That's the main thing that threw me for a loop. Maybe a forward reference to ManagedChannelImpl or some prose similar your comment explaining the situation would be useful. I'm happy to contribute, but want to make sure I wasn't misunderstanding the situation.

@carl-mastrangelo
Copy link
Contributor

@dapengzhang0 Do you know if the Channel state APIs are complete an implemented everywhere? Can the warning be removed?

@dapengzhang0
Copy link
Member

The warning should be removed since the API is implemented in the main impl of ManacheChannel in the gRPC library.

dcow pushed a commit to dcow/grpc-java that referenced this issue Nov 17, 2017
ManagedChannel now supports the getState/notifyWhenStateChanged API (grpc#2292).
ejona86 pushed a commit that referenced this issue Nov 17, 2017
ManagedChannel now supports the getState/notifyWhenStateChanged API (#2292).
ejona86 pushed a commit to ejona86/grpc-java that referenced this issue Nov 17, 2017
ManagedChannel now supports the getState/notifyWhenStateChanged API (grpc#2292).
@ejona86
Copy link
Member

ejona86 commented Nov 17, 2017

Fixed by #3768

@ejona86 ejona86 closed this as completed Nov 17, 2017
@ejona86 ejona86 added this to the 1.8 milestone Nov 17, 2017
@ejona86 ejona86 removed the question label Nov 17, 2017
ejona86 pushed a commit that referenced this issue Nov 17, 2017
ManagedChannel now supports the getState/notifyWhenStateChanged API (#2292).
@lock lock bot locked as resolved and limited conversation to collaborators Sep 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants