-
Notifications
You must be signed in to change notification settings - Fork 159
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
Create many tiny artifacts for faster download #2951
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2951 +/- ##
=========================================
Coverage 72.21% 72.21%
Complexity 2529 2529
=========================================
Files 136 136
Lines 14452 14452
Branches 991 991
=========================================
Hits 10436 10436
Misses 3472 3472
Partials 544 544
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
177c467
to
3a1188d
Compare
And not escaping the constraints many systems have on /tmp size. If someone installs multiple php versions at once, we still download the full package. The full package also remains available for offline installations of multiple targets for now. Signed-off-by: Bob Weinand <[email protected]>
3a1188d
to
37efa3b
Compare
datadog-setup.php
Outdated
@@ -1444,12 +1492,18 @@ function download($url, $destination) | |||
curl_setopt($ch, CURLOPT_NOPROGRESS, false); | |||
$progress_counter = 0; | |||
$return = curl_exec($ch); | |||
|
|||
$httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE); | |||
if ($httpCode == 404) { |
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 weird to me.
IIUC, at the first download attempt (for a specific version), if curl
is installed, we are going to try only with curl.
Then at the second attempt (full package), this could create an infinite loop if github (or a proxy or whatever) returns a 404.
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.
You're right, I messed that up.
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.
Wrapped in if($retry), looks good now?
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.
nit: IMHO, download
should always return a boolean, and the retry or exit logic handled by the caller.
And not escaping the constraints many systems have on /tmp size. If someone installs multiple php versions at once, we still download the full package.
The full package also remains available for offline installations of multiple targets for now.