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

Add support for docker compose v2 #362

Closed
wants to merge 3 commits into from

Conversation

psschwei
Copy link

@psschwei psschwei commented Jun 5, 2023

Co-authored-by: Johannes Roos [email protected]

Fixes #306

Uses @jhnnsrs 's approach in #312 while refactoring to use the new structure post #290
Also tweaked the tests a bit so they would pass when using docker compose

@psschwei
Copy link
Author

psschwei commented Jun 8, 2023

Seems cached_property isn't implemented in Python 3.7, which is what's causing those tests to fail.

edit: pushed a commit to drop it, but seems github is having some issues at the moment

edit edit: the fix never got picked up by the PR for some reason, so squashed and force-pushed to get it in. should be good to go now

Copy link
Contributor

@balint-backmaker balint-backmaker left a comment

Choose a reason for hiding this comment

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

Nice! Glad to see more people looking at this! ❤️

I've got one small request but I'm no codeowner so no "request changes")

compose/testcontainers/compose/__init__.py Outdated Show resolved Hide resolved
@psschwei
Copy link
Author

@tillahoffmann a few of the tests were canceled, but I didn't see any errors in the logs... is there anything else needed for this one?

@tillahoffmann
Copy link
Collaborator

@tillahoffmann a few of the tests were canceled, but I didn't see any errors in the logs... is there anything else needed for this one?

They were cancelled because the tests timed out after a few hours. There are likely some issues with spinning up the containers.

@psschwei
Copy link
Author

psschwei commented Jul 6, 2023

looks like I may need to add a comment to get the "requires attention" label back 😄

Signed-off-by: Paul S. Schweigert <[email protected]>
Signed-off-by: Paul S. Schweigert <[email protected]>
@balint-backmaker
Copy link
Contributor

Hey folks! I know things may be going slow right now, but I'd love to put some effort into #358 and would love to do so after this PR has been merged (likely to overhaul the compose package, but this contains some good ideas I want to keep)

@psschwei
Copy link
Author

@tillahoffmann gentle ping

@psschwei
Copy link
Author

The compose module was removed in #394 , so doesn't seem there's any need for this PR anymore...

@psschwei psschwei closed this Nov 27, 2023
@kiview
Copy link
Member

kiview commented Nov 28, 2023

As said in #394 (comment), we are interested in bringing back support to Compose v2 without depending on the Python Docker Compose v1 dependency (so only delegating to the binary). Can this PR be adopted towards this approach?

@psschwei
Copy link
Author

I understand the rationale for dropping the compose module. No hard feelings there 😄

As for adapting the PR... that's a possibility, though I'm not sure if/when I'd have the cycles... so and rather leaving the PR open indefinitely, I figured I'd go ahead and close it. If someone else wanted to resurrect it, that'd be okay with me too.

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.

Allow configuring the docker-compose executable path
4 participants