Skip to content

container: introduce per-instance default options#61

Merged
apocas merged 1 commit intoapocas:masterfrom
srijs:feature/container-default-options
Apr 29, 2014
Merged

container: introduce per-instance default options#61
apocas merged 1 commit intoapocas:masterfrom
srijs:feature/container-default-options

Conversation

@srijs
Copy link
Contributor

@srijs srijs commented Apr 29, 2014

In order to be able to set default options for common operations on a container,
introduce a default options object per operation that can be overwritten from the outside.

Together with #60 this resolves #57, because you can now do:

docker.run(..., function (...) {
  ...
}).on('container', function (container) {
  container.defaultOptions.start.Binds = ["/tmp:/tmp:rw"];
});

In order to be able to set default options for common operations on a container,
introduce a default options object per operation that can be overwritten from the outside.

Together with apocas#60 this resolves apocas#57, because you can now do:

docker.run(..., function (...) {
  ...
}).on('container', function (container) {
  container.defaultOptions.start.Binds = ["/tmp:/tmp:rw"];
});
@apocas
Copy link
Owner

apocas commented Apr 29, 2014

I like this pattern/combo.

But I think I see a problem, imagine that someone wants to define a defaultOption for "attach" instead of "start". This may asynchronous fail. (with "start" it will probably work, since "attach" is done first giving it time)

https://github.com/apocas/dockerode/blob/master/lib/docker.js#L258

@srijs
Copy link
Contributor Author

srijs commented Apr 29, 2014

There'll be no problem with that, because semantically all event handlers for the container event will be processed before continuing to the attach call. (see this for reference)

@apocas
Copy link
Owner

apocas commented Apr 29, 2014

👍
I always avoided patterns like these thinking that way, just read a bit it will work indeed.

Want do drop that run->start example into readme.md?
Think this combo will be helpful for a lot of people using "run" :)

We should deprecate the current "options" parameter from run, since it doesn't give complete functionality and this pattern is way superior.

@apocas
Copy link
Owner

apocas commented Apr 29, 2014

Will add the example myself, since I'm prepping the next release right now and really wanna to have this in :)

apocas added a commit that referenced this pull request Apr 29, 2014
container: introduce per-instance default options
@apocas apocas merged commit 230a00a into apocas:master Apr 29, 2014
@srijs srijs deleted the feature/container-default-options branch April 29, 2014 21:56
@srijs
Copy link
Contributor Author

srijs commented Apr 29, 2014

Are you talking about the options parameter on docker.run? It's the only way to pass parameters for creating a container right now. So you shouldn't deprecate that.

@apocas
Copy link
Owner

apocas commented Apr 29, 2014

Duh you are right. Will express this clearly in the documentation.

@apocas
Copy link
Owner

apocas commented Apr 30, 2014

Published v1.3.0

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.

Bind Host Volume to Container on .run()

2 participants