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

Support HTTP2 #233

Merged
merged 1 commit into from
May 13, 2020
Merged

Support HTTP2 #233

merged 1 commit into from
May 13, 2020

Conversation

roidelapluie
Copy link
Member

According to my readings, as we provide a custom transport, we have to
configure it to use HTTP2.

golang/go#20645

I have tested the behaviour of this and it is correct.

Signed-off-by: Julien Pivotto roidelapluie@inuits.eu

@brian-brazil
Copy link
Contributor

Maybe it's just ForceAttemptHTTP2 we need to set?

@roidelapluie
Copy link
Member Author

https://github.com/golang/net/blob/master/http2/transport.go#L136

it is doing way more than that, and errors if it is already configured

@brian-brazil
Copy link
Contributor

Are you sure they're doing something different? That'd seem like a bug to me.

@roidelapluie
Copy link
Member Author

I will try this out, indeed this more recent than what I have read

@roidelapluie
Copy link
Member Author

Breaks go < 1.13

Copy link
Contributor

@brian-brazil brian-brazil left a comment

Choose a reason for hiding this comment

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

👍

@@ -153,6 +154,12 @@ func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAli
conntrack.DialWithName(name),
),
}
// TODO: use ForceAttemptHTTP2 when we move to go 1.13+
Copy link
Contributor

Choose a reason for hiding this comment

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

Go

And don't forget your full stop.

According to my readings, as we provide a custom transport, we have to
configure it to use HTTP2.

golang/go#20645

I have tested the behaviour of this and it is correct.

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants