Skip to content

Update compose to work with both "docker-compose" as well as "docker compose" #254

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

Closed
wants to merge 0 commits into from

Conversation

jhnnsrs
Copy link

@jhnnsrs jhnnsrs commented Oct 13, 2022

Fixed using docker compose with the new "docker compose" api as well as the older "docker-compose" api.

@jooola
Copy link

jooola commented Feb 10, 2023

Fixes #306

Comment on lines 94 to 104
def base_docker_compose(self):
"""
Returns the basecommand parts used for the docker compose commands depending on the docker compose api

Returns
-------
list[str]
The docker compose command parts
"""
return ["docker","compose"] if subprocess.run(["docker", "compose", "--help"], stdout=subprocess.DEVNULL,
stderr=subprocess.STDOUT).returncode == 0 else ["docker-compose"]
Copy link

Choose a reason for hiding this comment

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

I wonder if this should be configurable by the user ? There might be cases where the user prefers docker-compose over docker compose. Maybe the magic here is good, but as fallback from a user provided setting?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That sounds reasonable, maybe we can include a compose_command argument in the constructor. If it is None, we can use the magic to figure out the right one. Otherwise the provided value will be used as-is.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tillahoffmann Given that we have moved all containers to their directory, should we rebase this PR ? Maybe one of us will have to do it if OP is not around.

Copy link
Author

Choose a reason for hiding this comment

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

Accidentally closed this PR and opened #312 which includes the suggested changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Docker Compose call when launching a Docker Compose command Allow configuring the docker-compose executable path
4 participants