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

Enable using stdin when starting docker compose #457

Conversation

mal-as
Copy link

@mal-as mal-as commented Jun 13, 2022

Closes #285

compose.go Outdated
return err
}

file, err := ioutil.TempFile("/Users/aleksamalyshev/", "__tmp_")
Copy link
Member

@mdelapenya mdelapenya Jun 13, 2022

Choose a reason for hiding this comment

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

I think this path is a leftover 😅

Copy link
Author

Choose a reason for hiding this comment

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

sorry, fixed it :)

Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

Thanks for your work with this PR. I left a few review comments before merging it. Please let me know what you think

compose.go Outdated Show resolved Hide resolved
compose.go Outdated
return err
}

file, err := ioutil.TempFile("", "__tmp_")
Copy link
Member

Choose a reason for hiding this comment

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

Do you foresee any collision between multiple runs? Do you think we could use the compose identifier as part of the file name in that case 🤔?

Copy link
Author

Choose a reason for hiding this comment

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

fixed it

@@ -103,8 +104,16 @@ func NewLocalDockerCompose(filePaths []string, identifier string, opts ...LocalD

dc.absComposeFilePaths = make([]string, len(filePaths))
for i, cfp := range dc.ComposeFilePaths {
abs, _ := filepath.Abs(cfp)
dc.absComposeFilePaths[i] = abs
if cfp == "-" {
Copy link
Member

Choose a reason for hiding this comment

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

Mmm I do not get this line. Is it expected to receive a - as a file path for a compose file?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it is

@mdelapenya
Copy link
Member

@mal-as #470 was created yesterday, and I think it collides with this one. What are your thoughts on that PR?

@mdelapenya
Copy link
Member

@mal-as I would like to bring you attention to #476, which is a big refactor of the docker compose module, towards a native API.

Would you mind if we review (and probably rebase) this PR right after that one is reviewed and merged? Thanks in advance

@mdelapenya
Copy link
Member

Hi @mal-as we have merged #476, which adds native support for docker compose. Would you mind revisiting this PR and update to that code? I can assist if needed, or we can even ping @baez90 for that.

Thanks in advance

@mdelapenya
Copy link
Member

I think this PR is superseded by #466, therefore I'm closing it. @mal-as could you please reopen it if you feel it's different? Thanks!

@mdelapenya mdelapenya closed this Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compose Docker Compose.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Enable using stdin when starting docker compose
2 participants