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

limit requests per time in download tests #2699

Merged
merged 1 commit into from
Sep 24, 2020

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Sep 22, 2020

This PR is an afterthought to #2665. Instead of blindly waiting 2 seconds after each request, with this we only wait if the interval between two requests to the same server is smaller than 2 seconds.

Right now this only adds (fairly minimal) overhead, since all requests target the same server. Even if the download tests comprise all datasets from torchvision the impact of this optimization will still be low, since the tests are only run once per day.

Still, I think this is good to have since it makes the intention behind the wait time more clear without adding any complexity for someone writing new tests. @fmassa If you disagree with me here, feel free to close this.

@pmeier pmeier requested a review from fmassa September 22, 2020 12:59
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I would just pay a bit attention so that the patched version of urlopen doesn't get used by mistake in other files.

@fmassa fmassa merged commit 6a43a1f into pytorch:master Sep 24, 2020
@pmeier
Copy link
Collaborator Author

pmeier commented Sep 24, 2020

@fmassa Given that urlopen is part of the builtins, I don't think that anyone will import it from here.

@pmeier pmeier deleted the limit-requests branch October 21, 2020 15:06
bryant1410 pushed a commit to bryant1410/vision-1 that referenced this pull request Nov 22, 2020
vfdev-5 pushed a commit to Quansight/vision that referenced this pull request Dec 4, 2020
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