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

transport: simplify httpClient by moving onGoAway func to onClose #5885

Merged
merged 1 commit into from
Dec 21, 2022

Conversation

arvindbr8
Copy link
Member

@arvindbr8 arvindbr8 commented Dec 21, 2022

This diff simplifies the transport code a little by removing onGoAway func and moving the logic into onClose. Updating tests to remove the onGoAway param

RELEASE NOTES: none

@arvindbr8 arvindbr8 added the Type: Internal Cleanup Refactors, etc label Dec 21, 2022
@arvindbr8 arvindbr8 added this to the 1.53 Release milestone Dec 21, 2022
@arvindbr8 arvindbr8 merged commit 07ac97c into grpc:master Dec 21, 2022
@arvindbr8 arvindbr8 deleted the removeOnGoAway branch December 21, 2022 21:44
easwars pushed a commit to easwars/grpc-go that referenced this pull request Dec 21, 2022
@@ -1290,8 +1291,10 @@ func (t *http2Client) handleGoAway(f *http2.GoAwayFrame) {
// Notify the clientconn about the GOAWAY before we set the state to
// draining, to allow the client to stop attempting to create streams
// before disallowing new streams on this connection.
t.onGoAway(t.goAwayReason)
t.state = draining
if t.state != draining {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a behavior change that needs to be release noted?

Copy link
Member

Choose a reason for hiding this comment

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

There should be no user-visible behavior change here, because of the corresponding changes in grpc's usage of the transport.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants