Repository navigation
Add pilot hooks for job deletion - #1500
Merged
Merged
Conversation
bgentry
force-pushed
the
bg/pilot-job-delete-hooks
branch
2 times, most recently
from
October 13, 2026 02:22
f89d12b to
a588258
Compare
`Client.JobDelete`, `Client.JobDeleteMany`, their `Tx` variants, and the job cleaner's `JobDeleteBefore` call the driver executor directly, so a pilot can't take part in deletes the way it can in cancels, retries, and completions. Add an optional `PilotJobDeleter` interface with `JobDelete`, `JobDeleteMany`, and `JobDeleteBefore`. Each hook receives the executor and the driver params and performs the delete itself, like `JobCancel` and `JobRetry`, so a pilot can substitute its own delete statement or run additional writes in the same transaction. The job cleaner now takes the client's pilot in its config. The interface is temporarily kept separate from `Pilot`, following the `PilotJobRescuer` pattern, so existing pilot implementations still compile; it's meant to be embedded into `Pilot`, and the fallbacks removed, once supported implementations have adopted it. The client and job cleaner use the hooks when the pilot implements them and otherwise fall back to the executor. `StandardPilot` implements them by calling the executor, so behavior is unchanged by default. Tests cover the client and cleaner routing deletes through the pilot, the executor fallback for pilots without the new interface, and the standard pilot's pass-through.
bgentry
force-pushed
the
bg/pilot-job-delete-hooks
branch
from
October 13, 2026 02:25
a588258 to
15f10b0
Compare
brandur
approved these changes
Oct 13, 2026
brandur
left a comment
Contributor
There was a problem hiding this comment.
Thanks for trying to keep it compatible with the existing versions. The last two pilot changes were breaking, and it really reminded me as I was cutting releases that we need to stop doing that. If any pro user tries to upgrade after we release River but before we release River Pro, they'll get an error, and the full pro release still takes substantial time so that window is like an hour.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Client.JobDelete,Client.JobDeleteMany, theirTxvariants, and the job cleaner call the driver executor directly, so unlike cancels, retries, and completions, deletes never go through the pilot.This adds an optional
PilotJobDeleterinterface withJobDelete,JobDeleteMany, andJobDeleteBefore(the job cleaner's batch delete). LikeJobCancelandJobRetry, each hook receives the executor and driver params and performs the delete itself, so a pilot can substitute its own delete statement or run additional writes in the same transaction, usingdbutil.WithTxVto reuse the caller's transaction when there is one. The job cleaner now receives the client's pilot through its config.The interface is temporarily kept separate from
Pilot, following thePilotJobRescuerpattern, so existing pilot implementations still compile. It isn't meant to stay optional: once supported implementations have adopted it, it gets embedded intoPilotand the executor fallbacks are removed, as the doc comment says. The client and cleaner use the hooks when the pilot implements them and fall back to the executor otherwise.StandardPilotimplements them as plain executor calls, so default behavior is unchanged.