Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Add docker_compose module #354

Closed
wants to merge 2 commits into from
Closed

Add docker_compose module #354

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 30, 2015

This adds support for using docker-compose (formerly fig) as a module.
For more information, see here: http://docs.docker.com/compose/

This adds support for using docker-compose (formerly fig) as a module.
For more information, see here: http://docs.docker.com/compose/
@bcoca
Copy link
Member

bcoca commented May 8, 2015

The exception handling is too generic, errors will depend on
underlying api being clear, which is not always the case, we prefer
more fine grained messages on our side, always adding the message from
the underlying library.

description:
- Select the compose action to perform
required: true
choices: ['build', 'kill', 'pull', 'rm', 'scale', 'start', 'stop', 'restart', 'up']
Copy link
Member

Choose a reason for hiding this comment

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

command is not a pattern we like to follow, we prefer a more imperative 'state', in this case some I would need to know more about some of the options which don't seem to fit

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean s/imperative/declarative/ right?

Copy link
Member

Choose a reason for hiding this comment

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

^ what he said.

Copy link
Author

Choose a reason for hiding this comment

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

In this case though, 'state' isn't really accurate. state=scale doesn't make much sense to me. state=up and state=start are two different commands and I think it would lead to confusion.

@abadger
Copy link
Contributor

abadger commented May 8, 2015

I noticed that this module lacks the .py extension. In current ansible, the ansible modules are python modules as well so they need to be named like docker_compose.py

@ghost
Copy link
Author

ghost commented May 9, 2015

To be clear, for error handling the expectation is to catch errors from docker-compose and have an explict message in the module instead of passing up the exceptions message?

@bcoca
Copy link
Member

bcoca commented May 11, 2015

no, normally the APIs give generic messages and are not very helpful to map to the ansible input, we prefer to use exception handling on 'connect/create/destroy' and return a message with the action attempted + the underlying exception.

@gregdek
Copy link
Contributor

gregdek commented Jun 19, 2015

closing and reopening to kick travis, since there are a bunch of unrelated failures here

@gregdek gregdek closed this Jun 19, 2015
@gregdek gregdek reopened this Jun 19, 2015
@gregdek
Copy link
Contributor

gregdek commented Jun 19, 2015

OK, travis issues are cleared. @SamYaple if you can address @bcoca issues about exception handling and then let us know, we can re-review for inclusion.

@ghost
Copy link
Author

ghost commented Jun 19, 2015

I am going to have to abandon this for now. Each new revision of docker-compose is changing key parts of the structure. Almost like it wasn't meant to be a library to be imported ;)

I am going to wait for that structure to stabilize before attempting. If someone else wants to continue the work they can go for it

@ghost ghost closed this Jun 19, 2015
@gregdek
Copy link
Contributor

gregdek commented Jun 19, 2015

Thanks @SamYaple.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants