implementation of initial_scale service option (issue #116)

[ekroymann]

Implementation of the initial_scale service option discussed in option (issue #116). Initial scale is applied whenever the containers for a service are newly created, ie. on "fig up" or "fig up --no-recreate".

Signed-off-by: Emil Kroymann emil.kroymann@gmail.com

[dnephin]

I think this validation should happen much easier. For one, it's duplicated in two places right now, which is not good.

This feels more like configuration validate code, which right now is done in the constructor for Service. You'll have to change can_be_scaled() slightly to accept the options as a param (instead of using self.options), but that should be fine (you could even move the function off the class if you wanted to, since self wouldn't be used any more).

I also noticed there's a bug in can_be_scaled() that will return True if the external interface is specified but no external port (it should really return False in that case) , but I guess that can be fixed later if you wanted to leave it for now.

[ekroymann]

I refactored the PR to match the suggestions of dnephin. I also added some unit tests for the feature. Along the way, I noticed, that a different test case was patching the Container class globally, which can get (and does, in this case) in the way of other test cases using Container.

[dnephin]

LGTM, but please do squash the commits into a single one.

[bfirsh]

@EmilTH Cool! Thanks! Needs rebasing though, I'm afraid. :(

[ekroymann]

@bfirsh OK, rebased onto docker/fig master.

[aanand]

Looks like it needs another rebase, sorry. It could also do with some documentation in yml.md.

[ekroymann]

I did the rebase and also added the documentation. Don't know, why the tests fail, but this seems to be a problem with the build setup.

[dnephin]

I reran the build, it ran this time. There is just one error from flake8 about whitespace

[ekroymann]

I fixed the flake8 inconsistency and also made sure all tests pass.
Continous integration build failed again, because of problem with build
setup.

2015-01-04 19:45 GMT+01:00 Daniel Nephin notifications@github.com:

I reran the build, it ran this time. There is just one error from flake8
about whitespace

—
Reply to this email directly or view it on GitHub
#630 (comment).

[aanand]

Just an update: I think this PR is good, but I'm holding off on an LGTM because I've been thinking about configuration a lot and I'm worried about introducing stuff into service definition that isn't focused on application structure.

I do think that it should be possible to define an initial scale for each service in a version-controlled configuration file, so that your first fig up brings up the right number of each service. I'm just not sure that initial_scale should live alongside container configuration options, or even in the same file.

For context, there are a handful of other concerns relating to the structure and content of fig.yml that I'm thinking about:

• non-portable configuration such as volume mount paths and links to external containers
• scheduling hints for Swarm
• paths to other fig.ymls (once something like Include external config #758 is finished)

Scaling directives fit in this category too, where there seems to be a boundary between an abstract, portable app definition and concrete, host-specific configuration. We're going to need to make a decision about where the latter should live but I still want to be cautious about what we allow in fig.yml, because once stuff's in there it's very hard to get it out again.

(Of course, since we already allow you to specify volume host paths etc, it's likely that there'll be breaking changes whatever we decide, so maybe it doesn't matter.)

[eduherraiz]

+1 to this PR. I think an app could be need a scale factor > 1 for a container at initial configuration. Not for service definition, needed to the app structure.

[NoumanSaleem]

Any update on this?

[rysiekpl]

Ping?

[ekroymann]

Sorry, I got side tracked and did not have time to look into @aanand's proposal or resolve conflicts with master branch. If desired, I can put some work into this.

[rysiekpl]

Well, I just tested it and docker-compose data volumes seem to work with tianan/true image pretty well (and that's what I needed it for), so I retract my ping, sorry.

Still, does sound like a useful feature nonetheless.

[dnephin]

Thanks for the contribution! As @aanand mentioned here this is something we're considering, but it doesn't hit into the Compose file.

We're discussing a new file or directory for this type of configuration here: #745 (comment)

And there is a issue tracking this feature in #1661