Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-16 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/ --- (Updated Jan. 17, 2018, 9:38 a.m.) Review request for mesos, Benjamin Mahler, G

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-16 Thread Qian Zhang
> On Jan. 17, 2018, 1:24 a.m., Vinod Kone wrote: > > src/slave/slave.cpp > > Line 5911 (original) > > > > > > this should have been in the previous review. Fixed. - Qian

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-16 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/#review195462 --- Fix it, then Ship it! src/slave/slave.hpp Lines 837-838 (patch

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-16 Thread Qian Zhang
> On Jan. 9, 2018, 8:04 a.m., Vinod Kone wrote: > > Can you add a test or update an existing test to verify the /files endpoint > > for task volume? Ideally, you could also verify that once the executor's > > work directory is gc'ed the files endpoint no longer serves the task volume > > direc

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-14 Thread Qian Zhang
> On Jan. 9, 2018, 8:04 a.m., Vinod Kone wrote: > > src/slave/slave.cpp > > Lines 9221-9246 (patched) > > > > > > Kill this. See above comments. > > Qian Zhang wrote: > The reason I did this change is, only doi

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-14 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/ --- (Updated Jan. 15, 2018, 2:21 p.m.) Review request for mesos, Benjamin Mahler, G

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-14 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/#review195404 --- src/slave/slave.cpp Lines 2813 (patched)

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-14 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/ --- (Updated Jan. 14, 2018, 10:30 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-10 Thread Qian Zhang
> On Jan. 9, 2018, 8:12 a.m., Jie Yu wrote: > > src/slave/slave.cpp > > Lines 2795-2829 (patched) > > > > > > I am flying by. I am wondering if we should add this logic to > > `Executor::addLaunchedTask` and `Execu

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/#review195178 --- src/slave/slave.cpp Line 2795 (original), 2795 (patched)

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-10 Thread Qian Zhang
> On Jan. 9, 2018, 8:04 a.m., Vinod Kone wrote: > > Can you add a test or update an existing test to verify the /files endpoint > > for task volume? Ideally, you could also verify that once the executor's > > work directory is gc'ed the files endpoint no longer serves the task volume > > direc

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-09 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/#review195038 --- PASS: Mesos patch 64978 was successfully built and tested. Review

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-09 Thread Qian Zhang
> On Jan. 9, 2018, 8:04 a.m., Vinod Kone wrote: > > src/slave/slave.cpp > > Lines 9221-9246 (patched) > > > > > > Kill this. See above comments. The reason I did this change is, only doing detach in `removeExecutor

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-09 Thread Qian Zhang
> On Jan. 9, 2018, 8:12 a.m., Jie Yu wrote: > > src/slave/slave.cpp > > Lines 2795-2829 (patched) > > > > > > I am flying by. I am wondering if we should add this logic to > > `Executor::addLaunchedTask` and `Execu

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-09 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/ --- (Updated Jan. 9, 2018, 10:11 p.m.) Review request for mesos, Benjamin Mahler, G

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-08 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/#review195003 --- src/slave/slave.cpp Lines 2795-2829 (patched)

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-08 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/#review195000 --- Can you add a test or update an existing test to verify the /files

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-08 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/#review194957 --- PASS: Mesos patch 64978 was successfully built and tested. Review

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

2018-01-07 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/ --- (Updated Jan. 7, 2018, 6 p.m.) Review request for mesos, Gilbert Song, Jie Yu,