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

Revert usage of "-" as separator for resource names #297

Merged
merged 1 commit into from Aug 1, 2022

Conversation

glours
Copy link
Contributor

@glours glours commented Aug 1, 2022

As noticed by some users in the original PR, the changes were breaking ones and unjustified regarding the value offered in counter part.

This reverts commit 56e6c33, reversing
changes made to 0ab97a2.

@glours glours requested a review from ndeloof as a code owner Aug 1, 2022
…esource-names"

This reverts commit 56e6c33, reversing
changes made to 0ab97a2.

Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
@glours glours self-assigned this Aug 1, 2022
@glours glours requested review from milas, ulyssessouza and laurazard Aug 1, 2022
ndeloof
ndeloof approved these changes Aug 1, 2022
Copy link
Collaborator

@ndeloof ndeloof left a comment

Choose a reason for hiding this comment

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

+1
while mix of dash and underscore can create some confusion, the benefit is poor regarding the backward compatibility impact

Copy link
Contributor

@ulyssessouza ulyssessouza left a comment

Choose a reason for hiding this comment

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

LGTM

@ulyssessouza ulyssessouza merged commit 1656887 into compose-spec:master Aug 1, 2022
11 checks passed
@Jyrno42
Copy link

Jyrno42 commented Aug 5, 2022

Also pinging here for more visibility.

I am having some trouble with the updated compose version 2.9.0 where this fix should be applied. Essentially the separator still seems to be - when COMPOSE_COMPATIBILITY=1 is not set.

see more context and a repro case here docker/compose#9700 (comment)

E: Turns out that I was just confused. The revertion was done only for related resources like networks, volumes, secrets and config. The container names will be using -.

ThorgateDeveloper pushed a commit to thorgate/django-project-template that referenced this pull request Aug 5, 2022
Docker changed the behaviour of the container naming to use `-`. Originally we
believed that this change was reverted in 2.9.0 as it caused a breaking change.
But it turns out instead they reverted it only for related resources such as
networks, volumes, secrets and config.

This means that going forward the container names use dashes (`-`).

Note: Also updated the poetry wrapper image name to match this new convention.

Refs:

- https://docs.docker.com/compose/release-notes/#290
- docker/compose#9700
- docker/compose#9700 (comment)
- compose-spec/compose-go#297
- Internal ref: https://thorgate.slack.com/archives/C02AMG235/p1659433239011469

Additional changes in the PR:

- Update poetry installation command
  - ref: https://python-poetry.org/docs/master/#installing-with-the-official-installer
- Remove pypi docker compose in ci image (docker-compose now comes from inside docker image anyway)
ThorgateDeveloper pushed a commit to thorgate/django-project-template that referenced this pull request Aug 5, 2022
Docker changed the behaviour of the container naming to use `-`. Originally we
believed that this change was reverted in 2.9.0 as it caused a breaking change.
But it turns out instead they reverted it only for related resources such as
networks, volumes, secrets and config.

This means that going forward the container names use dashes (`-`).

Note: Also updated the poetry wrapper image name to match this new convention.

Refs:

- https://docs.docker.com/compose/release-notes/#290
- docker/compose#9700
- docker/compose#9700 (comment)
- compose-spec/compose-go#297
- Internal ref: https://thorgate.slack.com/archives/C02AMG235/p1659433239011469

Additional changes in the PR:

- Update poetry installation command
  - ref: https://python-poetry.org/docs/master/#installing-with-the-official-installer
- Remove pypi docker compose in ci image (docker-compose now comes from inside docker image anyway)
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.

None yet

4 participants