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

Allow specifying the --no-deps flag #117

Merged
merged 1 commit into from Apr 9, 2018
Merged

Allow specifying the --no-deps flag #117

merged 1 commit into from Apr 9, 2018

Conversation

tessereth
Copy link
Contributor

I'm currently trying to migrate my buildkite pipeline to use the new plugin system but I've run into a little snag. I have some steps that require a bunch of dependent services but some steps that do not. I tried using the docker plugin for the steps that don't need the services but the docker plugin makes some assumptions that don't work in my case^. Given I have a docker compose file anyway and I'm using it for the other steps, it would be convenient for me to use the docker-compose plugin for everything but specify the --no-deps flag somehow. I'm not fussed how that option gets set but given I made this change to confirm this solved my problem, I figured I'd make a PR rather than just an issue.

^ For completeness, the docker plugin always specifies the --workdir commandline flag, overriding the default I set in my Dockerfile and mounts the pwd at that directory which I also don't want. My source code is already inside the image that I prebuilt in a previous step. Theoretically changes could be made on the docker plugin to solve my problem as well but this seems cleaner.

@lox
Copy link
Contributor

lox commented Apr 8, 2018

This seems very reasonable. I'm pretty in favor of adding most of the docker-compose run args. Thoughts @toolmantim?

@lox lox requested a review from toolmantim April 8, 2018 22:39
@lox
Copy link
Contributor

lox commented Apr 8, 2018

^ For completeness, the docker plugin always specifies the --workdir commandline flag, overriding the default I set in my Dockerfile and mounts the pwd at that directory which I also don't want. My source code is already inside the image that I prebuilt in a previous step. Theoretically changes could be made on the docker plugin to solve my problem as well but this seems cleaner.

We'll also have a think about this! Reckon that's a bug.

Copy link
Contributor

@toolmantim toolmantim 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 the PR @tessereth! Looks great.

@toolmantim
Copy link
Contributor

@lox yeah, I reckon we should support all of the flags!

@lox lox merged commit 92da7be into buildkite-plugins:master Apr 9, 2018
@lox
Copy link
Contributor

lox commented Apr 9, 2018

@toolmantim Should we implement them all as top level options, or add a compose_args option that passes them directly through?

@toolmantim
Copy link
Contributor

@lox top level options is nicer for the end-user, and the pipeline.yml, so I say that's worth the technical cost?

@lox
Copy link
Contributor

lox commented Apr 9, 2018

Agreed!

@toolmantim
Copy link
Contributor

(there's a only a couple more to add, as some don't make sense like --detach)

@tessereth
Copy link
Contributor Author

Sweet, thanks for merging this!

@lox
Copy link
Contributor

lox commented Apr 9, 2018

Thank you very much for your contribution @tessereth, we will get this into the next version 🙌🏻

@tessereth tessereth deleted the no-deps branch April 10, 2018 00:14
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

3 participants