You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Josef Cacek <no...@github.com> on 2016/02/22 17:31:54 UTC
[jclouds-labs] [JCLOUDS-1002] provide access to Docker container
Config object in the Node template options (#240)
JIRA: https://issues.apache.org/jira/browse/JCLOUDS-1002
Provide fine-grained control of container configuration to `DockerTemplateOptions`. If the `configBuilder` is set, then it's used and other members introduced in `DockerTemplateOptions` are ignored.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/240
-- Commit Summary --
* [JCLOUDS-1002] provide access to Docker container Config object in the Node template options
-- File Changes --
M docker/src/main/java/org/jclouds/docker/compute/options/DockerTemplateOptions.java (87)
M docker/src/main/java/org/jclouds/docker/compute/strategy/DockerComputeServiceAdapter.java (123)
M docker/src/test/java/org/jclouds/docker/compute/SshToCustomPortLiveTest.java (30)
M docker/src/test/java/org/jclouds/docker/compute/options/DockerTemplateOptionsTest.java (18)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/240.patch
https://github.com/jclouds/jclouds-labs/pull/240.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/240
Re: [jclouds-labs] [JCLOUDS-1002] provide access to Docker container
Config object in the Node template options (#240)
Posted by Ignasi Barrera <no...@github.com>.
Pushed to master as [4ff9889f](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/4ff9889f). Thanks @kwart!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/240#issuecomment-193990253
Re: [jclouds-labs] [JCLOUDS-1002] provide access to Docker container
Config object in the Node template options (#240)
Posted by Ignasi Barrera <no...@github.com>.
Just a minor comment. Change lgtm. @andreaturli good to merge?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/240#issuecomment-189219393
Re: [jclouds-labs] [JCLOUDS-1002] provide access to Docker container
Config object in the Node template options (#240)
Posted by Josef Cacek <no...@github.com>.
I forgot to remove the trailing call for setting the image in `configBuilder(Config.Builder)` after I changed its value on the input instance. :)
It's fixed now.
@nacx Thanks for the review.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/240#issuecomment-189257284
Re: [jclouds-labs] [JCLOUDS-1002] provide access to Docker container
Config object in the Node template options (#240)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -235,6 +281,18 @@ public DockerTemplateOptions extraHosts(Map<String, String> extraHosts) {
> return this;
> }
>
> + /**
> + * This method sets Config.Builder configuration object, which can be used as a replacement for all the other settings from this class.
> + * If a not-<code>null</code> instance is provided, then it the compu
> + * @param configBuilder Config.Builder instance. This instance can be changed in this method!
> + */
> + public DockerTemplateOptions configBuilder(Config.Builder configBuilder) {
> + this.configBuilder = configBuilder != null
> + ? Config.builder().fromConfig(configBuilder.image(NO_IMAGE).build()).image(NO_IMAGE)
Why setting the `NO_IMAGE` twice?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/240/files#r54228829
Re: [jclouds-labs] [JCLOUDS-1002] provide access to Docker container
Config object in the Node template options (#240)
Posted by Josef Cacek <no...@github.com>.
I tried also the way witha new template options class, but I've failed on `TemplateBuilderImpl.build()` which copies properties to new instance of the old options class. So I've put the `Config.Builder` member variable to the old class (`DockerTemplateOptions`) at the end.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/240#issuecomment-187258089
Re: [jclouds-labs] [JCLOUDS-1002] provide access to Docker container
Config object in the Node template options (#240)
Posted by Ignasi Barrera <no...@github.com>.
Closed #240.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/240#event-582431750
Re: [jclouds-labs] [JCLOUDS-1002] provide access to Docker container
Config object in the Node template options (#240)
Posted by Andrea Turli <no...@github.com>.
I think it gives a better control on the container spec and it is a semplification of the options as well. I agree with @nacx - *TemplateOptions couple with low-level features offered by the specific cloud so they dont need to be portable.
@gkrvlt any other thoughts?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/240#issuecomment-187373575