Skip to content

fix(extends): fix extends related logs#645

Merged
ndeloof merged 2 commits intocompose-spec:mainfrom
idsulik:issue-11571
Jul 2, 2024
Merged

fix(extends): fix extends related logs#645
ndeloof merged 2 commits intocompose-spec:mainfrom
idsulik:issue-11571

Conversation

@idsulik
Copy link
Copy Markdown
Contributor

@idsulik idsulik commented Jun 24, 2024

Fixes docker/compose#11571

It used to output wrong data if you extends from file and not full data if you want to use service in current file, new outputs:
local service:
before:
cannot extend service "known" in /Users/main/Development/idsulik/compose/tmp/docker-compose.yaml: service not found
after:
cannot extend service "known" in /Users/main/Development/*/compose/tmp/docker-compose.yaml: service "blah" not found

service from file:
before:
cannot extend service "blah" in in.yaml: service not found
after:
cannot extend service "known" in /Users/main/Development/*/compose/tmp/docker-compose.yaml: service "blah" not found in in.yaml

docker-compose-yaml:

services:
  known:
    extends:
        file: in.yaml
        service: blah
    image: image

@idsulik idsulik requested a review from ndeloof as a code owner June 24, 2024 05:39
@ndeloof ndeloof requested review from glours, jhrotko and laurazard June 24, 2024 07:06
if file != nil {
filename = file.(string)
services, err = getExtendsBaseFromFile(ctx, ref, filename, opts, tracker)
refFilename := file.(string)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I wonder about the impact not assigning filename anymore:
tracker.Add(filename, name) will then fail to capture the target file

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

assigned the name to filename after the func call

Copy link
Copy Markdown
Collaborator

@jhrotko jhrotko left a comment

Choose a reason for hiding this comment

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

as @ndeloof mentioned, could you revert the rename to filename?

@ndeloof ndeloof requested a review from jhrotko July 1, 2024 10:38
Copy link
Copy Markdown
Collaborator

@glours glours left a comment

Choose a reason for hiding this comment

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

LGTM

@glours
Copy link
Copy Markdown
Collaborator

glours commented Jul 2, 2024

@idsulik can you fix the conflict, please?

Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
@idsulik
Copy link
Copy Markdown
Contributor Author

idsulik commented Jul 2, 2024

@idsulik can you fix the conflict, please?

@glours done

@ndeloof ndeloof merged commit a2bc3b4 into compose-spec:main Jul 2, 2024
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.

[BUG] misleading error when extending an unknown service

4 participants