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

fix retry_interval: set it to 1 second instead of 1000 seconds #14357

Merged
merged 2 commits into from
Nov 9, 2020

Conversation

vbarbaresi
Copy link
Contributor

This looks like it was a typo introduced in #5785 (review)
The review comment makes me think the author meant 1 second, but maybe thought it was milliseconds.

This retry_interval is fed to time.sleep and asyncio.sleep
Both these methods' arguments are in seconds

in #14067 we are experiencing timeouts after ~15 to 16 min (matching 1000 seconds) that are not solved by configuring timeouts in the client

This looks like it was a typo introduced in Azure#5785 (review)
The review comment makes me think the author meant 1 second, but maybe thought it was milliseconds.
time.sleep and asyncio.sleep arguments are both in seconds

in Azure#14067 we are experiencing timeouts after ~15 to 16 min (matching 1000 seconds) that are not solved by configuring timeouts in the client
@ghost ghost added Azure.Core customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Oct 8, 2020
@ghost
Copy link

ghost commented Oct 8, 2020

Thank you for your contribution vbarbaresi! We will review the pull request and get back to you soon.

@ghost
Copy link

ghost commented Oct 8, 2020

CLA assistant check
All CLA requirements met.

@scbedd
Copy link
Member

scbedd commented Oct 8, 2020

/azp run python - core - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vbarbaresi
Copy link
Contributor Author

This looks unrelated to the issue I linked, I don't know where this code is used but it wouldn't hurt to fix it anyway

@xiangyan99 xiangyan99 merged commit c674e6c into Azure:master Nov 9, 2020
@xiangyan99
Copy link
Member

@vbarbaresi Good catch and thanks for your contribution. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants