applying backoff for rate-limited Maven downloads #100
Conversation
…aven Jf/applying backoff for maven
|
👍 |
…aven incrementing version number
…aven Applying changes to master
ghost
mentioned this pull request
| try: | ||
| return urlopen(url) | ||
| except HTTPError as e: | ||
| if e.code == 429: |
There was a problem hiding this comment.
@jtfalkenstein Can we change this to perform retries also for other error codes?
It seems that Maven is not consistently using 429, but also other error codes. Recently, we've seen 502 (or 504) errors returned from the gateway, but these errors appear to be related to client request throttling.
Including other error codes here would make the installation much more resilient. Thanks
There was a problem hiding this comment.
Weird. You know, we've been running this install via an automated Jenkins build a lot and haven't encountered anything other than a 429.
I'm inclined to do as you request on this, but I do have a concern on this... Any time we repeatedly pound a server with requests that we're getting error responses for, we risk being blacklisted by the server admins.
There was a problem hiding this comment.
Fair point. Good to know that you haven't had any issues recently with the 429 fix. Merging this PR as-is would already be a huge improvement over the status quo. :) Thanks
|
@pfifer, what are the chances of this thing getting merged? |
|
Hi! Any progress on PR? |
|
I just want to note that this code had been working flawlessly for us for almost a year of daily use. |
|
Bumping this PR one more time @pfifer @rk-yen @jtfalkenstein
Same here - this fix has been working flawlessly for us for a long period of time now. Any chance we can get this merged, so we don't have to depend on our own fork of this library? Thanks! |
|
+1 on this fix |
| @@ -50,7 +54,7 @@ | |||
|
|
|||
| PACKAGE_NAME = 'amazon_kclpy' | |||
| JAR_DIRECTORY = os.path.join(PACKAGE_NAME, 'jars') | |||
| PACKAGE_VERSION = '2.0.1' | |||
| PACKAGE_VERSION = '2.0.5' | |||
There was a problem hiding this comment.
Since the current version is 2.0.1, should this be changed to 2.0.2?
| @@ -17,16 +17,20 @@ | |||
|
|
|||
| import os | |||
| import shutil | |||
| from time import sleep | |||
There was a problem hiding this comment.
It looks like they have the imports organized a bit differently. Perhaps keep "import x" at the top and move the "from x import y" in the next section...all alphabetically, of course.
|
Who do we need to tag to get this merged? |
ghost
mentioned this pull request
|
I'm gong to close this PR in favor of #141, since I cannot update the repository any longer now that I'm not at that company. On that new pull request, I've fixed the above-mentioned version change and am more consistent about imports. |
ghost
closed this
Issue #, if available: #99
Description of changes:
When a 429 response is received when attempting to download one of the jars, this will apply exponential backoff, sleeping anywhere between 1 second (on the first retry) to 16 (on the final retry).
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.