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 default timeout to requests #27

Merged
merged 2 commits into from
May 25, 2021
Merged

Conversation

dabacon
Copy link
Contributor

@dabacon dabacon commented May 20, 2021

Adds a default timeout to requests of ten minutes. This is a timeout both on connect and request.

Without this, it is possible for the polling for complete jobs to hang indefinitely (we observed what we think was this while running against the QPU).

Ten minutes is a rather large timeout and was pulled out of a bag.

@josh146
Copy link
Member

josh146 commented May 20, 2021

Thanks for both catching (and fixing) this @dabacon!

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Looks good from my end, just a small typo. Perhaps this is something we could consider making user-settable in the future?

Regarding the tests failing, I had a look at the logs and I don't think this is anything to worry about, looks due to GitHub not allowing GitHub secrets to be passed to PRs created from forks.

@@ -129,6 +129,9 @@ def __init__(self, **kwargs):

self.HEADERS = {"User-Agent": self.USER_AGENT}

# Ten minute timout on requests.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Ten minute timout on requests.
# Ten minute timeout on requests.

@dabacon
Copy link
Contributor Author

dabacon commented May 20, 2021

Fixed typoooo.

@josh146 josh146 merged commit 841c0b3 into PennyLaneAI:master May 25, 2021
@rajkk1 rajkk1 mentioned this pull request Apr 24, 2024
@splch splch mentioned this pull request Apr 29, 2024
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