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.

Sign up for GitHub

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

Jump to bottom

ratelimits: Fix transaction building for Failed Authorizations Limit #7344

Merged
merged 3 commits into from
Mar 6, 2024
Merged

ratelimits: Fix transaction building for Failed Authorizations Limit #7344

merged 3 commits into from
Mar 6, 2024

Conversation

beautifulentropy
Copy link
Member

@beautifulentropy beautifulentropy commented Feb 29, 2024 •
edited
Loading

  • Update the failed authorizations limit to use 'enum:regId:domain' for transactions while maintaining 'enum:regId' for overrides.
  • Modify the failed authorizations transaction builder to generate a transaction for each order name.
  • Rename the FailedAuthorizationsPerAccount enum to FailedAuthorizationsPerDomainPerAccount to align with its corrected implementation. This change is possible because the limit isn't yet deployed in staging or production.

Blocks #7346
Part of #5545

@github-actions GitHub Actions
Copy link
Contributor

@beautifulentropy, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

@beautifulentropy beautifulentropy force-pushed the fix-ratelimits-failed-authorizations branch 3 times, most recently from 6471d3d to cd1b41b Compare February 29, 2024 16:16
@beautifulentropy beautifulentropy marked this pull request as ready for review February 29, 2024 16:16
@beautifulentropy beautifulentropy requested a review from a team as a code owner February 29, 2024 16:16
@beautifulentropy beautifulentropy force-pushed the fix-ratelimits-failed-authorizations branch from 288d159 to 3a6a21e Compare February 29, 2024 16:37
pgporada
pgporada previously approved these changes Feb 29, 2024
View reviewed changes
@beautifulentropy beautifulentropy requested a review from a team February 29, 2024 21:00
@beautifulentropy beautifulentropy mentioned this pull request Feb 29, 2024
Merged
aarongable
aarongable approved these changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

LGTM modulo one optional comment

ratelimits/bucket.go Show resolved Hide resolved
@beautifulentropy beautifulentropy merged commit 529157c into main Mar 6, 2024
20 checks passed
@beautifulentropy beautifulentropy deleted the fix-ratelimits-failed-authorizations branch March 6, 2024 18:48
@beautifulentropy
Copy link
Member Author

@beautifulentropy, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

I have informed SRE of the configuration changes made in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aarongable aarongable aarongable approved these changes

@pgporada pgporada pgporada approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@beautifulentropy @pgporada @aarongable