Skip to content

Conversation

@moppymopperson
Copy link
Contributor

This is proposed solution to #67. The newly added timeout parameter is None by default, so the default behavior remains unchanged, but gives the option to add a timeout to those who wish to use one.

Originally I had added a timeout variable to the API class, but after thinking about it for awhile, I believe making the timeout an argument on public_query and private_query methods is a better solution, and this PR adopts that stance. Having to specify the timeout on each request is a little bit more verbose, but makes it easier to specify timeouts independently and dynamically for each endpoint.

Default value is None, so the current default behavior remains unchanged
The default value is None, which means the default behavior remains unchanged
The default value is None, which means the default behavior remains unchanged
@veox
Copy link
Owner

veox commented Dec 24, 2017

LGTM!

IMO having a timeout variable on the API class, to specify a default timeout, in addition to the "arguments" approach, is the way this will end up; especially taking into account the "retries" implementations in PR #65. This might need some thought, though, and introduction of kwargs...

Maybe next year. :)

@veox veox merged commit 7ac3e38 into veox:master Dec 24, 2017
@veox veox mentioned this pull request Dec 24, 2017
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