----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20674/#review41371 -----------------------------------------------------------
Just a few comments on upgrading and handling when the new optional 'extract' field is missing. src/launcher/fetcher.cpp <https://reviews.apache.org/r/20674/#comment74793> What if the [X|N] is left off the uri in MESOS_EXECUTOR_URIS (for those upgrading from old URI lists)? For substr, "If [pos] is equal to the string length, the function returns an empty string." and empty string != "X", so old URIs will not be extracted when they previously were. src/slave/containerizer/mesos_containerizer.cpp <https://reviews.apache.org/r/20674/#comment74789> uri.has_extract() && uri.extract() ? src/tests/containerizer_tests.cpp <https://reviews.apache.org/r/20674/#comment74792> Can you also add a test for NoExtract if uri.set_extract is left off, since it's optional? - Adam B On April 24, 2014, 11:53 a.m., Tom Arnfeld wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20674/ > ----------------------------------------------------------- > > (Updated April 24, 2014, 11:53 a.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 > >
