subject:"Re\: Review Request 46168\: Add subdirectory support to URI.filename field."

Re: Review Request 46168: Add subdirectory support to URI.filename field.

2016-04-22 Thread Jiang Yan Xu


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


Ship it!




Looks great! I'll commit it with the following minor cleanups.


src/launcher/fetcher.cpp (line 316)


4 space indentation.



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


s/filename/path/.



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


No trailing period in error messsages.



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


No trailing period in error messsages.



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


s/filenameValidation/outputFileValiation/



src/tests/fetcher_cache_tests.cpp (line 691)


s/filename/output/



src/tests/fetcher_cache_tests.cpp (line 707)


s/Filename/OutputFile



src/tests/fetcher_cache_tests.cpp (line 736)


4 spaces



src/tests/fetcher_tests.cpp (line 135)


s/filename/path/



src/slave/containerizer/fetcher.hpp (line 70)


s/filename/output_file/



src/slave/containerizer/fetcher.hpp (line 71)


s/filename/path.


- Jiang Yan Xu


On April 21, 2016, 10:44 a.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46168/
> ---
> 
> (Updated April 21, 2016, 10:44 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
> -
> 
>   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.

2016-04-21 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [46168]

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 April 21, 2016, 5:44 p.m., Michael Browning wrote:
> 
> ---
> 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
> ---
> 
> 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.

2016-04-21 Thread Michael Browning


---
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.

2016-04-21 Thread Michael Browning


---
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.

2016-04-19 Thread Mesos ReviewBot


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



Bad patch!

Reviews applied: [46168]

Failed command: ./support/apply-review.sh -n -r 46168

Error:
2016-04-20 04:33:12 URL:https://reviews.apache.org/r/46168/diff/raw/ 
[545835/545835] -> "46168.patch" [1]
46168.patch:881: trailing whitespace.
# 
46168.patch:884: trailing whitespace.
# 
46168.patch:922: trailing whitespace.
# 
46168.patch:925: trailing whitespace.
# 
46168.patch:2742: trailing whitespace.

error: patch failed: 
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp:462
error: 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp: patch 
does not apply
error: patch failed: 
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp:38
error: 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp: patch 
does not apply
error: patch failed: include/mesos/zookeeper/authentication.hpp:60
error: include/mesos/zookeeper/authentication.hpp: patch does not apply
error: patch failed: src/CMakeLists.txt:40
error: src/CMakeLists.txt: patch does not apply
error: patch failed: 
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp:1251
error: src/slave/containerizer/mesos/isolators/network/cni/cni.cpp: patch does 
not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/12622/console

- Mesos ReviewBot


On April 19, 2016, 11:05 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46168/
> ---
> 
> (Updated April 19, 2016, 11:05 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
> ---
> 
> 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
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 7b3621d24a05a3b3b860e3b8c13a1531208e5e26 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 76f946dc11b66e86ce843808e371737b9e022e36 
>   3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt.template 
> 7f7105f5d55fd66f3e826c1c37c0074775bf89ea 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 33ddb06e25920096f2d16d4f372129ee2f6a8721 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> a81086860a17bd1912e55051dd7bb98a20772b51 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
> ce4e4ccd1340e2cff18f5d1b6a9236809bdc69f1 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> 33825280eb1404bcd89324f8ab5949f735b2d130 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
> a8010925c15fccc9bcd7c5d150ccdd9c98b8bb47 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/pagesize.hpp 
> f46da5577ecf4c336ff4d490f4f36ac0d3d058a9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pagesize.hpp 
> f3ae69adf096d558e083615dfcf848c94e017e6e 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pagesize.hpp 
> 6112e9781a9d42f7ec1ae0832c0c877d1915b09b 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> 52978f37a27c6db45b71fa1a1d41bb833a76e666 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 352ecc5fed99f52ef8ffce48291be720791b8b23 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> ac75baf3c660f2bdda308f4eaa856f44d80f1ea1 
>   3rdparty/libprocess/src/decoder.hpp 
> 2c41ce9f00c857aa320b1d2cfa3b1048c316976a 
>   3rdparty/libprocess/src/process.cpp 
> afeddec20495bb9621c3e26b0d425c9419654739 
>   3rdparty/libprocess/src/test-master.cpp 
> 5026af32c6d72d3e031ebf265680ab7bbf937435 
>   3rdparty/libprocess/src/test-slave.cpp 
> 4516bdca66de5889c3163ff7d6890a9806dc4322 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> 383d260a4c0b17e9a0b5af002bb35070e3ec0b01 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3fce6fc41faaa7b6e8a2a957e85de3de973a51ba 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 4d237815a03828b915e821c3af78132e2915c610 
>   CHANGELOG d2e902f8295644c527964123e409be460c2a5789 
>   bin/gdb-mesos-agent.sh.in 1e02c48c27b255139ab73e233abf577e402c5401 
>   bin/lldb-mesos-agent.sh.in 480d6cec9ee8b6bb1b698961df6a555a38226a0a 
>   bin/mesos-agent-flags.sh.in  
>   bin/mesos-agent.sh.in adf79e0eb0c62236fb6095bd3d3539308dded975 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53

Re: Review Request 46168: Add subdirectory support to URI.filename field.

2016-04-19 Thread Michael Browning


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

(Updated April 19, 2016, 11:05 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
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
7b3621d24a05a3b3b860e3b8c13a1531208e5e26 
  3rdparty/libprocess/3rdparty/Makefile.am 
76f946dc11b66e86ce843808e371737b9e022e36 
  3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt.template 
7f7105f5d55fd66f3e826c1c37c0074775bf89ea 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
33ddb06e25920096f2d16d4f372129ee2f6a8721 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
a81086860a17bd1912e55051dd7bb98a20772b51 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
ce4e4ccd1340e2cff18f5d1b6a9236809bdc69f1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
33825280eb1404bcd89324f8ab5949f735b2d130 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
a8010925c15fccc9bcd7c5d150ccdd9c98b8bb47 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/pagesize.hpp 
f46da5577ecf4c336ff4d490f4f36ac0d3d058a9 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pagesize.hpp 
f3ae69adf096d558e083615dfcf848c94e017e6e 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pagesize.hpp 
6112e9781a9d42f7ec1ae0832c0c877d1915b09b 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
52978f37a27c6db45b71fa1a1d41bb833a76e666 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
352ecc5fed99f52ef8ffce48291be720791b8b23 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ac75baf3c660f2bdda308f4eaa856f44d80f1ea1 
  3rdparty/libprocess/src/decoder.hpp 2c41ce9f00c857aa320b1d2cfa3b1048c316976a 
  3rdparty/libprocess/src/process.cpp afeddec20495bb9621c3e26b0d425c9419654739 
  3rdparty/libprocess/src/test-master.cpp 
5026af32c6d72d3e031ebf265680ab7bbf937435 
  3rdparty/libprocess/src/test-slave.cpp 
4516bdca66de5889c3163ff7d6890a9806dc4322 
  3rdparty/libprocess/src/tests/future_tests.cpp 
383d260a4c0b17e9a0b5af002bb35070e3ec0b01 
  3rdparty/libprocess/src/tests/process_tests.cpp 
3fce6fc41faaa7b6e8a2a957e85de3de973a51ba 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
4d237815a03828b915e821c3af78132e2915c610 
  CHANGELOG d2e902f8295644c527964123e409be460c2a5789 
  bin/gdb-mesos-agent.sh.in 1e02c48c27b255139ab73e233abf577e402c5401 
  bin/lldb-mesos-agent.sh.in 480d6cec9ee8b6bb1b698961df6a555a38226a0a 
  bin/mesos-agent-flags.sh.in  
  bin/mesos-agent.sh.in adf79e0eb0c62236fb6095bd3d3539308dded975 
  bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
  bin/valgrind-mesos-agent.sh.in 08d9730e995d71236b224786ecec96f526ed033a 
  configure.ac ae91a07e2bafd97566ac6e6873990302acad14c9 
  docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 
  docs/endpoints/master/create-volumes.md 
5c86480b488e2722a12ab09b0a28399402410e2b 
  docs/endpoints/master/destroy-volumes.md 
f75dd52bd6b75d35aa62dca54148a41cfec36ded 
  docs/endpoints/master/reserve.md 8f38cc0f9c16c368ee3698ec0a6e7fd484110ebc 
  docs/endpoints/master/unreserve.md 8e9a696a50f1821cfb11afae68cc4fb62b6f2a2c 
  docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
  docs/persistent-volume.md e5f2836c8867d5221da9e4f89167850ef9dab8ec 
  docs/reservation.md a400d19aec7a48d122ba1c9c23d38d792b8dbe6f 
  docs/upgrades.md 4f0c86db4c8d51f70487c03b2e75c1a4189b67b9 
  include/mesos/hook.hpp 210ffba09f5acae34ca49b888a781f683777f9ca 
  include/mesos/log/log.hpp 9c8634987181b1345005619e6d16d738903c49fc 
  include/mesos/mesos.proto 87af4a06db8cc3889fe4d3b314206103f5ce5f2d 
  include/mesos/state/in_memory.hpp 203274242854d482b01275597b249c58e6dfe2ad 
  include/mesos/state/leveldb.hpp 6c732d38d68a3d60d28ce68a6340e8771d849c53 
  include/mesos/state/log.hpp ac0312fdb92c46bfa2a7b83e95e04fd1eaf87d03 
  include/mesos/state/protobuf.hpp cde7b771f3c787293fb909a4b982c47ee19f4057 
  include/mesos/state/state.hpp f2fddee4fa803fa0572f6194e7f5f45a56254c00 
  include/mesos/state/state.proto 7a7d68e6abc85c0ead04f5771e878d10348f44b8 
  include/mesos/state/storage.hpp 2bfa0478b0edf76d592cc9644da83d15a00bc68c 
  include/mesos/state/zookeeper.hpp 8d8c19ce778f2499d86eb84008a61f211c528a3a 
  include/mesos/v1/mesos.proto 34da0a1484dc2f71262d8b97484b8edaae35bb6c 
  include/mesos/v1/scheduler.hpp 18e7a95f8342ea155f9e339945b05810b6bd82b5

Re: Review Request 46168: Add subdirectory support to URI.filename field.

2016-04-19 Thread Michael Browning


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

(Updated April 19, 2016, 11:04 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)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
7b3621d24a05a3b3b860e3b8c13a1531208e5e26 
  3rdparty/libprocess/3rdparty/Makefile.am 
76f946dc11b66e86ce843808e371737b9e022e36 
  3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt.template 
7f7105f5d55fd66f3e826c1c37c0074775bf89ea 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
33ddb06e25920096f2d16d4f372129ee2f6a8721 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
a81086860a17bd1912e55051dd7bb98a20772b51 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
ce4e4ccd1340e2cff18f5d1b6a9236809bdc69f1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
33825280eb1404bcd89324f8ab5949f735b2d130 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
a8010925c15fccc9bcd7c5d150ccdd9c98b8bb47 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/pagesize.hpp 
f46da5577ecf4c336ff4d490f4f36ac0d3d058a9 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pagesize.hpp 
f3ae69adf096d558e083615dfcf848c94e017e6e 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pagesize.hpp 
6112e9781a9d42f7ec1ae0832c0c877d1915b09b 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
52978f37a27c6db45b71fa1a1d41bb833a76e666 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
352ecc5fed99f52ef8ffce48291be720791b8b23 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ac75baf3c660f2bdda308f4eaa856f44d80f1ea1 
  3rdparty/libprocess/src/decoder.hpp 2c41ce9f00c857aa320b1d2cfa3b1048c316976a 
  3rdparty/libprocess/src/process.cpp afeddec20495bb9621c3e26b0d425c9419654739 
  3rdparty/libprocess/src/test-master.cpp 
5026af32c6d72d3e031ebf265680ab7bbf937435 
  3rdparty/libprocess/src/test-slave.cpp 
4516bdca66de5889c3163ff7d6890a9806dc4322 
  3rdparty/libprocess/src/tests/future_tests.cpp 
383d260a4c0b17e9a0b5af002bb35070e3ec0b01 
  3rdparty/libprocess/src/tests/process_tests.cpp 
3fce6fc41faaa7b6e8a2a957e85de3de973a51ba 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
4d237815a03828b915e821c3af78132e2915c610 
  CHANGELOG d2e902f8295644c527964123e409be460c2a5789 
  bin/gdb-mesos-agent.sh.in 1e02c48c27b255139ab73e233abf577e402c5401 
  bin/lldb-mesos-agent.sh.in 480d6cec9ee8b6bb1b698961df6a555a38226a0a 
  bin/mesos-agent-flags.sh.in  
  bin/mesos-agent.sh.in adf79e0eb0c62236fb6095bd3d3539308dded975 
  bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
  bin/valgrind-mesos-agent.sh.in 08d9730e995d71236b224786ecec96f526ed033a 
  configure.ac ae91a07e2bafd97566ac6e6873990302acad14c9 
  docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 
  docs/endpoints/master/create-volumes.md 
5c86480b488e2722a12ab09b0a28399402410e2b 
  docs/endpoints/master/destroy-volumes.md 
f75dd52bd6b75d35aa62dca54148a41cfec36ded 
  docs/endpoints/master/reserve.md 8f38cc0f9c16c368ee3698ec0a6e7fd484110ebc 
  docs/endpoints/master/unreserve.md 8e9a696a50f1821cfb11afae68cc4fb62b6f2a2c 
  docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
  docs/persistent-volume.md e5f2836c8867d5221da9e4f89167850ef9dab8ec 
  docs/reservation.md a400d19aec7a48d122ba1c9c23d38d792b8dbe6f 
  docs/upgrades.md 4f0c86db4c8d51f70487c03b2e75c1a4189b67b9 
  include/mesos/hook.hpp 210ffba09f5acae34ca49b888a781f683777f9ca 
  include/mesos/log/log.hpp 9c8634987181b1345005619e6d16d738903c49fc 
  include/mesos/mesos.proto 87af4a06db8cc3889fe4d3b314206103f5ce5f2d 
  include/mesos/state/in_memory.hpp 203274242854d482b01275597b249c58e6dfe2ad 
  include/mesos/state/leveldb.hpp 6c732d38d68a3d60d28ce68a6340e8771d849c53 
  include/mesos/state/log.hpp ac0312fdb92c46bfa2a7b83e95e04fd1eaf87d03 
  include/mesos/state/protobuf.hpp cde7b771f3c787293fb909a4b982c47ee19f4057 
  include/mesos/state/state.hpp f2fddee4fa803fa0572f6194e7f5f45a56254c00 
  include/mesos/state/state.proto 7a7d68e6abc85c0ead04f5771e878d10348f44b8 
  include/mesos/state/storage.hpp 2bfa0478b0edf76d592cc9644da83d15a00bc68c 
  include/mesos/state/zookeeper.hpp 8d8c19ce778f2499d86eb84008a61f211c528a3a 
  include/mesos/v1/mesos.proto 34da0a1484dc2f71262d8b97484b8edaae35bb6c 
  include/mesos/v1/scheduler.hpp 18e7a95f8342ea155f9e339945b05810b6bd82b5 
  include/mesos/zookeeper/authentication.hpp 
1c8855a5ec60d8628887fddff73d460e9ba1e643 
  include/mesos/zookeeper/contender.hpp 
348354ae1ab0a699f5d84b0e33b708cf06341789 
  include/mesos/zookeeper/detector.hpp 5c45f72b3fa540816b4f225004d9ae92158b4ef6 
  include/mesos/zookeeper/group.hpp

Re: Review Request 46168: Add subdirectory support to URI.filename field.

2016-04-19 Thread Michael Browning



> On April 19, 2016, 4:08 p.m., Jiang Yan Xu wrote:
> > src/launcher/fetcher.cpp, line 252
> > 
> >
> > `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
> > 
> >
> > 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
> > 
> >
> > 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.

2016-04-19 Thread Jiang Yan Xu

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

docs/fetcher.md (line 89)

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'/.

src/launcher/fetcher.cpp (line 252)

`basename` usually specifically refers to the `the component following the 
final '/'`.

So here s/basename/outputFile/

src/launcher/fetcher.cpp (lines 253 - 268)

To avoid setting a Try with a "" when Try explicity doesn't have a default 
contructor, here we can keep the conditional operator but separately mkdir if 
needed.

```
// Prepare output directory if necessary.
if (uri.has_filename()) {
  string dirname = Path(uri.filename()).dirname();
  if (dirname != ".") {
Try result =
  os::mkdir(path::join(sandboxDirectory, uriDirname), true);

if (result.isError()) {
  return Error("Unable to create subdirectory in sandbox");
}
  }
}

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

src/launcher/fetcher.cpp (line 255)

s/uriDirname/dirname/

src/launcher/fetcher.cpp (line 258)

2 space indentation here.

src/launcher/fetcher.cpp (line 261)

Here let's add the `dirname` to the error mesasge.

src/launcher/fetcher.cpp (line 308)

Same as above.

It sucks that a lot of code is duplicated here but the newly added code is 
only a part of it. Let's add a TODO to "refactor common logic in fetchFromCache 
and fetchBypassingCache into a helper".

src/slave/containerizer/fetcher.cpp (lines 150 - 153)

What does this check against?

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

src/slave/containerizer/fetcher.cpp (lines 155 - 158)

`filename` is not guaranteed to be at least one character right?

Plus this doesn't prevent users from specifying something like 
`subdir/../../../../`. 

For the latter issue ideally there should be a method `Path::canonical` 
that resolves it but it's OK if we punt on it for now. Leave a TODO here for it 
here.

Also s/validateFilename/validateOutputFile/

src/tests/fetcher_tests.cpp (lines 105 - 106)

Nothing wrong with this but the source doesn't have to reside in a subdir. 
:)

We can remove this so there's no confusion.

- Jiang Yan Xu

On April 13, 2016, 5:06 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46168/
> ---
> 
> (Updated April 13, 2016, 5:06 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
> ---
> 
> 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
>

Re: Review Request 46168: Add subdirectory support to URI.filename field.

2016-04-13 Thread Michael Browning


---
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

Re: Review Request 46168: Add subdirectory support to URI.filename field.

2016-04-13 Thread Mesos ReviewBot


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



Bad patch!

Reviews applied: [46168]

Failed command: ./support/apply-review.sh -n -r 46168

Error:
2016-04-13 23:46:57 URL:https://reviews.apache.org/r/46168/diff/raw/ 
[7349/7349] -> "46168.patch" [1]
Total errors found: 0
Checking 4 files
Error: No line in the commit message summary may exceed 72 characters.

Full log: https://builds.apache.org/job/mesos-reviewbot/12511/console

- Mesos ReviewBot


On April 13, 2016, 9:37 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46168/
> ---
> 
> (Updated April 13, 2016, 9:37 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
> ---
> 
> 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.

Re: Review Request 46168: Add subdirectory support to URI.filename field.

Re: Review Request 46168: Add subdirectory support to URI.filename field.

Re: Review Request 46168: Add subdirectory support to URI.filename field.

Re: Review Request 46168: Add subdirectory support to URI.filename field.

Re: Review Request 46168: Add subdirectory support to URI.filename field.

Re: Review Request 46168: Add subdirectory support to URI.filename field.

Re: Review Request 46168: Add subdirectory support to URI.filename field.

Re: Review Request 46168: Add subdirectory support to URI.filename field.

Re: Review Request 46168: Add subdirectory support to URI.filename field.

Re: Review Request 46168: Add subdirectory support to URI.filename field.

11 matches

Site Navigation

Mail list logo

Footer information