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 rate limiting #86

Merged
merged 1 commit into from
Sep 27, 2023
Merged

Add rate limiting #86

merged 1 commit into from
Sep 27, 2023

Conversation

stevejalim
Copy link
Collaborator

@stevejalim stevejalim commented Sep 27, 2023

This changeset adds rate limiting to the project in a way that is compatible with Wagtail.

Because the core CMS-based page-serving happens via a single view, we cannot just use the decorators from django-ratelimit as we do elsewhere in the org. Instead we have custom middleware that uses the core helpers from the django-ratelimit package to do the work.

This changeset also adds a 403 template as well as a 429 one for ratelimited responses.

Screenshot 2023-09-27 at 12 44 21 Screenshot 2023-09-27 at 14 55 21

Resolves #85

@stevejalim stevejalim force-pushed the 85-rate-limiting branch 3 times, most recently from 9a74d2d to 98baa26 Compare September 27, 2023 13:27
@@ -19,6 +19,16 @@ jobs:
run: |
pip install -r requirements/production.txt
pip install -r requirements/test.txt
- name: Set up Node
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll be replacing this setup with running tests inside the Docker container soon

This changeset adds rate limiting to the project in a way that is compatible
with Wagtail. Because the core CMS-based page-serving happens via a single
view, we cannot just use the decorators from django_ratelimit as we do
elsewhere in the org. Instead we have custom middleware that uses the core
helpers from the django-ratelimit package to do the work.

This changeset also adds a 403 template as well as a 429 one for ratelimited
responses.
Copy link
Member

@pmac pmac left a comment

Choose a reason for hiding this comment

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

Looking good! Can't be too careful.

@stevejalim stevejalim merged commit e6bf45f into main Sep 27, 2023
2 checks passed
@stevejalim stevejalim deleted the 85-rate-limiting branch September 27, 2023 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Rate limiting
2 participants