Skip to content
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

In CurlDownloader, use the global timeout (instead of a hardcoded 5-minute timeout) #10018

Closed
wants to merge 1 commit into from
Closed

Conversation

smokris
Copy link

@smokris smokris commented Jul 22, 2021

CurlDownloader is hardcoded to a 5-minute timeout. When downloading a large file on a slow internet connection, it hits this timeout, then aborts and restarts the download from the beginning, and composer install is unable to make any progress.

This PR modifies CurlDownloader to use the global ProcessExecutor timeout (COMPOSER_PROCESS_TIMEOUT) instead. (COMPOSER_PROCESS_TIMEOUT defaults to 5 minutes, just like the hardcoded value in CurlDownloader, so the default behavior remains the same when that environment variable is not set.) With this PR, when I set COMPOSER_PROCESS_TIMEOUT to an appropriate value for my internet connection, composer install is able to complete successfully.

@Seldaek Seldaek closed this in 1f44010 Jul 23, 2021
@Seldaek
Copy link
Member

Seldaek commented Jul 23, 2021

It's a valid point that this shouldn't be hardcoded, but IMO the process timeout isn't the way to change this. The default_socket_timeout ini setting is the closest thing we have to having this configurable in the past, so it now respects this too for curl transfers. I hope this lets you resolve your issue too.

@Seldaek Seldaek added this to the 2.1 milestone Jul 23, 2021
@smokris smokris deleted the download-timeout branch July 23, 2021 15:55
@smokris
Copy link
Author

smokris commented Jul 23, 2021

Ah, that makes sense. With your commit, I confirmed that I'm able to increase the timeout by setting default_socket_timeout=3600 in my php.ini, which enables me to download large files here. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants