Skip to content

Conversation

@patrick7kelly
Copy link
Contributor

Motivation

Many of the newer institutions (especially for Auth and Identity) take several minutes to receive a response. It would be ideal for anyone consuming this API to be able to set a timeout as they would for any other request.

This PR adds a timeout argument to the Client() object; all API requests are then limited to the specified timeout, else a ReadTimeoutError is raised.

@michaelckelly michaelckelly requested review from joyzheng and r-ohan and removed request for joyzheng May 3, 2017 21:36
Copy link
Contributor

@r-ohan r-ohan left a comment

Choose a reason for hiding this comment

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

Your changes look good, I left some minor comments about formatting. Thanks for submitting this!


ALLOWED_METHODS = {'post'}
TIMEOUT = 600 # 10 minutes
DEFAULT_TIMEOUT = 600 # 10 minutes
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you make this two spaces between 600 and the comment? (PEP 8)

plaid/client.py Outdated
return post_request(
urljoin('https://' + self.environment + '.plaid.com', path),
data=data,
data=data, timeout=self.timeout
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please put timeout=self.timeout on a new line?

@patrick7kelly
Copy link
Contributor Author

okay, I have made the requested updates

@michaelckelly michaelckelly merged commit 1aa6361 into plaid:master May 8, 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.

3 participants