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

feat: rate limiting #93

Merged
merged 7 commits into from
Jan 3, 2025
Merged

feat: rate limiting #93

merged 7 commits into from
Jan 3, 2025

Conversation

NotPeopling2day
Copy link
Contributor

@NotPeopling2day NotPeopling2day commented Jan 3, 2025

What I did

Implemented rate limiting for Alchemy and did a few little clean ups.

Based off of @antazoey work on core ApeWorX/ape#2398 and the original rate limiting PR for Alchemy #35

fixes: #

How I did it

How to verify it

Checklist

  • Passes all linting checks (pre-commit and CI jobs)
  • New test cases have been added and are passing
  • Documentation has been updated
  • PR title follows Conventional Commit standard (will be automatically included in the changelog)

@NotPeopling2day NotPeopling2day marked this pull request as ready for review January 3, 2025 18:53
antazoey
antazoey previously approved these changes Jan 3, 2025
Copy link
Member

@antazoey antazoey left a comment

Choose a reason for hiding this comment

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

LGTM
it may be worth adding a test to verify rate limiting is working but otherwise it is ok
but not a huge deal as the helper method is already tested in core.

i have 1 comment re: config too

ape_alchemy/config.py Show resolved Hide resolved
antazoey
antazoey previously approved these changes Jan 3, 2025
@NotPeopling2day NotPeopling2day merged commit 8de9fdb into main Jan 3, 2025
25 checks passed
@NotPeopling2day NotPeopling2day deleted the feat/rate-limiting branch January 3, 2025 20:37
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