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:

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 https://reviews.apache.org/r/36501/diff/3/?file=1013117#file1013117line72 How about check path length and use `path[0]` here? Instead of `*(path.begin())` I think we can use startsWith to check it.

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 https://reviews.apache.org/r/36501/diff/4/?file=1014441#file1014441line22 Need sort it lexicographically. ``` #include sstream #include string ``` #include sstream

Re: Review Request 36470: Updated scheduler driver to send TEARDOWN call.

2015-07-17 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36470/#review92031 --- Patch looks great! Reviews applied: [36461, 36462, 36463, 36464,

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 https://reviews.apache.org/r/36547/diff/3/?file=1013405#file1013405line134 I assume those are spaces? Could you please doublecheck? These are spaces. It's probably like this to indicate that the

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

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 https://reviews.apache.org/r/36547/diff/3/?file=1013405#file1013405line127 Why not just check for any 2xx code then? Aren't they all successful in one way or another? HTTP can return 206, which

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

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

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

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 -

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

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

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

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

2015-07-17 Thread Marco Massenzio
On July 15, 2015, 7:08 p.m., Vinod Kone wrote: src/common/type_utils.cpp, line 131 https://reviews.apache.org/r/36450/diff/2/?file=1011909#file1011909line131 Is the order of query parameters important? Aren't these URLs equivalent? http://a.b.c/?k1=ak2=b

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

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:

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

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 https://reviews.apache.org/r/36049/diff/7/?file=1000249#file1000249line241 I am assuming that the `LocalAuthorizer` should be considered unusable should its initialize function ever fail.

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

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 https://reviews.apache.org/r/36501/diff/4/?file=1014441#file1014441line22 Need sort it lexicographically. ``` #include sstream #include string haosdent huang wrote:

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

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 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 https://reviews.apache.org/r/34142/diff/2/?file=989783#file989783line1300 I believe we discussed this, but different acVersion will most likely have different schema. Unless we

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 https://reviews.apache.org/r/34137/diff/3/?file=1009143#file1009143line630 Hum, looks like a bug since, for example, slaveId is a reference and will be invalid when the lambda is

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

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 https://reviews.apache.org/r/36318/diff/2/?file=1003766#file1003766line78 please use javadoc format also unnecessary semicolon s/a/an (eg an Event) Marco

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,

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 https://reviews.apache.org/r/36501/diff/5/?file=1014595#file1014595line66 Because use stringify, could remove it now. yes. agree. i'll update the code. - Klaus

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

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,

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.

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.

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

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

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 https://reviews.apache.org/r/36488/diff/2-5/?file=1011899#file1011899line99 Did this removed text go somewhere else? I can't find it anymore. Seemed useful. Jie pointed out that the allocator

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: a drive by comment suggested by Joris 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

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]

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

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

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

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:

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 https://reviews.apache.org/r/36318/diff/2/?file=1003766#file1003766line78 please use javadoc format also unnecessary semicolon s/a/an (eg an Event) any particular

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

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

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

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 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 https://reviews.apache.org/r/34142/diff/2/?file=989787#file989787line143 What if the candidate is over-specified (has more labels)? Should this still be a match? According

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 https://reviews.apache.org/r/34140/diff/1/?file=957277#file957277line267 Why not do the decompress, hash untar as a pipeline to reduce disk usage? Ian Downes wrote:

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.

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 https://reviews.apache.org/r/36424/diff/2/?file=1010778#file1010778line302 You may ignore this, but I'm not sure if ignoring `cerr` when the command succeds is

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