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

Expire requestable synchronously #2254

Merged
merged 5 commits into from
Jun 13, 2024
Merged

Conversation

thomasleese
Copy link
Contributor

This fixes an issue where if more than one requestable happened to expire at the same time, there is a race condition between when the status and action required at fields are updated, leading to incorrect application statuses.

Copy link

@thomasleese thomasleese marked this pull request as ready for review June 12, 2024 08:40
@thomasleese thomasleese requested a review from a team as a code owner June 12, 2024 08:40
This adds a convenience method which returns true if the object should
expire at some point (i.e. it's got an expires_at set).
This adds an expired and not_expired scope on the expirable objects
which we can use to better filter the objects, and make it more
readable.
This gives more time to each expiration requestable type to ensure
they're finished before moving on to the next one.
When received the `expires_at` value becomes `nil` and `expires?` will
return `false`. This is to ensure that we don't accidentally expire it
when it shouldn't be.
This fixes an issue where if more than one requestable happened to
expire at the same time, there is a race condition between when the
status and action required at fields are updated, leading to incorrect
application statuses.
Copy link
Contributor

@syed87 syed87 left a comment

Choose a reason for hiding this comment

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

looks good to me

@thomasleese thomasleese merged commit e182141 into main Jun 13, 2024
12 checks passed
@thomasleese thomasleese deleted the perform-expiration-synchronously branch June 13, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants