-
Notifications
You must be signed in to change notification settings - Fork 2
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
MLPAB-2321: Delete scheduled tasks when CANNOT_REGISTER lpa-updated event received #1640
MLPAB-2321: Delete scheduled tasks when CANNOT_REGISTER lpa-updated event received #1640
Conversation
Some things to think about with scheduled tasks going forward:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1640 +/- ##
==========================================
+ Coverage 94.35% 94.83% +0.48%
==========================================
Files 283 282 -1
Lines 15581 15551 -30
==========================================
+ Hits 14701 14748 +47
+ Misses 709 632 -77
Partials 171 171
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
internal/dynamo/client.go
Outdated
@@ -375,6 +399,32 @@ func (c *Client) DeleteOne(ctx context.Context, pk PK, sk SK) error { | |||
return err | |||
} | |||
|
|||
func (c *Client) DeleteManyByUID(ctx context.Context, keys []Keys, uid string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there is any point having uid
again, we should be able to trust the previous query
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so could use DeleteKeys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd originally used DeleteKeys
but then switched to this. I know the likelihood of getting exactly the same timestamp + action type is low but as the keys aren't unique I wanted a failsafe there to be sure we weren't deleting unintended events. If you're comfortable with that risk feel free to drop this while I'm out and just rely on the keys 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
every (PK, SK) is unique, so it won't be an issue. I have spotted Put
doesn't use Create
, so it would currently overwrite an existing scheduled event, I'm going to change that
0ba59ba
to
d7563bd
Compare
…if cannot register
d7563bd
to
5675ae5
Compare
Purpose
Fixes MLPAB-2321