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

Added timeout support. #12

Closed

Conversation

catalanojuan
Copy link

I couldn't find any info regarding a default timeout on urllib3, it might be the one specified by socket, though I'm not sure about this. In any case I think it's desirable to at least be able to define a more granular timeout, at this level.

I think this implementation keeps backwards compatibility, but please let me know if it doesn't.

Also, I ran tests locally (had to create a trial account for that) and 2 of them fail. I couldn't find the reason but my guess is that it has to do with using a trial account vs an upgraded one. I'm guessing this since I run the whole suite without my modifications and it still fails. Again, please let me know if that's not the case.

I tested this without specifying a timeout - just to see if it doesn't break working code - against our staging servers and it works ok (both queries and documents CRUD).

Let me know what you think.

@catalanojuan
Copy link
Author

Weird, I thought it would use the same .travis.yml for running this PR's tests in travis and therefore the same API ID and SECRET, but it seems like it's not the case.

@catalanojuan
Copy link
Author

@speedblue what do you think? Is this sth you'd like to merge? Thanks in advance!

@speedblue
Copy link
Member

Thanks for the pull request, I will just allow to specify different timeout (connect_timeout=1s and read_timeout=30s by default).

@catalanojuan
Copy link
Author

@speedblue ok, great! that makes sense, thanks!

BTW just out of curiosity, to what extent is it possible to define the timeout for the whole Pool instead of passing the argument to each request? As far as I understand, right now is not possible because the pool is instantiated globally for the module and therefore it's parameters cannot be modified by the user. Maybe with an environment variable?

Thanks again for the prompt reply!

@speedblue
Copy link
Member

I have just released version 1.5.0 of the API client with connection timeout set.
Yes the option can be set on the pool directly via urllib3.PoolManager but it would still need to be set at request to let the user configure it via set_timeout

@speedblue speedblue closed this Dec 26, 2014
shortcuts pushed a commit that referenced this pull request Nov 24, 2023
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