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

ncm-download inconsistent use of timeout? #832

Open
stdweird opened this issue Aug 9, 2016 · 4 comments
Open

ncm-download inconsistent use of timeout? #832

stdweird opened this issue Aug 9, 2016 · 4 comments
Labels

Comments

@stdweird
Copy link
Member

stdweird commented Aug 9, 2016

currently ncm-download uses a per file timeout with check for age via HEAD only if proxies exist.
if not proxies exist, a component wide timeout is used and no HEAD checking is performed.
is this intentional?

@stdweird
Copy link
Member Author

stdweird commented Aug 9, 2016

@NZJourneyMan @ajf8 comment?

@jrha
Copy link
Member

jrha commented Aug 9, 2016

I think I may have added (or at least changed) that code... if so, it wasn't intentional.

@ned21
Copy link
Contributor

ned21 commented Aug 9, 2016

@ajf8 may remember more when he's back from holiday but ISTR 4896589 was due to the code taking too long to failover between dead proxies because curl's timeout doesn't work as you might expect and most of the time you want head_timeout not timeout. It's possible that for consistency the same fix should be moved to the non-proxy code path, but AFAIK a "fast-failure" is less useful there because there's only one server anyway.

@stdweird
Copy link
Member Author

ok, that makes sense. but a head timeout is still usefull if there are a lot of things to download, even if it's not from proxies.
the current refactoring uses the same timeout for both proxies and non-proxies (it's easier and cleaner that way). so unless there's a good reason not do it like that, i'd rather leave it like that.

what is (currently) not addressed is the (lack of) per file head_timeout.

@ned21 ned21 added the question label Aug 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants