Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support deploying with docker compose without local docker-compose #462
Support deploying with docker compose without local docker-compose #462
Changes from 10 commits
ae40e04
0cf5df8
2f90314
bee7c63
b1167b1
5fd0142
66253d3
e1c3ab8
3684807
5eb916a
5deebb8
cfe9fce
ebab78f
3246837
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still return a
Local
DockerCompose? It seems weird to me to use a containerised compose method that returns a LocalDC.I'd be open to using a dedicated DockerisedDockerCompose type, although I think could come with changes in return types and generalisation...
Of course, not a blocker for this PR, but if you are interested in following-up with more refactors to make a stable API, I'm too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started with creating dedicated
DockerisedDockerCompose
type, but got too many unreasonable duplications. Using currentLocalDockerCompose
with different executors is a best solution for my opinion.IMHO: interface
DockerCompose
should be removed and typeLocalDockerCompose
should renamed toDockerCompose
, new interfaceComposeExecutor
should be used for extensions.Alternatively:
Rename type
LocalDockerCompose
toDockerComposeXYZ
.Define type
LocalDockerCompose
as alias ofDockerComposeXYZ
for back compatibility and mark it as deprecated:As you can see I used existing docker-compose tests for containerized docker-compose and it is works out-of-box :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wdyt about using a configurable version of compose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced ref to function to interface
ComposeExecutor
with 2 implementations.