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

Add timeout parameter to requests calls because they can hang forever… #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Xangis
Copy link

@Xangis Xangis commented Apr 1, 2021

Add timeout parameter to requests calls because they can hang forever if the connection does not close properly.

I've been using this modified version for my own icon harvesting script and the addition of the timeout parameter prevents hangs.

You may want to adjust the timeout, but 20 seconds has worked well enough for me.

@scottwernervt
Copy link
Owner

@Xangis Thanks for the PR but the timeout value should be left up to the user of this package. We provide an example in the README for setting timeout arg:

Request library parameters can be passed to favicon.get() as keyword arguments:
...
favicon.get('https://www.python.org/', headers=headers, timeout=2)

Maybe instead we can add a note to the README to set it if you are experiencing hangs.

@Xangis
Copy link
Author

Xangis commented Apr 2, 2021

Sure, a note would be good. Requests recommends never calling it without a timeout value because it can hang infinitely, so I figured having some sort of default, even if it's ridiculously long, would prevent nasty surprises.

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.

2 participants