Skip to content

Use Redis for one-time passcode sending rate limits#7725

Merged
mitchellhenke merged 6 commits intomainfrom
mitchellhenke/remove-database-otp-rate-limiter
Feb 3, 2023
Merged

Use Redis for one-time passcode sending rate limits#7725
mitchellhenke merged 6 commits intomainfrom
mitchellhenke/remove-database-otp-rate-limiter

Conversation

@mitchellhenke
Copy link
Contributor

🛠 Summary of changes

Followup to #7554. The major environments have been upgraded to enable the read flag and the remaining environments will have been writing to the Redis value since the previous PR was deployed, so we should be ok to start reading from Redis everywhere and dropping the database writes.

changelog: Internal, Rate Limiting, Use Redis for one-time passcode sending rate limits
Mitchell Henke and others added 3 commits January 30, 2023 17:11
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM


# rubocop:disable Naming/MemoizedInstanceVariableName
def entry_for_current_phone
@entry ||= OtpRequestsTracker.find_or_create_with_phone_and_confirmed(phone, phone_confirmed)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same note about this method not being used anywhere and could be removed from OtpRequestsTracker.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify, are we planning to drop the otp_requests_trackers table altogether at some point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, plan is to drop everything related to OtpRequestsTracker in this, and drop the table if we're content with the functionality.

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/remove-database-otp-rate-limiter branch from 23e13df to 7ea58d4 Compare January 31, 2023 19:41
@mitchellhenke mitchellhenke marked this pull request as ready for review January 31, 2023 20:34
@mitchellhenke mitchellhenke merged commit 9c9cef4 into main Feb 3, 2023
@mitchellhenke mitchellhenke deleted the mitchellhenke/remove-database-otp-rate-limiter branch February 3, 2023 17:12
@aduth aduth mentioned this pull request Feb 9, 2023
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.

3 participants