Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix COMPOSE_PROJECT_NAME variable parsing order regression #805

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nkay08
Copy link

@nkay08 nkay08 commented Nov 21, 2023

In a previous change (9d5b255) the support to read COMPOSE_PROJECT_NAME from the name attribute in the compose.yml file was added. Since the attribute is only available after parsing the compose file, the resolution of the variable was done afterwards.

The variable is therefore usable inside the container, HOWEVER it can no longer be used for substitution while loading the compose file. Values that depend on this variable are therefore empty at the point of parsing the compose file.

COMPOSE_PROJECT_NAME is either loaded from the environment or set to dir_basename by default.

This commit changes the order of parsing COMPOSE_PROJECT_NAME variable. First the compose file is loaded into a dict, then the single name attribute is evaluated and replaced, then if it does not exist, the default value is used.

In a previous change (9d5b255) the support to read `COMPOSE_PROJECT_NAME` from the `name` attribute in the `compose.yml` file was added. Since the attribute is only available after parsing the compose file, the resolution of the variable was done afterwards.

The variable is therefore usable inside the container, HOWEVER it can no longer be used for substitution while loading the compose file. Values that depend on this variable are therefore empty at the point of parsing the compose file.

`COMPOSE_PROJECT_NAME` is either loaded from the environment or set to `dir_basename` by default.

This commit changes the order of parsing `COMPOSE_PROJECT_NAME ` variable.
First the compose file is loaded into a dict, then the single `name` attribute is evaluated and replaced, then if it does not exist, the default value is used.

Signed-off-by: NKay <[email protected]>
Copy link
Collaborator

@p12tic p12tic left a comment

Choose a reason for hiding this comment

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

Looks good, but we will need a unit test for this.

@Cabanon
Copy link

Cabanon commented Sep 27, 2024

I would really like to see this PR merged, this regression has broken many of my deployments

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.

3 participants