nginz: enable unlimited_requests_endpoint for authenticated requests, too#3138
Merged
nginz: enable unlimited_requests_endpoint for authenticated requests, too#3138
Conversation
Member
Author
|
I think we should get this in before https://github.com/zinfra/cailleach/pull/1574 to more clearly see possible effects. |
akshaymankar
approved these changes
Mar 28, 2023
Member
akshaymankar
left a comment
There was a problem hiding this comment.
Looks good. I am slightly confused by the commit message and the PR title: where is rate_limit_unlimited defined? I only see unlimited_requests_endpoint
Member
Author
Ah thanks, I got the words all tangled up. Changed description and changelog to use the same word as in the actual settings. |
Member
Author
|
With this PR, the config diff for the assets endpoint is this: location ~* ^(/v[0-9]+)?/assets/(.*) {
# ...
+ # Note that this endpoint has no rate limit per user for authenticated requests
# ...whereas for other authenticated endpoints: location ~* ^(/v[0-9]+)?/system/settings$ {
# ...
+ limit_req zone=reqs_per_user burst=20;
# ...and at the top - # Limit by $zauth_user if present and not part of rate limit exemptions
- limit_req zone=reqs_per_user burst=20; |
jschaul
added a commit
that referenced
this pull request
Mar 29, 2023
Also allow more than 25 concurrent connections for unlimited_requests_endpoint
jschaul
added a commit
that referenced
this pull request
Mar 29, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…quests
The change introduced in #2786 had no effect thus far. With this PR, it should start to have the desired effect.
See also https://github.com/zinfra/cailleach/pull/1574 for context of where this appeared.
Checklist
changelog.d