cli: one project-resolution story — single name precedence, no swallowed load error, strict service validation - #14151
Open
ndeloof wants to merge 1 commit into
Open
cli: one project-resolution story — single name precedence, no swallowed load error, strict service validation#14151ndeloof wants to merge 1 commit into
ndeloof wants to merge 1 commit into
Conversation
…wed load error, strict service validation projectOrName and toProjectName resolved the project with opposite precedences, and projectOrName silently swallowed any load error when COMPOSE_PROJECT_NAME was set: a broken compose file sent stop, down, ps... into label-based reconstruction without a word — even when the file was named explicitly with --file. One precedence now, documented on both resolvers and applied identically by compose-go while loading: --project-name, then COMPOSE_PROJECT_NAME, then the model's name. The failure policy becomes explicit: an unreadable explicit --file is a hard error; no file around with COMPOSE_PROJECT_NAME set stays the silent file-less workflow; a present-but-broken implicit file falls back to label-based mode with a warning. Service-name validation follows one rule — strict whenever a model is available: restart and wait no longer silently no-op on a typo (validateServiceNames, profile-disabled services remain legitimate targets), and the hand-rolled checks in ps and volumes are removed as dead code, the load-time selection already rejecting unknown names (pinned by test). Epic docker#14074, F.4. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
39 tasks
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The PR is well-structured and implements the described behavior correctly:
- Name precedence (
--project-name>COMPOSE_PROJECT_NAME> model name) is applied consistently through the refactoredprojectOrNameandtoProjectName. - Error handling for explicit
--filefailures (hard error), missing files with env name (silent), and broken implicit files with env name (warning + fallback) is correct and clearly documented. validateServiceNamescorrectly handles the nil-project case (label-based mode has no manifest to validate against) and covers profile-disabled services as legitimate targets.- The removal of hand-rolled service checks in
ps.goandvolumes.gois safe:ToProjectservice-selection already rejects unknowns at load time when a model is available. - The new test file covers the full resolution matrix with
t.Context()and clearly-named subtests.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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
2 participants
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.
Epic #14074, F.4, per the agreed behavior:
One name precedence (Q1), documented on both resolvers and applied identically by compose-go while loading:
--project-name>COMPOSE_PROJECT_NAME> the model's name.projectOrNameandtoProjectNameused to disagree.Explicit failure policy (Q2.c) —
projectOrNameused to swallow any load error whenCOMPOSE_PROJECT_NAMEwas set, silently sendingstop/down/ps… into label-based reconstruction, even for an explicit--file:--file→ hard error;COMPOSE_PROJECT_NAME→ the normal file-less workflow, silent;COMPOSE_PROJECT_NAME→ label-based fallback with a warning.Strict service validation whenever a model is available (Q3):
restartandwaitno longer silently no-op on a typo (profile-disabled services remain legitimate targets); the hand-rolled checks inps/volumesare removed as dead code — load-time selection already rejects unknown names (pinned by test).Behavioral changes:
restart/waiton an unknown service now error; a broken explicit--filenow errors instead of silently falling back; a broken implicit file now warns. Unit tests cover the full resolution matrix.🤖 Generated with Claude Code