refactor: unify duplicate execution paths in executor#2
Conversation
Eliminate ~90% code duplication between queue-based and polling-based execution modes by: - Merging execute() and executeClaimedCommand() into execute(instance, claimed bool) with 2 small guards at the 3 points where behavior diverges - Merging safeExecute() and safeExecuteClaimed() into safeExecute(instance, claimed bool) - Adding scheduleFn function field (set in Start()) that is a no-op in polling mode, making the dead code path explicit instead of accidental - Renaming schedule() to queueSchedule() for clarity - Simplifying recoverStuckCommands() to use scheduleFn instead of manual LockingStorage type assertion Net: ~100 lines removed, single execution path, zero behavior change. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change `durex` to `/durex` so only the root-level binary is ignored, not the `cmd/durex/` directory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
simonovic86
force-pushed
the
claude/xenodochial-taussig
branch
from
92d4146 to
8f12b27
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b711f55139
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Schedule for execution | ||
| e.schedule(instance) | ||
| e.scheduleFn(instance) |
There was a problem hiding this comment.
ReplayFromDLQ now invokes e.scheduleFn(instance) directly, but scheduleFn is only assigned inside Start() (see executor.go initialization in Start), so calling this public API before the executor is started will panic with a nil function call; before this refactor, schedule() was always callable and did not crash in that scenario. This affects DLQ admin flows that replay commands while the worker loop is not yet running.
Useful? React with 👍 / 👎.
The `buildir` checker doesn't exist in golangci-lint's govet settings, causing CI to fail with "the configuration contains invalid elements". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove ineffectual argNum++ after last query condition in postgres Find() - Initialize scheduleFn to no-op in New() so it's never nil before Start() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Addressed review feedback in 098b4eb:
|
1 participant
Summary
execute()andexecuteClaimedCommand()into a singleexecute(instance, claimed bool)method — theclaimedbool controls the 3 points where queue-mode and polling-mode behavior divergessafeExecute()andsafeExecuteClaimed()similarlyscheduleFnfunction field set inStart()— no-op in polling mode,queueSchedulein queue mode — replacing the deadschedule()code pathrecoverStuckCommands()to usescheduleFninstead of manualLockingStoragetype assertiongofmt -sto CLI command filesNet: ~100 lines removed from executor.go, zero behavior change, all tests pass with race detector.
Test plan
go build ./...succeedsgo test -race ./...— all tests pass (38s main, 5s storage)🤖 Generated with Claude Code