Repository navigation
feat(run): --no-recreate reuses existing dependencies as-is - #14144
Conversation
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The flag implementation is clean and correct. The flag wires properly from the CLI option through createOptions.noRecreate → recreateStrategy() / dependenciesRecreateStrategy() (both already handle noRecreate → RecreateNever), and the propagation into startDependencies via options.Recreate / options.RecreateDependencies is complete. Default behavior is unchanged. No logic errors, nil-pointer risks, or missing error handling found in the changed code.
The dependencies a one-off starts go through the regular create path and its config-hash divergence check, with no way to opt out: any resolution difference with the command that created them (environment, model edit) recreates the dependency — destroying state a previous run built up (issue docker#13769's database losing its users). up already offers --no-recreate for exactly this; run now aligns: the flag maps to the "never" recreate policy, propagated through the RunOptions.CreateOptions fields that startDependencies previously dropped. Default behavior is unchanged (empty policy = "diverged"). Closes docker#13769 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
8e555a7 to
21cbb53
Compare
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The implementation is correct. --no-recreate maps cleanly to the existing RecreateNever policy via the existing recreateStrategy()/dependenciesRecreateStrategy() helpers, and both Recreate and RecreateDependencies are propagated from RunOptions.CreateOptions into startDependencies. The E2E test covers both sides (default recreates a diverged dependency; --no-recreate preserves it). No bugs found in the added lines.
Lower-confidence findings (not posted inline)
- [low] pkg/compose/run.go:303 — Comment says 'empty means the default diverged policy' but the actual value passed is the non-empty string
"diverged"(confidence: not scored — low severity, not delegated to verifier) - [low] cmd/compose/run.go:415 — Comment about setting both Recreate and RecreateDependencies could clarify that RecreateDependencies covers the transitive dependency chain (confidence: not scored — low severity, not delegated to verifier)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
runstarts its dependencies through the regular create path and its config-hash divergence check, with no way to opt out: any resolution difference with the command that created them recreates the dependency — destroying state a previous run built up (#13769: database losing its users).upalready offers--no-recreatefor exactly this;runnow aligns.The flag maps to the existing "never" recreate policy, propagated through the
RunOptions.CreateOptionsfieldsstartDependenciespreviously dropped. Default behavior is unchanged (empty policy = "diverged"). E2E test pins both sides: default recreates a diverged dependency,--no-recreatereuses it as-is.Closes #13769
🤖 Generated with Claude Code