Review Request 46717: Added sketch of fetcher cache metrics.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46717/ --- Review request for mesos and Bernd Mathiske. Bugs: MESOS-4760 https://issues.apache.org/jira/browse/MESOS-4760 Repository: mesos Description --- Added sketch of fetcher cache metrics. Diffs - src/slave/containerizer/fetcher.hpp 9e3018dc087ed55c61b2824d0105bc5339b83043 src/slave/containerizer/fetcher.cpp 176d8863d1becd8864218a0012ab45c614f0ad77 src/slave/metrics.cpp 86eb8db644227cb593e52305bfbd05444bb87a6e src/slave/slave.hpp 57b18882e30e44dcc40449b0e3be8ee970c45bc8 Diff: https://reviews.apache.org/r/46717/diff/ Testing --- Thanks, Michael Browning
Re: Review Request 46168: Add subdirectory support to URI.filename field.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46168/ --- (Updated April 21, 2016, 5:44 p.m.) Review request for mesos, Bernd Mathiske, Jiang Yan Xu, and Zhitao Li. Bugs: MESOS-5119 https://issues.apache.org/jira/browse/MESOS-5119 Repository: mesos Description (updated) --- Add subdirectory support to URI.filename field. URI.filename allows the user to specify the name of the file that will be saved in the sandbox when the URI is fetched, but previously it would fail at fetch time if "filename" had a directory component. This change allows users to specify a relative path for custom filenames within the sandbox. Diffs - CHANGELOG d2e902f8295644c527964123e409be460c2a5789 docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 include/mesos/mesos.proto 87af4a06db8cc3889fe4d3b314206103f5ce5f2d include/mesos/v1/mesos.proto 34da0a1484dc2f71262d8b97484b8edaae35bb6c src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d src/slave/containerizer/fetcher.hpp eeb663eac4c86e079228ac806018050d5d039e07 src/slave/containerizer/fetcher.cpp d5910ad570371ba54580be5bb94344a1de38d1f9 src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 Diff: https://reviews.apache.org/r/46168/diff/ Testing --- Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory tests that the fetcher creates a file in a specified subdirectory in the sandbox, and AbsoluteCustomSubdirectoryFails tests that a custom filename with an absolute path is rejected. In fetcher_cache_tests.cpp, CachedCustomFilenameWithSubdirectory tests that the same behavior holds when the URI is fetched from the cache. Thanks, Michael Browning
Re: Review Request 46168: Add subdirectory support to URI.filename field.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46168/ --- (Updated April 21, 2016, 5:43 p.m.) Review request for mesos, Bernd Mathiske, Jiang Yan Xu, and Zhitao Li. Bugs: MESOS-5119 https://issues.apache.org/jira/browse/MESOS-5119 Repository: mesos Description (updated) --- Add subdirectory support to URI.filename field. Diffs (updated) - CHANGELOG d2e902f8295644c527964123e409be460c2a5789 docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 include/mesos/mesos.proto 87af4a06db8cc3889fe4d3b314206103f5ce5f2d include/mesos/v1/mesos.proto 34da0a1484dc2f71262d8b97484b8edaae35bb6c src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d src/slave/containerizer/fetcher.hpp eeb663eac4c86e079228ac806018050d5d039e07 src/slave/containerizer/fetcher.cpp d5910ad570371ba54580be5bb94344a1de38d1f9 src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 Diff: https://reviews.apache.org/r/46168/diff/ Testing --- Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory tests that the fetcher creates a file in a specified subdirectory in the sandbox, and AbsoluteCustomSubdirectoryFails tests that a custom filename with an absolute path is rejected. In fetcher_cache_tests.cpp, CachedCustomFilenameWithSubdirectory tests that the same behavior holds when the URI is fetched from the cache. Thanks, Michael Browning
Re: Review Request 46168: Add subdirectory support to URI.filename field.
_tests.cpp ee58488b0b927c7c5833add4718941539663e6d2 src/tests/sorter_tests.cpp eb207a36c18198588cd8a98b3860a66c2ff7a641 src/tests/state_tests.cpp e9f6e850373b0ba16bd84745926efb0e35574f97 src/tests/zookeeper_test_server.hpp 4ca9aca11f2d2834d30d07a99bd9d4a5d0616dd8 src/tests/zookeeper_tests.cpp 0a360c6a12133e9208638db64e12b27a5328a9d2 src/tests/zookeeper_url_tests.cpp 2b6345fd65addcae7d2ed5cc47f1f7396017fb7e src/zookeeper/authentication.cpp 0fd99b019a5b4f65d2094eee637351f7ff2206a9 src/zookeeper/contender.cpp 4b1cc654cb96d3fd87c5f36c93a8ee1e170bea77 src/zookeeper/detector.cpp f1def68740e4941a5daa3a889bc14278d7ea7366 src/zookeeper/group.cpp 01680899778e554af70b176db82498ca92b51b60 src/zookeeper/zookeeper.cpp 5b136bbc294be83730235e342bda788035b00d59 support/generate-endpoint-help.py 70221febe9b7009f243b8faa8979ea10a8bd724e support/hooks/commit-msg 755309e70581835d01f53c1910b37b6cd3ea98ca support/test-upgrade.py 49932006f781072ee517863f1d77232e0cf80552 Diff: https://reviews.apache.org/r/46168/diff/ Testing --- Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory tests that the fetcher creates a file in a specified subdirectory in the sandbox, and AbsoluteCustomSubdirectoryFails tests that a custom filename with an absolute path is rejected. In fetcher_cache_tests.cpp, CachedCustomFilenameWithSubdirectory tests that the same behavior holds when the URI is fetched from the cache. Thanks, Michael Browning
Re: Review Request 46168: Add subdirectory support to URI.filename field.
0a360c6a12133e9208638db64e12b27a5328a9d2 src/tests/zookeeper_url_tests.cpp 2b6345fd65addcae7d2ed5cc47f1f7396017fb7e src/zookeeper/authentication.cpp 0fd99b019a5b4f65d2094eee637351f7ff2206a9 src/zookeeper/contender.cpp 4b1cc654cb96d3fd87c5f36c93a8ee1e170bea77 src/zookeeper/detector.cpp f1def68740e4941a5daa3a889bc14278d7ea7366 src/zookeeper/group.cpp 01680899778e554af70b176db82498ca92b51b60 src/zookeeper/zookeeper.cpp 5b136bbc294be83730235e342bda788035b00d59 support/generate-endpoint-help.py 70221febe9b7009f243b8faa8979ea10a8bd724e support/hooks/commit-msg 755309e70581835d01f53c1910b37b6cd3ea98ca support/test-upgrade.py 49932006f781072ee517863f1d77232e0cf80552 Diff: https://reviews.apache.org/r/46168/diff/ Testing --- Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory tests that the fetcher creates a file in a specified subdirectory in the sandbox, and AbsoluteCustomSubdirectoryFails tests that a custom filename with an absolute path is rejected. In fetcher_cache_tests.cpp, CachedCustomFilenameWithSubdirectory tests that the same behavior holds when the URI is fetched from the cache. Thanks, Michael Browning
Re: Review Request 46168: Add subdirectory support to URI.filename field.
> On April 19, 2016, 4:08 p.m., Jiang Yan Xu wrote: > > src/launcher/fetcher.cpp, line 252 > > <https://reviews.apache.org/r/46168/diff/1/?file=1343101#file1343101line252> > > > > `basename` usually specifically refers to the `the component following > > the final '/'`. > > > > So here s/basename/outputFile/ It is a proper basename in the case where we're just truncating the URI, but I agree that outputFile is more general given the expanded use here. > On April 19, 2016, 4:08 p.m., Jiang Yan Xu wrote: > > docs/fetcher.md, line 89 > > <https://reviews.apache.org/r/46168/diff/1/?file=1343100#file1343100line89> > > > > I feel that "filename" is a bit odd when it can be a path. Many > > utilities simply call this a file, or output file to be more generic and > > flexible (when they do take a path). > > > > e.g., > > ``` > > wget --output-document file > > url --output file > > tar --file file > > gcc -o outfile > > clang -o output-file > > ``` > > > > I think `output_file` sounds good. (Notice the snake_casing in proto). > > What do you think? > > > > We need to change docs and code (including CHANGELOG) elsewhere too but > > luckily the API is not released yet. > > > > In CHANGELOG we can still group things under [MESOS-4735], just > > s/'filename'/'output_file'/. output_file sounds good to me. > On April 19, 2016, 4:08 p.m., Jiang Yan Xu wrote: > > src/slave/containerizer/fetcher.cpp, lines 150-153 > > <https://reviews.apache.org/r/46168/diff/1/?file=1343102#file1343102line150> > > > > What does this check against? > > > > Path(filename).basename() never returns an Error(). Ah, got it confused with Fetcher::basename, which can return an Error. - Michael ------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46168/#review129487 --- On April 14, 2016, 12:06 a.m., Michael Browning wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46168/ > --- > > (Updated April 14, 2016, 12:06 a.m.) > > > Review request for mesos, Bernd Mathiske, Jiang Yan Xu, and Zhitao Li. > > > Bugs: MESOS-5119 > https://issues.apache.org/jira/browse/MESOS-5119 > > > Repository: mesos > > > Description > --- > > Add subdirectory support to URI.filename field. > > URI.filename allows the user to specify the name of the file that will > be saved in the sandbox when the URI is fetched, but previously it would > fail at fetch time if "filename" had a directory component. This change > allows users to specify a relative path for custom filenames within the > sandbox. > > > Diffs > - > > docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 > src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d > src/slave/containerizer/fetcher.cpp > d5910ad570371ba54580be5bb94344a1de38d1f9 > src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e > src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 > > Diff: https://reviews.apache.org/r/46168/diff/ > > > Testing > --- > > Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory > tests that the fetcher creates a file in a specified subdirectory in the > sandbox, and AbsoluteCustomSubdirectoryFails tests that a custom filename > with an absolute path is rejected. In fetcher_cache_tests.cpp, > CachedCustomFilenameWithSubdirectory tests that the same behavior holds when > the URI is fetched from the cache. > > > Thanks, > > Michael Browning > >
Re: Review Request 46168: Add subdirectory support to URI.filename field.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46168/ --- (Updated April 13, 2016, 11:55 p.m.) Review request for mesos, Bernd Mathiske and Jiang Yan Xu. Bugs: MESOS-5119 https://issues.apache.org/jira/browse/MESOS-5119 Repository: mesos Description (updated) --- Add subdirectory support to URI.filename field. URI.filename allows the user to specify the name of the file that will be saved in the sandbox when the URI is fetched, but previously it would fail at fetch time if "filename" had a directory component. This change allows users to specify a relative path for custom filenames within the sandbox. Diffs - docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d src/slave/containerizer/fetcher.cpp d5910ad570371ba54580be5bb94344a1de38d1f9 src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 Diff: https://reviews.apache.org/r/46168/diff/ Testing --- Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory tests that the fetcher creates a file in a specified subdirectory in the sandbox, and AbsoluteCustomSubdirectoryFails tests that a custom filename with an absolute path is rejected. In fetcher_cache_tests.cpp, CachedCustomFilenameWithSubdirectory tests that the same behavior holds when the URI is fetched from the cache. Thanks, Michael Browning
Review Request 46168: Add subdirectory support to URI.filename field.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46168/ --- Review request for mesos, Bernd Mathiske and Jiang Yan Xu. Bugs: MESOS-5119 https://issues.apache.org/jira/browse/MESOS-5119 Repository: mesos Description --- Add subdirectory support to URI.filename field. URI.filename allows the user to specify the name of the file that will be saved in the sandbox when the URI is fetched, but previously it would fail at fetch time if "filename" had a directory component. This change allows users to specify a relative path for custom filenames within the sandbox. Diffs - docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d src/slave/containerizer/fetcher.cpp d5910ad570371ba54580be5bb94344a1de38d1f9 src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 Diff: https://reviews.apache.org/r/46168/diff/ Testing --- Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory tests that the fetcher creates a file in a specified subdirectory in the sandbox, and AbsoluteCustomSubdirectoryFails tests that a custom filename with an absolute path is rejected. In fetcher_cache_tests.cpp, CachedCustomFilenameWithSubdirectory tests that the same behavior holds when the URI is fetched from the cache. Thanks, Michael Browning
Review Request 45461: Create tempfiles in test temporary directory.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45461/ --- Review request for mesos and Vinod Kone. Repository: mesos Description --- Create tempfiles in test temporary directory. Previously, some tests were creating tempfiles outside of the temporary directory created by the test case and then manually removing them, which could lead to their being leaked if the test were interrupted. Diffs - src/tests/fetcher_tests.cpp 84c8d487bbf61c77452bd293062a80fa132e069f Diff: https://reviews.apache.org/r/45461/diff/ Testing --- This change merely creates all temporary files used in the process of running fetcher tests inside of the temp directories created by the test case, so no further testing was performed. Thanks, Michael Browning
Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.
--- 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.
--- 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.
> On March 20, 2016, 6:36 p.m., Vinod Kone wrote:
> > src/slave/containerizer/fetcher.cpp, lines 156-160
> > <https://reviews.apache.org/r/45046/diff/3/?file=1307410#file1307410line156>
> >
> > 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
> > <https://reviews.apache.org/r/45046/diff/3/?file=1307412#file1307412line645>
> >
> > 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.
--- 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.
> On March 19, 2016, 10:07 a.m., Guangya Liu wrote: > > src/tests/fetcher_cache_tests.cpp, lines 690-691 > > <https://reviews.apache.org/r/45046/diff/2/?file=1307068#file1307068line690> > > > > 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.
--- 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
Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.
--- 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
Review Request 43587: Replaced use of *_MESSAGE macros with *_PROTOBUF equivalents.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43587/ --- Review request for mesos, Joris Van Remoortere and Neil Conway. Bugs: MESOS-3486 https://issues.apache.org/jira/browse/MESOS-3486 Repository: mesos Description --- Replaced use of *_MESSAGE macros with *_PROTOBUF equivalents. Diffs - src/tests/hook_tests.cpp 59a00ea722a17bbc82b14c69bda826f68cbac6e9 src/tests/log_tests.cpp 923d71f48e743a77f2a3ba9e982aef4417c6c7fe src/tests/partition_tests.cpp c5badbe90e302793bfbf3f16373efe241decb7d5 src/tests/slave_recovery_tests.cpp e2a78a0f55b7657057ee351a747caff51024fd67 src/tests/slave_tests.cpp c7f5a701eff2c2f9aa3df5722583a131bf2c072a Diff: https://reviews.apache.org/r/43587/diff/ Testing --- This change involves changing existing macro invocations in testing code, so no additional testing was performed. Thanks, Michael Browning
