Re: Review Request 36501: MESOS-3023

2015-07-17 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36501/#review92033 --- src/tests/utils.hpp (line 30)

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Jan Schlicht
> On July 17, 2015, 4:59 a.m., Adam B wrote: > > src/launcher/fetcher.cpp, lines 127-128 > > > > > > Why not just check for any 2xx code then? Aren't they all successful in > > one way or another? HTTP can return 2

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Jan Schlicht
> On July 16, 2015, 7:41 p.m., Joerg Schad wrote: > > src/launcher/fetcher.cpp, line 134 > > > > > > I assume those are spaces? Could you please doublecheck? These are spaces. It's probably like this to indicate tha

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 10:32 a.m.) Review request for mesos, Bernd Mathiske an

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92041 --- src/launcher/fetcher.cpp (line 120)

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 10:40 a.m.) Review request for mesos, Bernd Mathiske an

Re: Review Request 36424: Created a command executor helper method.

2015-07-17 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36424/#review92038 --- 3rdparty/libprocess/include/process/subprocess.hpp (lines 302 - 304

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92042 --- Ship it! Ship It! - Joerg Schad On July 17, 2015, 8:40 a.m., Jan

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 10:56 a.m.) Review request for mesos, Bernd Mathiske an

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92045 --- Ship it! Ship It! - Joerg Schad On July 17, 2015, 8:56 a.m., Jan

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92046 --- src/launcher/fetcher.cpp (line 126)

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 11:05 a.m.) Review request for mesos, Bernd Mathiske an

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92048 --- Ship it! Ship It! - Joerg Schad On July 17, 2015, 9:05 a.m., Jan

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

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

Re: Review Request 36547: Fixed fetcher failing for FTP URIs.

2015-07-17 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92052 --- Ship it! Ship It! - Bernd Mathiske On July 17, 2015, 2:05 a.m.,

Re: Review Request 36049: Added support for modularized Authorizer

2015-07-17 Thread Alexander Rojas
> On July 8, 2015, 12:12 a.m., Till Toenshoff wrote: > > src/local/local.cpp, line 241 > > > > > > I am assuming that the `LocalAuthorizer` should be considered unusable > > should its initialize function ever fail.

Re: Review Request 36501: MESOS-3023

2015-07-17 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36501/ --- (Updated July 17, 2015, 2:06 p.m.) Review request for mesos. Bugs: MESOS-3023

Re: Review Request 36501: MESOS-3023

2015-07-17 Thread Klaus Ma
> On July 17, 2015, 7:13 a.m., haosdent huang wrote: > > src/tests/utils.hpp, line 72 > > > > > > How about check path length and use `path[0]` here? Instead of > > `*(path.begin())` I think we can use startsWith to

Re: Review Request 36501: MESOS-3023

2015-07-17 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36501/#review92082 --- Ship it! Ship It! src/tests/fetcher_tests.cpp (line 22)

Re: Review Request 36501: MESOS-3023

2015-07-17 Thread haosdent huang
> On July 17, 2015, 2:20 p.m., haosdent huang wrote: > > Ship It! 3rdparty/libprocess/src/tests/http_tests.cpp have a unit test case "TEST(URLTest, Stringification)" show how to use URL and stringify it. Maybe you could use URL according that. - haosdent ---

Re: Review Request 36501: MESOS-3023

2015-07-17 Thread haosdent huang
> On July 17, 2015, 2:20 p.m., haosdent huang wrote: > > src/tests/fetcher_tests.cpp, line 22 > > > > > > Need sort it lexicographically. > > > > ``` > > #include > > #include ``` #include #includ

Re: Review Request 36501: MESOS-3023

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

Re: Review Request 36501: MESOS-3023

2015-07-17 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36501/ --- (Updated July 17, 2015, 4:13 p.m.) Review request for mesos. Bugs: MESOS-3023

Re: Review Request 36501: MESOS-3023

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

Re: Review Request 34142: AppC provisioner.

2015-07-17 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34142/#review91503 --- src/slave/containerizer/mesos/containerizer.cpp (line 65)

Re: Review Request 36501: MESOS-3023

2015-07-17 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36501/#review92108 --- src/tests/fetcher_tests.cpp (line 22)

Re: Review Request 36501: MESOS-3023

