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

[#4819] fixed failing to push some files to http remote #4820

Merged
merged 1 commit into from
Nov 1, 2020

Conversation

simonlsk
Copy link
Contributor

@simonlsk simonlsk commented Nov 1, 2020

  • ❗ I have followed the Contributing to DVC checklist.

  • πŸ“– If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.

  • Increased the HTTPTree.REQUEST_TIMEOUT from 10 to 60 seconds, as it was not changed after implementing upload for http.

Will fix #4819

@@ -33,7 +33,7 @@ class HTTPTree(BaseTree): # pylint:disable=abstract-method

SESSION_RETRIES = 5
SESSION_BACKOFF_FACTOR = 0.1
REQUEST_TIMEOUT = 10
REQUEST_TIMEOUT = 60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might also be not enough if the file is big enough and/or the connection is slow enough. What is the biggest file size that you are uploading? We are having a similar discussion about webdav where apparently chunked upload is not always possible and so that makes #829 more relevant. Maybe it does here too.

Copy link
Contributor Author

@simonlsk simonlsk Nov 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chunk size has never caused a problem for a single large file in my case. Only having too many concurrent files uploading would make some of them time out.
In general, with a slow enough connection, or too many concurrent jobs, some file will eventually fail to upload.
This also relates to other issues discussing the implementation of retries.

Copy link
Contributor

@efiop efiop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! Looks good! Please see one comment above.

For the record: windows failures are unrelated.

@efiop efiop merged commit 59d5c73 into iterative:master Nov 1, 2020
@simonlsk simonlsk deleted the http-timeout branch November 1, 2020 21:12
@skshetry skshetry added the bugfix fixes bug label Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix fixes bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pushing multiple files to http remote fails
3 participants