You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Till Toenshoff <to...@me.com> on 2014/04/24 18:15:54 UTC
Review Request 20668: Updated containerizer.proto's Launch message,
added Destroy-, Wait- and Usage-messages.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20668/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Repository: mesos-git
Description
-------
Updated Launch message to reflect latest commits on the TaskInfo related containerizer Launch call/s. Renamed the Launch message attribute "mesos_executor_path" to "mesos_libexec_directory" which will now hold the libexec-path instead of the mesos-executor path directly - thus being usable for other mesos provided executables (e.g. mesos-usage).
Added Destroy, Wait and Usage messages to allow extending those messages in the future, where needed.
Diffs
-----
include/mesos/containerizer/containerizer.proto 6ecd82e
Diff: https://reviews.apache.org/r/20668/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 20668: Updated containerizer.proto's Launch message,
added Destroy-, Wait- and Usage-messages.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20668/
-----------------------------------------------------------
(Updated April 24, 2014, 7:08 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Updated commit message to reflect latest changes.
Repository: mesos-git
Description (updated)
-------
Updated Launch message to reflect latest commits on the TaskInfo related containerizer Launch call/s.
Added Destroy, Wait and Usage messages to allow extending those messages in the future, where needed.
Diffs
-----
include/mesos/containerizer/containerizer.proto 6ecd82e
Diff: https://reviews.apache.org/r/20668/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 20668: Updated containerizer.proto's Launch message,
added Destroy-, Wait- and Usage-messages.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20668/
-----------------------------------------------------------
(Updated April 24, 2014, 7:07 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Addressed comments.
Repository: mesos-git
Description
-------
Updated Launch message to reflect latest commits on the TaskInfo related containerizer Launch call/s. Renamed the Launch message attribute "mesos_executor_path" to "mesos_libexec_directory" which will now hold the libexec-path instead of the mesos-executor path directly - thus being usable for other mesos provided executables (e.g. mesos-usage).
Added Destroy, Wait and Usage messages to allow extending those messages in the future, where needed.
Diffs (updated)
-----
include/mesos/containerizer/containerizer.proto 6ecd82e
Diff: https://reviews.apache.org/r/20668/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 20668: Updated containerizer.proto's Launch message,
added Destroy-, Wait- and Usage-messages.
Posted by Till Toenshoff <to...@me.com>.
> On April 24, 2014, 5:55 p.m., Benjamin Hindman wrote:
> > include/mesos/containerizer/containerizer.proto, line 40
> > <https://reviews.apache.org/r/20668/diff/1/?file=567777#file567777line40>
> >
> > For some external containerizer programs the Mesos libexec directory will also be valuable for other calls, such as Usage (to find mesos-usage) so rather than adding this to every protobuf let's make the ExternalContainerizer put this in the environment as MESOS_LIBEXEC_DIRECTORY just as it does for things like MESOS_DEFAULT_CONTAINER_IMAGE.
Hah, that is exactly what I had in mind and wanted to discuss with you :) .... awesome, yes, let's do that.
> On April 24, 2014, 5:55 p.m., Benjamin Hindman wrote:
> > include/mesos/containerizer/containerizer.proto, lines 32-33
> > <https://reviews.apache.org/r/20668/diff/1/?file=567777#file567777line32>
> >
> > I think we want s/task/task_info/ and s/executor/executor_info/.
Aye
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20668/#review41332
-----------------------------------------------------------
On April 24, 2014, 5:22 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20668/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 5:22 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Updated Launch message to reflect latest commits on the TaskInfo related containerizer Launch call/s. Renamed the Launch message attribute "mesos_executor_path" to "mesos_libexec_directory" which will now hold the libexec-path instead of the mesos-executor path directly - thus being usable for other mesos provided executables (e.g. mesos-usage).
>
> Added Destroy, Wait and Usage messages to allow extending those messages in the future, where needed.
>
>
> Diffs
> -----
>
> include/mesos/containerizer/containerizer.proto 6ecd82e
>
> Diff: https://reviews.apache.org/r/20668/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 20668: Updated containerizer.proto's Launch message,
added Destroy-, Wait- and Usage-messages.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20668/#review41332
-----------------------------------------------------------
Ship it!
include/mesos/containerizer/containerizer.proto
<https://reviews.apache.org/r/20668/#comment74753>
I think we want s/task/task_info/ and s/executor/executor_info/.
include/mesos/containerizer/containerizer.proto
<https://reviews.apache.org/r/20668/#comment74766>
Is this field actually being used? It seems like it existed originally because we weren't passing ExecutorInfo. Given the eventual API if only a TaskInfo is passed we won't have the FrameworkID (unless we also passed it in Containerizer::launch or added it to TaskInfo) and if we pass an ExecutorInfo it will have an optional FrameworkID, which makes this one seem redundant. I'd rather we didn't add it until we really needed it (so if deimos is somehow using it please correct me) especially since maybe the right thing to do is make it optional in TaskInfo?
include/mesos/containerizer/containerizer.proto
<https://reviews.apache.org/r/20668/#comment74755>
For some external containerizer programs the Mesos libexec directory will also be valuable for other calls, such as Usage (to find mesos-usage) so rather than adding this to every protobuf let's make the ExternalContainerizer put this in the environment as MESOS_LIBEXEC_DIRECTORY just as it does for things like MESOS_DEFAULT_CONTAINER_IMAGE.
- Benjamin Hindman
On April 24, 2014, 5:22 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20668/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 5:22 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Updated Launch message to reflect latest commits on the TaskInfo related containerizer Launch call/s. Renamed the Launch message attribute "mesos_executor_path" to "mesos_libexec_directory" which will now hold the libexec-path instead of the mesos-executor path directly - thus being usable for other mesos provided executables (e.g. mesos-usage).
>
> Added Destroy, Wait and Usage messages to allow extending those messages in the future, where needed.
>
>
> Diffs
> -----
>
> include/mesos/containerizer/containerizer.proto 6ecd82e
>
> Diff: https://reviews.apache.org/r/20668/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 20668: Updated containerizer.proto's Launch message,
added Destroy-, Wait- and Usage-messages.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20668/
-----------------------------------------------------------
(Updated April 24, 2014, 5:22 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
fixed comment lenghts
Repository: mesos-git
Description
-------
Updated Launch message to reflect latest commits on the TaskInfo related containerizer Launch call/s. Renamed the Launch message attribute "mesos_executor_path" to "mesos_libexec_directory" which will now hold the libexec-path instead of the mesos-executor path directly - thus being usable for other mesos provided executables (e.g. mesos-usage).
Added Destroy, Wait and Usage messages to allow extending those messages in the future, where needed.
Diffs (updated)
-----
include/mesos/containerizer/containerizer.proto 6ecd82e
Diff: https://reviews.apache.org/r/20668/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 20668: Updated containerizer.proto's Launch message,
added Destroy-, Wait- and Usage-messages.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20668/
-----------------------------------------------------------
(Updated April 24, 2014, 5:13 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Repository: mesos-git
Description
-------
Updated Launch message to reflect latest commits on the TaskInfo related containerizer Launch call/s. Renamed the Launch message attribute "mesos_executor_path" to "mesos_libexec_directory" which will now hold the libexec-path instead of the mesos-executor path directly - thus being usable for other mesos provided executables (e.g. mesos-usage).
Added Destroy, Wait and Usage messages to allow extending those messages in the future, where needed.
Diffs (updated)
-----
include/mesos/containerizer/containerizer.proto 6ecd82e
Diff: https://reviews.apache.org/r/20668/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 20668: Updated containerizer.proto's Launch message,
added Destroy-, Wait- and Usage-messages.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20668/
-----------------------------------------------------------
(Updated April 24, 2014, 5:09 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Updated comments wording.
Repository: mesos-git
Description
-------
Updated Launch message to reflect latest commits on the TaskInfo related containerizer Launch call/s. Renamed the Launch message attribute "mesos_executor_path" to "mesos_libexec_directory" which will now hold the libexec-path instead of the mesos-executor path directly - thus being usable for other mesos provided executables (e.g. mesos-usage).
Added Destroy, Wait and Usage messages to allow extending those messages in the future, where needed.
Diffs (updated)
-----
include/mesos/containerizer/containerizer.proto 6ecd82e
Diff: https://reviews.apache.org/r/20668/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 20668: Updated containerizer.proto's Launch message,
added Destroy-, Wait- and Usage-messages.
Posted by Till Toenshoff <to...@me.com>.
> On April 24, 2014, 4:53 p.m., Niklas Nielsen wrote:
> > include/mesos/containerizer/containerizer.proto, line 54
> > <https://reviews.apache.org/r/20668/diff/1/?file=567777#file567777line54>
> >
> > Is it just me or is the comments a bit verbose and repetitive? :)
> >
> > How about something like,
> >
> > /* Encodes wait command for external containerizer program */
> >
> > ?
> >
> > Containerized wait/destroy/usage doesn't add much context.
It was an earlier comment suggesting to be descriptive on the source and destination of these messages. Your suggestion however still covers that and I really like it better -> hence will adapt accordingly.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20668/#review41325
-----------------------------------------------------------
On April 24, 2014, 4:15 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20668/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 4:15 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Updated Launch message to reflect latest commits on the TaskInfo related containerizer Launch call/s. Renamed the Launch message attribute "mesos_executor_path" to "mesos_libexec_directory" which will now hold the libexec-path instead of the mesos-executor path directly - thus being usable for other mesos provided executables (e.g. mesos-usage).
>
> Added Destroy, Wait and Usage messages to allow extending those messages in the future, where needed.
>
>
> Diffs
> -----
>
> include/mesos/containerizer/containerizer.proto 6ecd82e
>
> Diff: https://reviews.apache.org/r/20668/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 20668: Updated containerizer.proto's Launch message,
added Destroy-, Wait- and Usage-messages.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20668/#review41325
-----------------------------------------------------------
Ship it!
include/mesos/containerizer/containerizer.proto
<https://reviews.apache.org/r/20668/#comment74750>
Is it just me or is the comments a bit verbose and repetitive? :)
How about something like,
/* Encodes wait command for external containerizer program */
?
Containerized wait/destroy/usage doesn't add much context.
- Niklas Nielsen
On April 24, 2014, 9:15 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20668/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 9:15 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Updated Launch message to reflect latest commits on the TaskInfo related containerizer Launch call/s. Renamed the Launch message attribute "mesos_executor_path" to "mesos_libexec_directory" which will now hold the libexec-path instead of the mesos-executor path directly - thus being usable for other mesos provided executables (e.g. mesos-usage).
>
> Added Destroy, Wait and Usage messages to allow extending those messages in the future, where needed.
>
>
> Diffs
> -----
>
> include/mesos/containerizer/containerizer.proto 6ecd82e
>
> Diff: https://reviews.apache.org/r/20668/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>