Re: Review Request 36179: Fix duplicate -e environment variables option in Docker::run.

2015-07-05 Thread haosdent huang
On July 5, 2015, 6:36 a.m., Anindya Sinha wrote: I know this is already merged but can't we just get rid of the foreach (const Environment::Variable...) block since env already contains the contents of commandInfo.environment().variables(). Or are there scenarios when that is not

Re: Review Request 36179: Fix duplicate -e environment variables option in Docker::run.

2015-07-05 Thread Anindya Sinha
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36179/#review90402 --- I know this is already merged but can't we just get rid of the

Review Request 36185: Create pre-launch hook before a docker container launches in slave.

2015-07-05 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- Review request for mesos and Timothy Chen. Bugs: MESOS-2588

Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

2015-07-05 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/#review90405 --- Patch looks great! Reviews applied: [36185] All tests passed. -

Re: Review Request 36106: cgroups: added cpuacct subsystem

2015-07-05 Thread Jojy Varghese
On July 2, 2015, 11:32 p.m., Ben Mahler wrote: src/linux/cgroups.cpp, lines 2014-2027 https://reviews.apache.org/r/36106/diff/1/?file=997647#file997647line2014 Any reason you're not just re-using cgroups::stat here? I'd suggest just calling cgroups::stat for now, should simplify

Re: Review Request 36106: cgroups: added cpuacct subsystem

2015-07-05 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 5, 2015, 6:30 p.m.) Review request for mesos, Ian Downes, Jie

Re: Review Request 36106: cgroups: added cpuacct subsystem

2015-07-05 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/#review90407 --- Patch looks great! Reviews applied: [36106] All tests passed. -

Re: Review Request 36037: Adding /call endpoint to Master

2015-07-05 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36037/#review90408 --- src/master/http.cpp (lines 328 - 345)

Re: Review Request 36037: Adding /call endpoint to Master

2015-07-05 Thread Ben Mahler
On July 3, 2015, 12:29 a.m., Ben Mahler wrote: I chatted with Isabel on IRC and asked her to break apart this change into more bite-sized chunks, so that we can do smaller reviews and get things committed incrementally: (1) Dummy /call handler on the master. (2) Validation. (3)

Re: Review Request 36173: Update attributes doc to reflect current supported attributes types.

2015-07-05 Thread Timothy Chen
On July 6, 2015, 5:25 a.m., Adam B wrote: docs/attributes-resources.md, line 31 https://reviews.apache.org/r/36173/diff/1/?file=999015#file999015line31 No sets? Or are attributes implicitly sets? I was suprised when I read the source code too, but we actually explicitly don't

Re: Review Request 32543: Documented problem and solution with slave recovery and systemd settings.

2015-07-05 Thread Benjamin Hindman
On March 27, 2015, 9:17 a.m., Adam B wrote: docs/slave-recovery.md, line 71 https://reviews.apache.org/r/32543/diff/2/?file=907123#file907123line71 (If the slave does not come back, each executorDriver shuts itself down after $MESOS_RECOVERY_TIMEOUT.) Important

Re: Review Request 32543: Documented problem and solution with slave recovery and systemd settings.

2015-07-05 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32543/#review90433 --- Ship it! I updated this to be specific to using 'posix' isolation

Re: Review Request 36173: Update attributes doc to reflect current supported attributes types.

2015-07-05 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36173/#review90437 --- Ship it! Seems like scalar and text are basic types, and then

Re: Review Request 36116: MESOS-2965: Add implicit conversion from Path to std::string.

2015-07-05 Thread Benjamin Hindman
On July 6, 2015, 2:36 a.m., Benjamin Hindman wrote: Ship It! Thanks Joseph! It would be great to follow up this with JIRA + reviews that eliminate the use of 'value' from a lot of our existing code that uses Path. - Benjamin ---

Re: Review Request 36189: Add strings::Mode to strings::trim.

2015-07-05 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36189/#review90427 --- Bad patch! Reviews applied: [36189] Failed command:

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-07-05 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34646/ --- (Updated July 6, 2015, 2:25 a.m.) Review request for mesos and Adam B. Bugs:

Re: Review Request 36116: MESOS-2965: Add implicit conversion from Path to std::string.

2015-07-05 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36116/ --- (Updated July 6, 2015, 2:34 a.m.) Review request for mesos, Artem Harutyunyan

Re: Review Request 36116: MESOS-2965: Add implicit conversion from Path to std::string.

2015-07-05 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36116/#review90425 --- Ship it! Ship It! - Benjamin Hindman On July 6, 2015, 2:34

Review Request 36189: Add strings::Mode to strings::trim.

2015-07-05 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36189/ --- Review request for mesos and Artem Harutyunyan. Bugs: MESOS-2862

Re: Review Request 35755: Changed fetcher to handle leading whitespace in URLs.

2015-07-05 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35755/#review90423 --- src/launcher/fetcher.cpp (line 160)

Re: Review Request 32384: Adding perf check to configure

2015-07-05 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32384/#review90428 --- Ship it! Your check for 'perf' in the configuration is really not

Re: Review Request 32384: Adding perf check to configure

2015-07-05 Thread Benjamin Hindman
On July 2, 2015, 11:45 a.m., haosdent huang wrote: configure.ac, line 1121 https://reviews.apache.org/r/32384/diff/2/?file=998167#file998167line1121 Seems other exist checks use AC_CHECK_TOOL, such as ``` AC_CHECK_TOOL([PROTOCOMPILER_TEST], [protoc], [],