-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20674/#review41439
-----------------------------------------------------------
Ship it!
This is great Tom, thanks for contributing!
You commented on IRC that the format of MESOS_EXECUTOR_URIS is indeed a bit
bizarre. It's an artifact of not having JSON/protobuf parsing when this code
was originally written. Now that we _can_ parse JSON/protobuf, however, we've
been meaning to clean this up. That is, we should just send JSON in the
environment (and to simplify we might as well send the entire CommandInfo
rather than pulling out the URIs). This would boil down to just the following
in the containerizer:
environment["MESOS_COMMAND_INFO"] = stringify(JSON::Protobuf(commandInfo));
And something along the lines of the following in the fetcher:
Try<JSON::Object> parse =
JSON::parse<JSON::Object>(os::getenv("MESOS_COMMAND_INFO"));
if (parse.isError()) {
...
}
Try<CommandInfo> commandInfo = protobuf::parse<CommandInfo>(parse.get());
if (commandInfo.isError()) {
...
}
foreach (const CommandInfo::URI& uri, commandInfo.get().uris()) {
...
}
I'm happy to accept your patch in it's current state and have you (or someone
else) follow up with a subsequent patch for doing the JSON/protobuf parsing too.
src/tests/containerizer_tests.cpp
<https://reviews.apache.org/r/20674/#comment74870>
We try to put two newlines between each of our tests (below too). Thanks!
- Benjamin Hindman
On April 24, 2014, 6:53 p.m., Tom Arnfeld wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20674/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 6:53 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ian Downes,
> Niklas Nielsen, and Vinod Kone.
>
>
> Bugs: MESOS-1241
> https://issues.apache.org/jira/browse/MESOS-1241
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> This is a patch that includes a new `extract` boolean value on
> CommandInfo.URI to toggle the auto-extraction of compressed archives when
> shipping URIs with an executor or task.
>
> It's worth noting that this changes the behaviour of the MESOS_EXECUTOR_URI
> environment variable used. I've added some comments which outlines the
> expected format, instead of (e.g) hdfs://foobar/testing.tar.gz+0 (1
> signifying it's not executable) there is now an extra flag on the end (X or
> N) defining whether to auto-extract or not, resulting in
> hdfs://foobar/testing.tar.gz+0X by default.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 37f8a7f
> src/launcher/fetcher.cpp 98ebc2e
> src/slave/containerizer/mesos_containerizer.cpp 722f3fe
> src/tests/containerizer_tests.cpp 6c48146
>
> Diff: https://reviews.apache.org/r/20674/diff/
>
>
> Testing
> -------
>
> - Tested with a single-slave mesos cluster (Mac OSX 10.9)
> - Verified `make check` after adding extra unit tests
>
>
> Thanks,
>
> Tom Arnfeld
>
>
