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

Resolve drift in Netty http/2 apis. #4

Closed
wants to merge 1 commit into from
Closed

Resolve drift in Netty http/2 apis. #4

wants to merge 1 commit into from

Conversation

codefromthecrypt
Copy link
Contributor

Fixes api drift, addressing inbound flow control.

@ejona86
Copy link
Member

ejona86 commented Dec 18, 2014

If you could also update the lib/netty submodule to the commit you are basing this work, that would be helpful (either HEAD or Nathan's commit).

We are definitely interested in inbound flow control, but it also seems like you ported over our usage of it. Did you mean outbound flow control? If so, then the answer is "yes, but not yet."

We haven't yet figured out the method name/API for exposing outbound flow control to applications in the Call/ServerCall class. We actually had an idea of how we wanted it to look, but couldn't quite decide on a name. There is now a conversation of changing flow control to look more like reactive-streams, which would also impact the interface.

We will certainly want to provide outbound flow control feedback to the application, but we don't yet know concretely what that will look like.

@codefromthecrypt
Copy link
Contributor Author

ok updated to HEAD and fixed tests

@nmittler
Copy link
Member

@ejona86 should we cherrypick, or are we waiting on something?

@ejona86
Copy link
Member

ejona86 commented Jan 16, 2015

We can cherrypick.

@nmittler
Copy link
Member

@adriancole would you mind rebasing against the latest head? I'll be happy to cherry-pick when you're ready. Thanks!

@codefromthecrypt
Copy link
Contributor Author

ok, but don't merge yet. This will fail tests until netty/netty#3348 is in and my commit resets the netty sha accordingly.

@nmittler
Copy link
Member

@adriancole sgtm, thanks!

@codefromthecrypt
Copy link
Contributor Author

okie ready to go.

@nmittler
Copy link
Member

Cherry-picked as c4a43e6

Thanks @adriancole! :)

@nmittler nmittler closed this Jan 23, 2015
songy23 pushed a commit to songy23/grpc-java that referenced this pull request Oct 31, 2017
Update grpc core to use new OpenCensus API.
@lock lock bot locked as resolved and limited conversation to collaborators Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants