Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-04-04 Thread Jiang Yan Xu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review127027
---




src/slave/containerizer/fetcher.cpp (line 150)


What does this check against?

`Path(filename).basename()` never returns an Error().


- Jiang Yan Xu


On March 20, 2016, 8:14 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 20, 2016, 8:14 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.hpp 
> bbdce88da6e41dbb88681bc9d604b00923033b3d 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-21 Thread Vinod Kone 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124618
---


Ship it!




Ship It!

- Vinod Kone


On March 21, 2016, 3:14 a.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 21, 2016, 3:14 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.hpp 
> bbdce88da6e41dbb88681bc9d604b00923033b3d 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-20 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124497
---



Patch looks great!

Reviews applied: [45046]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 21, 2016, 3:14 a.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 21, 2016, 3:14 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.hpp 
> bbdce88da6e41dbb88681bc9d604b00923033b3d 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-20 Thread Vinod Kone 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124480
---




src/tests/fetcher_tests.cpp (line 646)


s/fix/Fix/

Also, instead of here this TODO should be located at tests that do 
os::mktemp() (e.g., #431).



src/tests/fetcher_tests.cpp (line 648)


why is this directory named "cutom_gzip_filename" ?

also you are calling mkdtemp() but not using 'XXX' pattern? if you want to 
a well known directory name you can as well just use os::mkdir().



src/tests/fetcher_tests.cpp (line 651)


ditto. this should be os::touch() if you want a well known filename.



src/tests/fetcher_tests.cpp (line 687)


ditto. see above.



src/tests/fetcher_tests.cpp (lines 688 - 692)


ditto. see above.



CHANGELOG (line 1)


s/0.29.0/0.29.0 (WIP)/



CHANGELOG (line 7)


2 blank lines.


- Vinod Kone


On March 21, 2016, 12:51 a.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 21, 2016, 12:51 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.hpp 
> bbdce88da6e41dbb88681bc9d604b00923033b3d 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-20 Thread Michael Browning 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/
---

(Updated March 21, 2016, 12:51 a.m.)


Review request for mesos, Vinod Kone and Zhitao Li.


Changes
---

Rebase.


Bugs: MESOS-4735
https://issues.apache.org/jira/browse/MESOS-4735


Repository: mesos


Description
---

Created URI.filename to name fetched files in sandbox where appropriate.


Diffs (updated)
-

  CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
  docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
  include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
  include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
  src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
  src/slave/containerizer/fetcher.hpp bbdce88da6e41dbb88681bc9d604b00923033b3d 
  src/slave/containerizer/fetcher.cpp 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
  src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
  src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 

Diff: https://reviews.apache.org/r/45046/diff/


Testing
---

There are two paths by which a file gets fetched to the executor sandbox: the 
without-cache path, where the fetcher downloads the file directly from the 
specified URI, and the with-cache path, where it copies it from the cache. In 
both cases, we verify that the file is saved to the sandbox directory with the 
name specified by the "filename" field in the CommandInfo.URI proto.


Thanks,

Michael Browning

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-20 Thread Michael Browning 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/
---

(Updated March 21, 2016, 12:49 a.m.)


Review request for mesos, Vinod Kone and Zhitao Li.


Changes
---

Address comments and add API changes to CHANGELOG.


Bugs: MESOS-4735
https://issues.apache.org/jira/browse/MESOS-4735


Repository: mesos


Description
---

Created URI.filename to name fetched files in sandbox where appropriate.


Diffs (updated)
-

  3rdparty/libprocess/include/process/help.hpp 
3f859803f3096d3161fffb6485ce1ce3cb6b04bc 
  3rdparty/libprocess/src/help.cpp e046933975883fb046af8e51c31a742d983a6321 
  3rdparty/libprocess/src/logging.cpp bdbf716ee46a459f1004f0f4b72c23aae135f347 
  CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
  docs/endpoints/master/api/v1/scheduler.md 
538979977e07b484956be85b08b0dbc6cffcef01 
  docs/endpoints/master/create-volumes.md 
52ae3a8159bb7d26b63f5889ce3f122371afbdc4 
  docs/endpoints/master/destroy-volumes.md 
a0bb1e8d1ce42ab2b96518cd4d325bfc541ad4ff 
  docs/endpoints/master/flags.md e9038fece0d17432f839c1719beac65d1a351550 
  docs/endpoints/master/frameworks.md 15ecabfdd87faee6a1f24895088a3c08edf62b96 
  docs/endpoints/master/health.md dc1c1cd6f78874b9264279dd78c57aa572d048a4 
  docs/endpoints/master/machine/down.md 
f7e81417aa663e287a61124b351f2461b1c106e6 
  docs/endpoints/master/machine/up.md d96be726f5cf524937e3ebc5b046bf5f47886a1c 
  docs/endpoints/master/maintenance/schedule.md 
577e43c5faa961acb75f66e0db6729d7db9d88bf 
  docs/endpoints/master/maintenance/status.md 
249dae2706ccf30d79e0cd2f19821acd6ada74b7 
  docs/endpoints/master/observe.md 9ebd527788bbd2fd2dd833242c24ac561f826c30 
  docs/endpoints/master/quota.md b1e3462bd35c5acfd65323db5d0660277b110b11 
  docs/endpoints/master/redirect.md 833d9935b8fb038977fe08d826d73f03f2059bab 
  docs/endpoints/master/reserve.md 1d481b56d380d45218001513330b225ca4a0a55c 
  docs/endpoints/master/roles.json.md 01e265330c4c30de11301a2c4abbddfee10ac35d 
  docs/endpoints/master/roles.md 9c01d370a86d77eb95fcc157de70304e73f5f1e9 
  docs/endpoints/master/slaves.md 9e9fff7329d6bf05ae78997b1292657ff4dd10c3 
  docs/endpoints/master/state-summary.md 
a0da400b34c202602cb661a19c6199291da4ff4b 
  docs/endpoints/master/state.json.md 7998b80f29042f4b67565b03e2464b6e3a009335 
  docs/endpoints/master/state.md 59518d624a0c5e010f856c900a9d6512a35b21af 
  docs/endpoints/master/tasks.json.md 29bb9738b1519174eaeac4f4db423000fe48d0d3 
  docs/endpoints/master/tasks.md ab9bb09631400668adddeee363dd612c71e98e16 
  docs/endpoints/master/teardown.md 9b62b2656162b4160a9845152aecf7b75231fbae 
  docs/endpoints/master/unreserve.md b9282df659ebb6090ef49ef8fc0f01411cd53103 
  docs/endpoints/master/weights.md 7c4517343aa61bf6156fa54547690930051616a3 
  docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
  include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
  include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
  src/exec/exec.cpp 8f672602daf090dec032d2b684e407e5d043af9c 
  src/executor/executor.cpp 48b00504cac83aa0f4b1fc0b81480d3010edebae 
  src/launcher/executor.cpp 63eb8f1ae5510acdb5b504807a37c4e2e68ad6bd 
  src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
  src/master/http.cpp bfea8fa811c24d37b2d7a8109728e1f40217e02f 
  src/slave/containerizer/fetcher.hpp bbdce88da6e41dbb88681bc9d604b00923033b3d 
  src/slave/containerizer/fetcher.cpp 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
  src/slave/containerizer/mesos/containerizer.cpp 
ee7a265975323ca891114a286357c8e42901560c 
  src/slave/http.cpp 0117827d73bd1b4c96ef261d4573abf70f52c6b3 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
f3ca32b2d8b8ef9dcfa8f20d9ceaff48b6598a66 
  src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
  src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 

Diff: https://reviews.apache.org/r/45046/diff/


Testing
---

There are two paths by which a file gets fetched to the executor sandbox: the 
without-cache path, where the fetcher downloads the file directly from the 
specified URI, and the with-cache path, where it copies it from the cache. In 
both cases, we verify that the file is saved to the sandbox directory with the 
name specified by the "filename" field in the CommandInfo.URI proto.


Thanks,

Michael Browning

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-20 Thread Guangya Liu 


> On 三月 20, 2016, 6:36 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, lines 645-647
> > 
> >
> > I know you followed the existing pattern in this file but we shouldn't 
> > do mktemp() like this in the tests because if the test fails for whatever 
> > reason the temp file and the directory are leaked. 
> > 
> > The FetcherTest fixture inherits from TemporaryDirectoryTest so that 
> > the test runs in a temporary sandbox.
> > 
> > So you should do:
> > 
> > ```
> > Try dir = os::mkdtemp("./"));
> > ASSERT_SOME(dir);
> > 
> > Try path = os::mktemp(path::join(dir.get(), ""));
> > ASSERT_SOME(path);
> > ```
> > 
> > Feel free to fix the other tests in this file in a subsequent review or 
> > just add a TODO for now.
> 
> Michael Browning wrote:
> Happy to fix the other tests in this file. I think it'd be simpler to do 
> it in a separate review -- should I also create an issue for that?
> 
> Vinod Kone wrote:
> no need to create an issue.

What about using `path::join(os::getcwd(), "");` to simplify the test? I 
saw that the `ROOT_LocalPullerSimpleCommand` is using such format. 
https://github.com/apache/mesos/blob/master/src/tests/containerizer/provisioner_docker_tests.cpp#L340


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124456
---


On 三月 19, 2016, 10:03 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated 三月 19, 2016, 10:03 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-20 Thread Vinod Kone 


> On March 20, 2016, 6:36 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, lines 645-647
> > 
> >
> > I know you followed the existing pattern in this file but we shouldn't 
> > do mktemp() like this in the tests because if the test fails for whatever 
> > reason the temp file and the directory are leaked. 
> > 
> > The FetcherTest fixture inherits from TemporaryDirectoryTest so that 
> > the test runs in a temporary sandbox.
> > 
> > So you should do:
> > 
> > ```
> > Try dir = os::mkdtemp("./"));
> > ASSERT_SOME(dir);
> > 
> > Try path = os::mktemp(path::join(dir.get(), ""));
> > ASSERT_SOME(path);
> > ```
> > 
> > Feel free to fix the other tests in this file in a subsequent review or 
> > just add a TODO for now.
> 
> Michael Browning wrote:
> Happy to fix the other tests in this file. I think it'd be simpler to do 
> it in a separate review -- should I also create an issue for that?

no need to create an issue.


- Vinod


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124456
---


On March 19, 2016, 10:03 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 19, 2016, 10:03 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-20 Thread Michael Browning 


> On March 20, 2016, 6:36 p.m., Vinod Kone wrote:
> > src/slave/containerizer/fetcher.cpp, lines 156-160
> > 
> >
> > Why are you validating the filename as an URI? IIUC, filename should be 
> > a simple relative path?

In the absence of a URI schema, validateUri basically validates exactly that 
case -- but you're right, that's not really in line with the semantics of the 
validateUri method. I'll factor that logic out into a separate validateFilename 
function.


> On March 20, 2016, 6:36 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, lines 645-647
> > 
> >
> > I know you followed the existing pattern in this file but we shouldn't 
> > do mktemp() like this in the tests because if the test fails for whatever 
> > reason the temp file and the directory are leaked. 
> > 
> > The FetcherTest fixture inherits from TemporaryDirectoryTest so that 
> > the test runs in a temporary sandbox.
> > 
> > So you should do:
> > 
> > ```
> > Try dir = os::mkdtemp("./"));
> > ASSERT_SOME(dir);
> > 
> > Try path = os::mktemp(path::join(dir.get(), ""));
> > ASSERT_SOME(path);
> > ```
> > 
> > Feel free to fix the other tests in this file in a subsequent review or 
> > just add a TODO for now.

Happy to fix the other tests in this file. I think it'd be simpler to do it in 
a separate review -- should I also create an issue for that?


- Michael


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124456
---


On March 19, 2016, 10:03 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 19, 2016, 10:03 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-20 Thread Vinod Kone 


> On March 19, 2016, 10:07 a.m., Guangya Liu wrote:
> >

@michael you should mark the issues as "Fixed" or "Drop" once you address them. 
See http://mesos.apache.org/documentation/latest/submitting-a-patch/.


- Vinod


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124393
---


On March 19, 2016, 10:03 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 19, 2016, 10:03 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-20 Thread Vinod Kone 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124456
---



Can you also add a CHANGELOG entry for this under "Additonal API changes" 
section for 0.29.0 (create one if it doesn't exist)?


src/launcher/fetcher.cpp (lines 252 - 260)


The way this is written, we will throw an error if basename cannot be 
determined irrespective of whether filename was specified.

How about:

```
Try basename = uri.has_filename()
  ? uri.filename()
  : Fetcher::basename(uri.value());
```



src/launcher/fetcher.cpp (lines 295 - 298)


ditto. see above.



src/launcher/fetcher.cpp (lines 295 - 298)


ditto. see above.



src/slave/containerizer/fetcher.cpp (lines 156 - 160)


Why are you validating the filename as an URI? IIUC, filename should be a 
simple relative path?



src/tests/fetcher_cache_tests.cpp (lines 689 - 697)


//
const string executablePath = path::join(
task.get().runDirectory.value, customFilename);

EXPECT_TRUE(isExecutable(executablePath));

// The script...
const string outputPath = path::join(
  task.get().runDirectory.value, COMMAND_NAME);

EXPECT_TRUE(os::exists(outputPath + taskName(index)));



src/tests/fetcher_tests.cpp (lines 645 - 647)


I know you followed the existing pattern in this file but we shouldn't do 
mktemp() like this in the tests because if the test fails for whatever reason 
the temp file and the directory are leaked. 

The FetcherTest fixture inherits from TemporaryDirectoryTest so that the 
test runs in a temporary sandbox.

So you should do:

```
Try dir = os::mkdtemp("./"));
ASSERT_SOME(dir);

Try path = os::mktemp(path::join(dir.get(), ""));
ASSERT_SOME(path);
```

Feel free to fix the other tests in this file in a subsequent review or 
just add a TODO for now.



src/tests/fetcher_tests.cpp (line 677)


kill this. if you did as suggested above, this will be automatically 
cleanedup as part of the teardown.



src/tests/fetcher_tests.cpp (lines 684 - 685)


ditto. see above.



src/tests/fetcher_tests.cpp (line 716)


kill this.


- Vinod Kone


On March 19, 2016, 10:03 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 19, 2016, 10:03 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-19 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124436
---



Patch looks great!

Reviews applied: [45046]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 19, 2016, 10:03 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 19, 2016, 10:03 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-19 Thread Michael Browning 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/
---

(Updated March 19, 2016, 10:03 p.m.)


Review request for mesos, Vinod Kone and Zhitao Li.


Changes
---

Addressed style comments.


Bugs: MESOS-4735
https://issues.apache.org/jira/browse/MESOS-4735


Repository: mesos


Description
---

Created URI.filename to name fetched files in sandbox where appropriate.


Diffs (updated)
-

  docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
  include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
  include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
  src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
  src/slave/containerizer/fetcher.cpp 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
  src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
  src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 

Diff: https://reviews.apache.org/r/45046/diff/


Testing
---

There are two paths by which a file gets fetched to the executor sandbox: the 
without-cache path, where the fetcher downloads the file directly from the 
specified URI, and the with-cache path, where it copies it from the cache. In 
both cases, we verify that the file is saved to the sandbox directory with the 
name specified by the "filename" field in the CommandInfo.URI proto.


Thanks,

Michael Browning

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-19 Thread Michael Browning 


> On March 19, 2016, 10:07 a.m., Guangya Liu wrote:
> > src/tests/fetcher_cache_tests.cpp, lines 690-691
> > 
> >
> > blank line here

I'd like to push back against this one -- I think adding a line break here (I 
assume because of the comment block?) would suggest that the previous line is 
distinct in some way from the rest of the paragraph, and that's not really the 
case. If anything, I would expect it between the assignments and the following 
assertions, but I don't have a strong preference between break/no break in that 
case.


- Michael


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124393
---


On March 18, 2016, 11:30 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 18, 2016, 11:30 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-19 Thread Michael Browning 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/
---

(Updated March 18, 2016, 11:30 p.m.)


Review request for mesos, Vinod Kone and Zhitao Li.


Changes
---

Added test to exercise case where "filename" specifies an archive and "extract" 
is true.


Bugs: MESOS-4735
https://issues.apache.org/jira/browse/MESOS-4735


Repository: mesos


Description
---

Created URI.filename to name fetched files in sandbox where appropriate.


Diffs (updated)
-

  docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
  include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
  include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
  src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
  src/slave/containerizer/fetcher.cpp 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
  src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
  src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 

Diff: https://reviews.apache.org/r/45046/diff/


Testing
---

There are two paths by which a file gets fetched to the executor sandbox: the 
without-cache path, where the fetcher downloads the file directly from the 
specified URI, and the with-cache path, where it copies it from the cache. In 
both cases, we verify that the file is saved to the sandbox directory with the 
name specified by the "filename" field in the CommandInfo.URI proto.


Thanks,

Michael Browning

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-19 Thread Guangya Liu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124393
---




src/launcher/fetcher.cpp (lines 255 - 256)


a blank line here and ditto for the following



src/tests/fetcher_cache_tests.cpp (line 669)


const?



src/tests/fetcher_cache_tests.cpp (lines 690 - 691)


blank line here



src/tests/fetcher_tests.cpp (line 654)


const?



src/tests/fetcher_tests.cpp (line 693)


const



src/tests/fetcher_tests.cpp (line 711)


const


- Guangya Liu


On 三月 18, 2016, 11:30 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated 三月 18, 2016, 11:30 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-19 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/#review124381
---



Patch looks great!

Reviews applied: [45046]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 18, 2016, 11:30 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 18, 2016, 11:30 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

2016-03-18 Thread Michael Browning 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45046/
---

Review request for mesos, Vinod Kone and Zhitao Li.


Bugs: MESOS-4735
https://issues.apache.org/jira/browse/MESOS-4735


Repository: mesos


Description
---

Created URI.filename to name fetched files in sandbox where appropriate.


Diffs (updated)
-

  docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
  include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
  include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
  src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
  src/slave/containerizer/fetcher.cpp 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
  src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
  src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 

Diff: https://reviews.apache.org/r/45046/diff/


Testing (updated)
---

There are two paths by which a file gets fetched to the executor sandbox: the 
without-cache path, where the fetcher downloads the file directly from the 
specified URI, and the with-cache path, where it copies it from the cache. In 
both cases, we verify that the file is saved to the sandbox directory with the 
name specified by the "filename" field in the CommandInfo.URI proto.


Thanks,

Michael Browning