Skip to content

Rate limit requests to PyPI due to recent PyPI changes#59

Merged
nicoddemus merged 4 commits intopytest-dev:masterfrom
gnikonorov:issue_58
Nov 20, 2020
Merged

Rate limit requests to PyPI due to recent PyPI changes#59
nicoddemus merged 4 commits intopytest-dev:masterfrom
gnikonorov:issue_58

Conversation

@gnikonorov
Copy link
Member

Rate limit how we send requests to PyPI in update_index.py due to pypi/warehouse#8753.

We should ideally move to https://wiki.python.org/moin/PyPIJSON, but this is a suitable intermediary. After this merges, I will open an issue to make the migration

I tested everything locally, and was able to successfully update my index.json with these code changes

Closes #58

@gnikonorov gnikonorov self-assigned this Nov 18, 2020
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Thanks a lot @gnikonorov!

Left a few comments, let me know what you think!


BLACKLIST = {"pytest-nbsmoke"}

class RateLimitedServerProxy:
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we can use some tenacity decorators around the actual proxy instead of implementing a custom logic?

@retry(wait=wait_fixed(30), stop=stop_after_attempt(10))
def browse(self, classifiers):
    return self._server_proxy.browse(classifiers)

I understand the logic is not as clever as what you implemented, which looks at the actual message to decide what to do, but this might be good enough.

But of course if the plan is to throw this away in favor of a reimplementation using PyPIJSON, I'm happy to have this in the mean time.

Copy link
Member Author

Choose a reason for hiding this comment

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

The plan would be to eventually deprecate this in favor of PyPIJSON

Thank you for sharing tenacity! I didn't know this existed. I think we should look into using it for the proper migration

@gnikonorov
Copy link
Member Author

@nicoddemus - just updated the PR with your feedback and tested locally. I think this is good to go, if you're ok with it

Copy link
Member

@nicoddemus nicoddemus 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 again @gnikonorov!

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.

pypi querry in update_index.py is affected by ratelimit changes

2 participants