Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Ryan Thomas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26709/ --- Review request for mesos. Bugs: MESOS-1925

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26709/#review56585 --- src/docker/docker.cpp

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Timothy Chen
On Oct. 14, 2014, 10:18 p.m., Timothy Chen wrote: src/docker/docker.cpp, line 472 https://reviews.apache.org/r/26709/diff/1/?file=721122#file721122line472 Since this method doesn't simple kill anymore, can you also refactor the method name? Good to provide some comments as well

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Ankur Chauhan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26709/#review56602 --- src/docker/docker.cpp

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Ryan Thomas
On Oct. 14, 2014, 11:39 p.m., Ankur Chauhan wrote: src/docker/docker.cpp, line 472 https://reviews.apache.org/r/26709/diff/1/?file=721122#file721122line472 A 30 second timeout seems pretty arbitrary. I would suggest allowing the user to specify this as a arg. Something on the

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Ankur Chauhan
On Oct. 14, 2014, 11:39 p.m., Ankur Chauhan wrote: src/docker/docker.cpp, line 472 https://reviews.apache.org/r/26709/diff/1/?file=721122#file721122line472 A 30 second timeout seems pretty arbitrary. I would suggest allowing the user to specify this as a arg. Something on the

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Ryan Thomas
On Oct. 14, 2014, 11:39 p.m., Ankur Chauhan wrote: src/docker/docker.cpp, line 472 https://reviews.apache.org/r/26709/diff/1/?file=721122#file721122line472 A 30 second timeout seems pretty arbitrary. I would suggest allowing the user to specify this as a arg. Something on the

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Ankur Chauhan
On Oct. 14, 2014, 11:39 p.m., Ankur Chauhan wrote: src/docker/docker.cpp, line 472 https://reviews.apache.org/r/26709/diff/1/?file=721122#file721122line472 A 30 second timeout seems pretty arbitrary. I would suggest allowing the user to specify this as a arg. Something on the

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26709/#review56611 --- src/docker/docker.cpp

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Ankur Chauhan
On Oct. 15, 2014, 12:15 a.m., Timothy Chen wrote: src/docker/docker.cpp, line 472 https://reviews.apache.org/r/26709/diff/1/?file=721122#file721122line472 I'm not sure what executor timeout you're referring to, but for example the executor signal escalation timeout although

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Timothy Chen
On Oct. 15, 2014, 12:15 a.m., Timothy Chen wrote: src/docker/docker.cpp, line 472 https://reviews.apache.org/r/26709/diff/1/?file=721122#file721122line472 I'm not sure what executor timeout you're referring to, but for example the executor signal escalation timeout although

Re: Review Request 26709: Change from kill to stop for docker

2014-10-14 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26709/#review56646 --- Patch looks great! Reviews applied: [26709] All tests passed. -