2015-07-17 Thread haosdent huang
> On July 17, 2015, 2:20 p.m., haosdent huang wrote: > > src/tests/fetcher_tests.cpp, line 22 > > > > > > Need sort it lexicographically. > > > > ``` > > #include > > #include > > haosdent huang wr

Re: Review Request 36562: Store MasterInfo instead of UPID in the scheduler driver.

2015-07-17 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36562/#review92115 --- Ship it! Ship It! - Vinod Kone On July 17, 2015, 1:36 a.m., Ben

Re: Review Request 36497: Implemented the SUBSCRIBE Event handler in the scheduler driver.

2015-07-17 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36497/#review92116 --- Ship it! src/sched/sched.cpp (line 453)

Re: Review Request 36450: Introduced Address and URL protobufs.

2015-07-17 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36450/#review92118 --- Ship it! Ship It! - Vinod Kone On July 17, 2015, 1:36 a.m., Ben

Re: Review Request 36498: Implemented the OFFERS Event handler in the scheduler driver.

2015-07-17 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36498/#review92119 --- Ship it! Ship It! - Vinod Kone On July 17, 2015, 1:36 a.m., Ben

Re: Review Request 34140: AppC image store

2015-07-17 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34140/#review91989 --- Not necessary to be done in this review but we should allow `putting

Re: Review Request 34142: AppC provisioner.

2015-07-17 Thread Jiang Yan Xu
> On July 2, 2015, 1:48 a.m., Timothy Chen wrote: > > include/mesos/mesos.proto, line 1300 > > > > > > I believe we discussed this, but different acVersion will most likely > > have different schema. > > > >

Re: Review Request 34137: Add support for container image provisioners.

2015-07-17 Thread Jiang Yan Xu
> On July 16, 2015, 6:36 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 630 > > > > > > Hum, looks like a bug since, for example, slaveId is a reference and > > will be invalid when th

Re: Review Request 34141: AppC provsioning backend.

2015-07-17 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34141/#review91685 --- src/slave/containerizer/provisioners/appc/backend.hpp (line 41)

Re: Review Request 36488: Added oversubscription user doc.

2015-07-17 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36488/ --- (Updated July 17, 2015, 12:47 p.m.) Review request for mesos, Adam B and Jie Yu

Review Request 36575: Added Labels to TaskStatus protobuf and expose them via state.json.

2015-07-17 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36575/ --- Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen. Bug

Review Request 36580: Added TaskStatus label decorator hook for Slave.

2015-07-17 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36580/ --- Review request for mesos. Repository: mesos Description --- This allows

Review Request 36574: Added helper testing functions for Labels.

2015-07-17 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36574/ --- Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen. Bug

Re: Review Request 36488: Added oversubscription user doc.

2015-07-17 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36488/ --- (Updated July 17, 2015, 1 p.m.) Review request for mesos, Adam B and Jie Yu.

Re: Review Request 36488: Added oversubscription user doc.

2015-07-17 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36488/#review92122 --- Ship it! I can't see the jpg to see how you changed it, but the tex

Review Request 36585: Exposed `docker inspect` output via state.json

2015-07-17 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36585/ --- Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy Chen. Bugs:

Re: Review Request 36580: Added TaskStatus label decorator hook for Slave.

2015-07-17 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36580/#review92123 --- Patch looks great! Reviews applied: [36574, 36575, 36580] All test

Re: Review Request 36424: Created a command executor helper method.

2015-07-17 Thread Marco Massenzio
> On July 17, 2015, 8:48 a.m., Alexander Rojas wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, lines 302-304 > > > > > > You may ignore this, but I'm not sure if ignoring `cerr` when the > > command s

Re: Review Request 36424: Created a command executor helper method.

2015-07-17 Thread Marco Massenzio
> On July 14, 2015, 12:20 a.m., Paul Brett wrote: > > > > Artem Harutyunyan wrote: > is inline really necessary here? well, it is - as this is a header file (not having the `inline` will cause a linker error for a 'duplicate definition' or something). Happy to move it to a `.cpp` file, bu

Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

2015-07-17 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36586/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-3055 https://issues.ap

Re: Review Request 36488: Added oversubscription user doc.

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

Re: Review Request 36488: Added oversubscription user doc.

2015-07-17 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36488/ --- (Updated July 17, 2015, 2:27 p.m.) Review request for mesos, Adam B and Jie Yu.

Re: Review Request 36378: Refactor Linux Performance monitor to handle changing 'perf stat' output versions depending on kernel version.

