-
Notifications
You must be signed in to change notification settings - Fork 527
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
Define a new remote.DefaultTransport
.
#1165
Conversation
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
@googlebot I fixed it. |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
@googlebot I fixed it. |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
@googlebot I fixed it. |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
31bbe05
to
4c81250
Compare
Switched to the old email 🙃 |
4c81250
to
13cab84
Compare
Fixed the gcrane test that assumed |
Codecov Report
@@ Coverage Diff @@
## main #1165 +/- ##
==========================================
- Coverage 75.24% 75.23% -0.01%
==========================================
Files 108 108
Lines 7857 7855 -2
==========================================
- Hits 5912 5910 -2
Misses 1379 1379
Partials 566 566
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks okay to me. I'm not super in love with making crane users think about the dial timeout flag, but I guess it's okay to let them get back to previous behavior if they want it.
Friendly ping. If we do a release soon do we feel like we need to get this included in it? |
This reverts commit 080751a.
Rebasing now, then I'll add a commit for the |
Previously we used `http.DefaultTransport` (on which this is based), but this uses a default dial timeout of 30s, which when we (by default) wrap things in 5x retries can lead to ~150s delays simply pinging an http-based registry. If folks encounter issues with this via the library they can restore the current behavior with: ```go remote.WithTransport(http.DefaultTransport) ``` If folks encounter issues with this via `crane` they can restore the current behavior with: ``` --dial-timeout 30s ```
13cab84
to
7df8286
Compare
This implements a "happy eyeballs" (RFC 6555) style of race in order to speed up http fallback during registry pings. This heavily borrows code from net/dial.go which implements this same kind of race for dual stack DNS. Rather than waiting for the initial "https" ping to time out completely before attempting to use "http", we will instead start a second fallback ping after 300ms and return whichever response comes back first. This partially reverts the portion of google#1165 that reduced the dial timeout to workaround this issue.
This implements a "happy eyeballs" (RFC 6555) style of race in order to speed up http fallback during registry pings. This heavily borrows code from net/dial.go which implements this same kind of race for dual stack DNS. Rather than waiting for the initial "https" ping to time out completely before attempting to use "http", we will instead start a second fallback ping after 300ms and return whichever response comes back first. This partially reverts the portion of google#1165 that reduced the dial timeout to workaround this issue.
This implements a "happy eyeballs" (RFC 6555) style of race in order to speed up http fallback during registry pings. This heavily borrows code from net/dial.go which implements this same kind of race for dual stack DNS. Rather than waiting for the initial "https" ping to time out completely before attempting to use "http", we will instead start a second fallback ping after 300ms and return whichever response comes back first. This partially reverts the portion of google#1165 that reduced the dial timeout to workaround this issue. Also, drop unused parseChallenge to appease the linter.
This implements a "happy eyeballs" (RFC 6555) style of race in order to speed up http fallback during registry pings. This heavily borrows code from net/dial.go which implements this same kind of race for dual stack DNS. Rather than waiting for the initial "https" ping to time out completely before attempting to use "http", we will instead start a second fallback ping after 300ms and return whichever response comes back first. This partially reverts the portion of #1165 that reduced the dial timeout to workaround this issue. Also, drop unused parseChallenge to appease the linter.
The first commit here reverts #1163 which went in before I could change it to the transport-based option.
Previously we used
http.DefaultTransport
(on which this is based),but this uses a default dial timeout of 30s, which when we (by default)
wrap things in 5x retries can lead to ~150s delays simply pinging
an http-based registry.
If folks encounter issues with this via the library they can restore
the current behavior with:
If folks encounter issues with this via
crane
they can restore thecurrent behavior with: