Add restart option to Fig. #594

paulRbr · 2014-10-28T15:56:49Z

Hello!

I simply find this project really good, and ended up needing the restart option (Related to #478).

Since Docker 1.2 there is a restart option on docker running containers to add restart policies. I think fig options should be able to set restart policies to it's orchestrated containers.
This PR adds the option in Fig.

Let me know if there it's all ok?

bfirsh · 2014-10-28T16:07:27Z

Hello! Thanks for this.

It looks like the option to start() is restart_policy not restart. I think I prefer restart in fig.yml though - it matches the docker run option.

paulRbr · 2014-10-28T22:33:59Z

Hi @bfirsh!

Indeed it's restart_policy I hadn't check with the docker-py api.
Got the modification done and left the option name to match the docker run one, hope to see this in the next release so we can use it in our project.

Cheers!

thaJeztah · 2014-10-28T22:42:41Z

👍 @bfirsh any chance this could make it in the 1.0.1 release? Will that be at the same time as the Docker 1.3.1?

bfirsh · 2014-10-29T12:08:59Z

I think we'll release 1.0.1 as a quick bugfix release as soon as dockerpty is ready. We can then put this into a larger 1.1 feature release.

thaJeztah · 2014-10-29T13:41:31Z

Guess I'll have to be patient then, or look into compiling fig myself :)

paulRbr · 2014-10-29T14:49:44Z

I have to admit that I would have loved to see this in 1.0.1 too.

If we consider that Fig should be in sync with the docker releases, then this is a bug fix and not a new feature 😄

bfirsh · 2014-10-29T15:01:13Z

Fig isn't in sync with Docker.

bfirsh · 2014-10-29T15:02:33Z

It might make it into 1.0.1, but it's not going to be a blocker for a bugfix release, that's all.

I would quite like to make 1.0.x bug fix releases and 1.x.0 feature releases, as per semantic versioning (and how Docker does it).

paulRbr · 2014-10-29T15:04:58Z

Yes I thought so and agree clearly with your point.

Obviously this is a new feature for Fig. Thus should be in a 1.x.0 release.

Thanks for your work !
Em 29/10/2014 16:01, "Ben Firshman" notifications@github.com escreveu:

Fig isn't in sync with Docker.

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

bfirsh · 2014-10-29T15:10:16Z

Though that's not to disregard your PR. I really want this too. ;)

Currently figuring out how to get our CI system to run the damn test suite. The tests run fine locally, so I think this is okay.

bfirsh · 2014-10-29T15:12:09Z

fig/service.py

+    if len(parts) == 2:
+        name, max_retry_count = parts
+    else:
+        name, = parts


Stray comma here

Because I don't want the List but the only element inside

>>> a, = ['hello'] >>> a 'hello' >>> a = ['hello'] >>> a ['hello']

name = parts[0] is much clearer. ;) Not a big deal.

Fair enough :)

paulRbr · 2014-10-29T15:29:00Z

I pushed force the first commit and pushed normally the second one, hope this forces the tests to run

bfirsh · 2014-10-30T10:22:49Z

That worked. https://app.wercker.com/#build/5451078547c2f0c812105681 Looks like commit statuses are broken.

Thanks!

paulRbr · 2014-10-30T10:25:44Z

It seems the build was launch on the main changes (cc003a4) but not on my second commit of documentation (f457057).

bfirsh · 2014-10-30T10:26:51Z

Oh yeah. Weird.

paulRbr · 2014-11-07T14:22:37Z

@bfirsh any chance to get this merged in?

dnephin · 2014-11-07T19:18:06Z

LGTM

dnephin · 2014-11-07T19:21:13Z

I'm not seeing a way to force the build, but @popox if you git commit --amend and make a whitespace change to the comment it should trigger another build I think.

paulRbr · 2014-11-08T09:57:56Z

@dnephin commit amending didn't do the trick, so I just squashed and rebased on master.

Signed-off-by: Paul Bonaud <paul@bonaud.fr>

paulRbr · 2014-11-08T11:36:59Z

The build is launched on my fork but not on the PR.

dnephin · 2014-11-08T20:17:03Z

Cool, works for me, looks good

thaJeztah · 2014-11-22T12:43:29Z

docs/yml.md

@@ -142,7 +142,7 @@ dns:
  - 9.9.9.9
 ```

-### working\_dir, entrypoint, user, hostname, domainname, mem\_limit, privileged
+### working\_dir, entrypoint, user, hostname, domainname, mem\_limit, privileged, restart

 Each of these is a single value, analogous to its [docker run](https://docs.docker.com/reference/run/) counterpart.


Is this still true? Thinking of :max_retry here (or is that to be considered a "single" value? idk)

thaJeztah · 2014-11-22T12:51:16Z

@dnephin is there anything needed to move this PR forward? I added two small suggestions, but apart from that, wondering if this is ready to merge?

docteurklein · 2014-11-22T14:25:20Z

would be great to see this merged :)

dnephin · 2014-12-02T05:21:33Z

LGTM

ping @bfirsh

bfirsh · 2014-12-08T16:43:25Z

LGTM

Add restart option to Fig.

bfirsh · 2014-12-08T16:43:59Z

Thanks @popox!

cgcgbcbc · 2014-12-18T05:56:44Z

I wonder when this feature will be published? along with 1.1.0 milestone? I noticed that there is no due date for 1.1.0 milestone.. so is there a current walk-around besides using the HEAD version?

bsideup · 2014-12-22T10:57:18Z

+1

Neat feature and it would be good to see it in release version ASAP

blackrosezy · 2014-12-30T10:02:45Z

+1

Add restart option to Fig. Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>

paulRbr force-pushed the restart-option branch 10 times, most recently from 9d0ea1c to 7b91175 Compare October 28, 2014 22:12

bfirsh reviewed Oct 29, 2014
View reviewed changes

paulRbr force-pushed the restart-option branch from 7b91175 to cc003a4 Compare October 29, 2014 15:28

paulRbr force-pushed the restart-option branch from f457057 to 8bda279 Compare November 8, 2014 09:33

paulRbr force-pushed the restart-option branch 2 times, most recently from 35793f1 to b9bd478 Compare November 8, 2014 09:56

Add restart option to Fig. Related to docker#478

04da6b0

Signed-off-by: Paul Bonaud <paul@bonaud.fr>

paulRbr force-pushed the restart-option branch from b9bd478 to 04da6b0 Compare November 8, 2014 11:20

dnephin added the enhancement label Nov 8, 2014

dnephin mentioned this pull request Nov 10, 2014

Docker 1.2 - New functionality #434

Closed

thaJeztah reviewed Nov 22, 2014
View reviewed changes

ghost mentioned this pull request Nov 26, 2014

docker restart with fig #678

Closed

dnephin added this to the 1.1.0 milestone Dec 3, 2014

dnephin mentioned this pull request Dec 7, 2014

Added support for restart_policy parameter #712

Closed

bfirsh added a commit that referenced this pull request Dec 8, 2014

Merge pull request #594 from popox/restart-option

e9d946b

Add restart option to Fig.

bfirsh merged commit e9d946b into docker:master Dec 8, 2014

paulRbr deleted the restart-option branch December 8, 2014 17:44

bfirsh mentioned this pull request Dec 9, 2014

restart option #478

Closed

thaJeztah mentioned this pull request Jan 4, 2015

How to start on reboot on Ubuntu? #796

Closed

yuval-k pushed a commit to yuval-k/compose that referenced this pull request Apr 10, 2015

Merge pull request docker#594 from popox/restart-option

bddb7e3

Add restart option to Fig. Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>

bfirsh added kind/enhancement and removed enhancement labels Jul 16, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add restart option to Fig. #594

Add restart option to Fig. #594

paulRbr commented Oct 28, 2014

bfirsh commented Oct 28, 2014

paulRbr commented Oct 28, 2014

thaJeztah commented Oct 28, 2014

bfirsh commented Oct 29, 2014

thaJeztah commented Oct 29, 2014

paulRbr commented Oct 29, 2014

bfirsh commented Oct 29, 2014

bfirsh commented Oct 29, 2014

paulRbr commented Oct 29, 2014

bfirsh commented Oct 29, 2014

bfirsh Oct 29, 2014

paulRbr Oct 29, 2014

bfirsh Oct 30, 2014

bfirsh Oct 30, 2014

paulRbr Oct 30, 2014

paulRbr commented Oct 29, 2014

bfirsh commented Oct 30, 2014

paulRbr commented Oct 30, 2014

bfirsh commented Oct 30, 2014

paulRbr commented Nov 7, 2014

dnephin commented Nov 7, 2014

dnephin commented Nov 7, 2014

paulRbr commented Nov 8, 2014

paulRbr commented Nov 8, 2014

dnephin commented Nov 8, 2014

thaJeztah Nov 22, 2014

thaJeztah commented Nov 22, 2014

docteurklein commented Nov 22, 2014

dnephin commented Dec 2, 2014

bfirsh commented Dec 8, 2014

bfirsh commented Dec 8, 2014

cgcgbcbc commented Dec 18, 2014

bsideup commented Dec 22, 2014

blackrosezy commented Dec 30, 2014

Add restart option to Fig. #594

Add restart option to Fig. #594

Conversation

paulRbr commented Oct 28, 2014

bfirsh commented Oct 28, 2014

paulRbr commented Oct 28, 2014

thaJeztah commented Oct 28, 2014

bfirsh commented Oct 29, 2014

thaJeztah commented Oct 29, 2014

paulRbr commented Oct 29, 2014

bfirsh commented Oct 29, 2014

bfirsh commented Oct 29, 2014

paulRbr commented Oct 29, 2014

bfirsh commented Oct 29, 2014

bfirsh Oct 29, 2014

Choose a reason for hiding this comment

paulRbr Oct 29, 2014

Choose a reason for hiding this comment

bfirsh Oct 30, 2014

Choose a reason for hiding this comment

bfirsh Oct 30, 2014

Choose a reason for hiding this comment

paulRbr Oct 30, 2014

Choose a reason for hiding this comment

paulRbr commented Oct 29, 2014

bfirsh commented Oct 30, 2014

paulRbr commented Oct 30, 2014

bfirsh commented Oct 30, 2014

paulRbr commented Nov 7, 2014

dnephin commented Nov 7, 2014

dnephin commented Nov 7, 2014

paulRbr commented Nov 8, 2014

paulRbr commented Nov 8, 2014

dnephin commented Nov 8, 2014

thaJeztah Nov 22, 2014

Choose a reason for hiding this comment

thaJeztah commented Nov 22, 2014

docteurklein commented Nov 22, 2014

dnephin commented Dec 2, 2014

bfirsh commented Dec 8, 2014

bfirsh commented Dec 8, 2014

cgcgbcbc commented Dec 18, 2014

bsideup commented Dec 22, 2014

blackrosezy commented Dec 30, 2014