Support alternate Dockerfile name.#1075
Conversation
compose/service.py
Outdated
There was a problem hiding this comment.
This isn't quite right, fileobj would be for the whole context. This needs to use the dockerfile kwarg.
I don't think that option is in a release yet. I added it after the 1.0.0 release
There was a problem hiding this comment.
Yeah the version I had locally had the following, so I'm assuming it hasn't been released yet
def build(self, path=None, tag=None, quiet=False, fileobj=None,
nocache=False, rm=False, stream=False, timeout=None,
custom_context=False, encoding=None, pull=True,
forcerm=False):This was the only way I could figure out how to get it to work (with current release), dockerfile param would be much better once it's released.
There was a problem hiding this comment.
Do you think I should update this to use dockerfile (even know it's not released yet?)
There was a problem hiding this comment.
That would be great. I think we can ask for a new release when this is ready.
I believe fileobj doesn't actually work when the Dockerfile makes use of other files in the build context.
There was a problem hiding this comment.
yea I was just testing that, and you are correct. I'm fighting with getting the client installed to test it out.
There was a problem hiding this comment.
you can use a git url with pip https://pip.pypa.io/en/latest/reference/pip_install.html#git to get the latest master of docker-py.
As a side note, I wonder if the config option should be dockerfile, to keep it consistent with the api key.
There was a problem hiding this comment.
Within docker-py there's a utils.compare_version
def compare_version(v1, v2):
"""Compare docker versions
>>> v1 = '1.9'
>>> v2 = '1.10'
>>> compare_version(v1, v2)
1
>>> compare_version(v2, v1)
-1
>>> compare_version(v2, v2)
0
"""
s1 = StrictVersion(v1)
s2 = StrictVersion(v2)
if s1 == s2:
return 0
elif s1 > s2:
return -1
else:
return 1Do you know if there's a helper like this already within compose so I could add something like :
if 'dockerfile' in options and utils.compare_version('1.17', client.api_version()) is -1:
raise ConfigError('Docker client API must be greater than 1.17 to support the dockerfile option')and when building:
def build(self, no_cache=False):
log.info('Building %s...' % self.name)
build_kwars = dict(
tag=self.full_name,
stream=True,
rm=True,
nocache=no_cache,
)
if 'dockerfile' in self.options and \
utils.compare_version('1.17', self.client.api_version()) is not -1:
build_kwars['dockerfile'] = self.options.get('dockerfile', None)
build_output = self.client.build(
self.options['build'],
**build_kwars
)There was a problem hiding this comment.
There isn't, but I wouldn't worry about it. We're planning on requiring the latest docker to pickup some new features in 1.6 anyway.
|
I haven't tested this with unreleased docker-py, but hopefully it should work. The Let me know what you think, thanks for helping with this. |
|
Note: Not sure where the api version is coming from I did the following to update
|
|
Just saw your command about the version check (got squished under the change), let me know if you'd like me to remove the checks that I added. @dnephin |
|
Ya, I would remove it. If someone is an on earlier version, they can just not specify the option and it should continue to work without the dockerfile support. |
|
I tried to use this locally, but ran into some issues. I got the error about the API version, so I removed those checks, but now I'm getting $ docker version
Client version: 1.5.0
Client API version: 1.17
Go version (client): go1.4.1
Git commit (client): a8a31ef
OS/Arch (client): darwin/amd64
Server version: 1.5.0
Server API version: 1.17
Go version (server): go1.4.1
Git commit (server): a8a31efAnd I also updated docker-py to the latest commit. Is there anything else I need to do to get this working? Thanks! |
|
@mwean You need to install the version from github From: To: otherwise you'll get a version conflict. |
|
@dnephin I removed the checks, but would you like me to add the **build_kwargs portion back in. That way if they are on an outdated version of |
|
Maybe this? |
|
I think it's a lot less intuitive if a field can be a string or a mapping. It also makes it a lot harder (maybe impossible) to encode the expected structure of the configuration (in something like http://json-schema.org/ for example). If we were able to break backwards compatibility, and always do and build was never just a string, I think it would be nice. |
|
BTW, there is now a docker-py 1.1.0, so if you update the setup.py:install_requires and the requirements.txt , the tests should pass on docker 1.5+ |
|
Oh awesome, I'll give it a try today!!! |
|
It's not working for me, the container builds, but then exits on up will look into this more when I have time later, unless someone has an idea, thanks. |
|
This PR has been working well for me although I had to:
|
|
@loic you mean this line? compose/compose/cli/docker_client.py Line 35 in 4869fed Sorry for the close PR phone miss clicked.. Not sure what was going on, I had to remove my docker images and it seems fine now after rebuilding. |
|
Needs rebasing now that #1100 is merged. |
|
Rebased with upstream/master |
|
@KyleJamesWalker yes that was the line. Maybe consider squashing into a single commit? |
|
@loic Didn't know about squashing, hope that did it correctly. Thanks for the tip! |
|
Looks like all the errors are:
@aanand do I need to do anything else to get this merged in? |
|
If you rebase on master, then remove all the Docker versions apart from 1.6.0-rc2, it should fix the build. |
|
@aanand Rebased. Also amended commit to include a properly formatted DCO marker. |
|
Dockerfile still needs updating to remove Docker versions 1.3.3, 1.4.1 and 1.5.0. |
|
+1 I can't wait for this |
|
+1 this is gonna be awesome |
|
+1 cool feature - we are waiting for it |
|
requests 2.6.1 is out, so try updating |
|
Thanks, I'll give it a try when I make it into work today! |
Signed-off-by: Kyle James Walker <[email protected]>
|
WOOOO!!!!!!!!!!! |
|
👍 |
|
LGTM |
|
LGTM! |
Support alternate Dockerfile name.
|
|
|
Thank you! |
|
👍 |
|
Great to see this finally in!
|
|
👍👍👍👍👍👍👍👍👍👍👍 |
|
Nevermind, after digging more I see that this will be part of Docker Compose v1.3.0 which is slated to be released on June 26, 2015. |
|
Oh you beat me to it, can't wait for it in the main release! :P |
|
Docker Compose v1.3.0rc2 Homebrew Formula: https://gist.github.com/joost/20934650f68dd561fc35 |
|
Please note that Homebrew isn't an official installation method! Head here to install from binaries or pip: https://github.com/docker/compose/releases/tag/1.3.0rc2 |
19 participants
Add support for compose support Docker's
-f/--fileoption from within your docker-compose.yml file.I quickly threw this in, and it seems to work for me, please let me know if anything else is needed to get this into master.