Skip to content

http2: add configuration options when constructing http2 transport #74

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

Closed

Conversation

caesarxuchao
Copy link
Contributor

@caesarxuchao caesarxuchao commented Jun 4, 2020

This allows the caller to change some configurations of the http2 transport when constructing it from an http1 transport.

Fixes golang/go#40201.

@gopherbot
Copy link
Contributor

This PR (HEAD: a242348) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/236498 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Chao Xu:

Patch Set 1:

This directly solves https://go-review.googlesource.com/c/net/+/198040/18#message-d1a7ce662534f798bb6b6702d6af575bebf41e45.

I'm happy to open an issue in golang/go if that's preferred.


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Russ Cox:

Patch Set 1:

We're closing in on the Go 1.15 beta.
It's probably too late to be adding new API.
Why would you need to change these parameters?


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Chao Xu:

Patch Set 1:

Patch Set 1:

We're closing in on the Go 1.15 beta.
It's probably too late to be adding new API.
Why would you need to change these parameters?

In order to put the health check I added in https://go-review.googlesource.com/c/net/+/198040 in use. The feature by default is off.

Without this change, it's very difficult to change the ReadIdleTimeout and PingTimeout. One would need to use the unsafe package like what I did in the test.


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@caesarxuchao caesarxuchao force-pushed the http2-configure-options branch from a242348 to d7da2b4 Compare July 29, 2020 18:32
@gopherbot
Copy link
Contributor

This PR (HEAD: d7da2b4) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/236498 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Chao Xu:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Michael Fraenkel:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Chao Xu:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Chao Xu:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Michael Fraenkel:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@caesarxuchao caesarxuchao force-pushed the http2-configure-options branch from d7da2b4 to 6619d1f Compare September 23, 2020 05:17
@gopherbot
Copy link
Contributor

This PR (HEAD: 6619d1f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/236498 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

This allows the caller to change some configurations of the http2
transport when constructing it from an http1 transport.
@caesarxuchao caesarxuchao force-pushed the http2-configure-options branch from 6619d1f to 1d2c348 Compare September 23, 2020 05:26
@gopherbot
Copy link
Contributor

This PR (HEAD: 1d2c348) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/236498 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Chao Xu:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 6: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 6:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Brad Fitzpatrick:

Patch Set 6: Code-Review-2

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Chao Xu:

Patch Set 6:

(3 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 6:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Chao Xu:

Patch Set 6:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Chao Xu:

Patch Set 6:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/236498.
After addressing review feedback, remember to publish your drafts!

@caesarxuchao
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x/net/http2: ReadIdleTimeout/PingTimeout not exposed if configured through http package
4 participants