2015-07-17 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36378/#review92128 --- Thanks Paul for taking this on! See my detailed comments. We should

Re: Review Request 36585: Exposed `docker inspect` output via state.json

2015-07-17 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36585/#review92131 --- Patch looks great! Reviews applied: [36574, 36575, 36580, 36585] A

Re: Review Request 36488: Added oversubscription user doc.

2015-07-17 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36488/#review92132 --- Ship it! LGTM! Thanks Niklas! docs/oversubscription.md (line 35)

Re: Review Request 36585: Exposed `docker inspect` output via state.json

2015-07-17 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36585/ --- (Updated July 17, 2015, 6:06 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 36378: Refactor Linux Performance monitor to handle changing 'perf stat' output versions depending on kernel version.

2015-07-17 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36378/#review92133 --- src/linux/perf.cpp (line 178)

Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

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

Re: Review Request 36488: Added oversubscription user doc.

2015-07-17 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36488/ --- (Updated July 17, 2015, 3:19 p.m.) Review request for mesos, Adam B and Jie Yu.

Re: Review Request 36488: Added oversubscription user doc.

2015-07-17 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36488/ --- (Updated July 17, 2015, 3:27 p.m.) Review request for mesos, Adam B and Jie Yu.

Re: Review Request 36488: Added oversubscription user doc.

2015-07-17 Thread Niklas Nielsen
> On July 17, 2015, 1:37 p.m., Adam B wrote: > > docs/oversubscription.md, lines 99-104 > > > > > > Did this removed text go somewhere else? I can't find it anymore. > > Seemed useful. Jie pointed out that the all

Re: Review Request 34142: AppC provisioner.

2015-07-17 Thread Jiang Yan Xu
> On July 1, 2015, 5:40 p.m., Lily Chen wrote: > > src/slave/containerizer/provisioners/appc.cpp, lines 143-151 > > > > > > What if the candidate is over-specified (has more labels)? Should this > > still be a match?

Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

2015-07-17 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36586/ --- (Updated July 17, 2015, 11:08 p.m.) Review request for mesos and Ben Mahler.

Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

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

Re: Review Request 34140: AppC image store

2015-07-17 Thread Jiang Yan Xu
> On May 27, 2015, 4:10 p.m., Paul Brett wrote: > > src/slave/containerizer/provisioners/appc/store.cpp, line 267 > > > > > > Why not do the decompress, hash & untar as a pipeline to reduce disk > > usage? > > Ian Do

Re: Review Request 36318: [MESOS-2294] Add support to master for streaming subscribed events

2015-07-17 Thread Marco Massenzio
> On July 9, 2015, 10:24 p.m., Marco Massenzio wrote: > > src/common/protobuf_utils.hpp, lines 78-79 > > > > > > please use javadoc format > > also unnecessary semicolon > > > > s/a/an > > (eg "an Eve

Re: Review Request 36424: Created a command executor helper method.

2015-07-17 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36424/ --- (Updated July 18, 2015, 1:55 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 36424: Created a command executor helper method.

2015-07-17 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36424/ --- (Updated July 18, 2015, 2 a.m.) Review request for mesos, Benjamin Hindman, Ben

Re: Review Request 36318: [MESOS-2294] Add support to master for streaming subscribed events

2015-07-17 Thread Anand Mazumdar
> On July 9, 2015, 10:24 p.m., Marco Massenzio wrote: > > src/common/protobuf_utils.hpp, lines 78-79 > > > > > > please use javadoc format > > also unnecessary semicolon > > > > s/a/an > > (eg "an Eve

Re: Review Request 36318: [MESOS-2294] Add support to master for streaming subscribed events

2015-07-17 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36318/#review92161 --- Ship it! Ship It! - Marco Massenzio On July 15, 2015, 4:56 a.m.,

Re: Review Request 36501: MESOS-3023

2015-07-17 Thread Klaus Ma
> On 七月 17, 2015, 5:39 p.m., haosdent huang wrote: > > src/tests/fetcher_tests.cpp, line 66 > > > > > > Because use stringify, could remove it now. yes. agree. i'll update the code. - Klaus --

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

2015-07-17 Thread haosdent huang
> On July 11, 2015, 9:38 a.m., Mesos ReviewBot wrote: > > Patch looks great! > > > > Reviews applied: [36185] > > > > All tests passed. ping @tnachen , Could you review this patch again? Thank you in advance. - haosdent --- This is an