Skip to content

Require use of JobDeleteManyParams.UnsafeAll to delete all jobs#1033

Merged
brandur merged 1 commit into
masterfrom
brandur-unsafe-all
Sep 11, 2025
Merged

Require use of JobDeleteManyParams.UnsafeAll to delete all jobs#1033
brandur merged 1 commit into
masterfrom
brandur-unsafe-all

Conversation

@brandur

@brandur brandur commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

Addresses #1031. It's come to light that it's relatively easy to
accidentally delete all non-running jobs by invoking JobDeleteMany
with nil parameters or parameters with all defaults.

Here, make it a little harder to do so by requiring an invocation to
JobDeleteManyParams.UnsafeAll before a blanket delete is allowed.
Calling UnsafeAll isn't necessary if any filters are applied.

Fixes #1031.

@brandur
brandur force-pushed the brandur-unsafe-all branch 3 times, most recently from de55825 to b604cf5 Compare September 9, 2025 01:52
@brandur

brandur commented Sep 9, 2025

Copy link
Copy Markdown
Contributor Author

@bgentry Thoughts on this approach?

@brandur
brandur requested a review from bgentry September 9, 2025 01:54

@bgentry bgentry left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah this is roughly what I had in mind! I think this is about as good as we can do with the param builder pattern.

@defany do you have any thoughts before we ship this?

@defany

defany commented Sep 9, 2025

Copy link
Copy Markdown

Hey! I think you should mention "right" pattern in the docs that user should redeclare variable due to struct copying under the hood and/or mention this in the error

Yeah this is roughly what I had in mind! I think this is about as good as we can do with the param builder pattern.

@defany do you have any thoughts before we ship this?

Addresses #1031. It's come to light that it's relatively easy to
accidentally delete all non-running jobs by invoking `JobDeleteMany`
with `nil` parameters or parameters with all defaults.

Here, make it a little harder to do so by requiring an invocation to
`JobDeleteManyParams.UnsafeAll` before a blanket delete is allowed.
Calling `UnsafeAll` isn't necessary if any filters are applied.

Fixes #1031.
@brandur

brandur commented Sep 11, 2025

Copy link
Copy Markdown
Contributor Author

Thanks guys!

Hey! I think you should mention "right" pattern in the docs that user should redeclare variable due to struct copying under the hood and/or mention this in the error

Alright, I just put a concrete example of how to use it right in its docblock. Hopefully that'll help hint people in the right direction.

@brandur
brandur merged commit d5410e9 into master Sep 11, 2025
10 checks passed
@brandur
brandur deleted the brandur-unsafe-all branch September 11, 2025 06:22
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.

Non-mutable builder pattern for JobDeleteMany might be dangerous

3 participants