Skip to content

CORS: cache redirect uris from DB#5575

Merged
zachmargolis merged 3 commits intomainfrom
margolis-cors-cache-redirect-uris
Nov 2, 2021
Merged

CORS: cache redirect uris from DB#5575
zachmargolis merged 3 commits intomainfrom
margolis-cors-cache-redirect-uris

Conversation

@zachmargolis
Copy link
Contributor

@zachmargolis zachmargolis commented Nov 2, 2021

This is a very hot codepath, so caching this helps us alleviate pressure on the DB

- Very hot codepath, and 5 minutes of outdated data is likely acceptable
aamva_sp_banlist_issuers: '[]'
aamva_verification_request_timeout: '5'
aamva_verification_url: https://example.org:12345/verification/url
all_redirect_uris_cache_duration_minutes: '5'
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be open to reducing it to 2 minutes? Without a good way to expire, I'd rather err on the side of caution for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
all_redirect_uris_cache_duration_minutes: '5'
all_redirect_uris_cache_duration_minutes: '2'

Copy link
Contributor Author

@zachmargolis zachmargolis Nov 2, 2021

Choose a reason for hiding this comment

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

Done! I think if we wanted to expire the cache manually we could so something like this in the console:

Rails.cache.delete('all_service_provider_redirect_uris')

but sure, switched to 2 minutes

Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>
@zachmargolis
Copy link
Contributor Author

cc @jmhooper maybe a good candidate to include in RC 164 if we want to

@jmhooper
Copy link
Contributor

jmhooper commented Nov 2, 2021

Yeah, I can patch this one in!

@zachmargolis zachmargolis merged commit d15f47f into main Nov 2, 2021
@zachmargolis zachmargolis deleted the margolis-cors-cache-redirect-uris branch November 2, 2021 21:28
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