Conversation
grogancolin
force-pushed
the
grogancolin/#1167
branch
from
3773fad to
e292998
Compare
|
See also: #1190 though not conflicting with this PR. |
source/dub/packagesupplier.d
Outdated
| foreach(i; 0..3) { | ||
| try{ | ||
| download(url, path); | ||
| break; |
source/dub/packagesupplier.d
Outdated
| logDebug("Failed to download package %s from %s (Attempt %s of 3)", packageId, url, i + 1); | ||
| continue; | ||
| } | ||
| throw new Exception("Failed to download package %s from %s".format(packageId, url)); |
There was a problem hiding this comment.
This needs to be moved outside of the loop.
source/dub/packagesupplier.d
Outdated
| break; | ||
| } | ||
| catch(HTTPStatusException e) { | ||
| logDebug("Failed to download package %s from %s (Attempt %s of 3)", packageId, url, i + 1); |
There was a problem hiding this comment.
There should be a check for 404 errors here, or maybe even limit the retry behavior to 500 errors and connection failures. Retrying for 404 errors would mean that package download in scenarios with multiple registries (that contain different sets of packages) potentially gets slowed down considerably.
grogancolin
force-pushed
the
grogancolin/#1167
branch
from
e292998 to
f5ed558
Compare
Packagesupplier.d now tries 3 times to download a package and metadata. This should aid travis to not be as flaky as it is currently. Implements enhancement request dlang#1167
s-ludwig
left a comment
s-ludwig
left a comment
There was a problem hiding this comment.
We should probably refactor this, so that the retry-logic is contained in the download function, but that doesn't have to be done as part of the PR.
source/dub/packagesupplier.d
Outdated
| catch(HTTPStatusException e) { | ||
| if (e.status == 404) { | ||
| logDebug("Failed to download package %s from %s (404)", packageId, url); | ||
| return; |
There was a problem hiding this comment.
I'm not sure how the outer code will behave if nothing downloaded, but to be consistent with the previous behavior, this would also have to throw instead of returning.
|
Thanks for your pull request, @grogancolin! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Some tips to help speed things up:
Bear in mind that large or tricky changes may require multiple rounds of review and revision. Please see CONTRIBUTING.md for more information. |
s-ludwig
left a comment
s-ludwig
left a comment
There was a problem hiding this comment.
I've added the throw for the 404 case.
|
Thanks! |
|
@s-ludwig maybe it's time to drop support for accient |
|
Looks more like a random failure - let's have a look whether restarting works |
|
AFAIR, 2.068 is the frontend that the latest released version of GDC uses, so I think we should keep it for a while. But it's indeed strange that it fails for this PR, I already restarted it, but it still failed. |
5 participants
Try 3 times to download a package/metadata
Packagesupplier.d now tries 3 times to download a package and metadata.
This should aid travis to not be as flaky as it is currently.
Implements enhancement request #1167