-
Notifications
You must be signed in to change notification settings - Fork 591
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
fix: increase the default for proxy requests #1610
Conversation
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.
Can we use some retry library instead of implementing retries ourselves? The second best thing to do is unit test the retry mechanism implemented in this PR but I think it would not be good use of our time.
4562ade
to
6a3c65c
Compare
Given that there's any contention I've opted to remove the retry mechanism from this PR in favor of focusing on the better default so we can get this fix quicker. |
Codecov Report
@@ Coverage Diff @@
## next #1610 +/- ##
==========================================
+ Coverage 62.35% 62.56% +0.20%
==========================================
Files 82 82
Lines 7180 7180
==========================================
+ Hits 4477 4492 +15
+ Misses 2372 2361 -11
+ Partials 331 327 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
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.
lgtm.
What this PR does / why we need it:
Increases the default proxy timeout based on experiences in GKE (which was failing).
Which issue this PR fixes
Adds fixes that supports #1095
Supports the analysis found in #1519
PR Readiness Checklist:
CHANGELOG.md
release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR