Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36440/#review132371 --- Patch looks great! Reviews applied: [45377, 36440] Passed

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47123/#review132372 --- src/module/manager.cpp (line 357)

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

> On May 10, 2016, 5:27 a.m., Cody Maloney wrote: > > I believe this covers the use cases we need. Be great to have a demo PR of it in action for: https://github.com/dcos/dcos to validate that it does. - Cody --- This is an

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36440/#review132210 --- Patch looks great! Reviews applied: [36440] Passed command:

Re: Review Request 46491: Ensured subsequent kills are ignored after the task is reaped.

> On May 7, 2016, 1:22 p.m., haosdent huang wrote: > > src/docker/executor.cpp, line 398 > > > > > > Should it change to `if (killed || terminated ||..)`? Currently, if `terminated == true` then `killed == true` as

Re: Review Request 46491: Ensured subsequent kills are ignored after the task is reaped.

> On May 7, 2016, 1:22 p.m., haosdent huang wrote: > > src/docker/executor.cpp, line 398 > > > > > > Should it change to `if (killed || terminated ||..)`? > > Alexander Rukletsov wrote: > Currently, if

Re: Review Request 46936: Documented the agent endpoint '/flags'.

> On May 5, 2016, 4:15 p.m., Neil Conway wrote: > > src/slave/http.cpp, lines 356-357 > > > > > > +1, although I'd say "The current principal should be ..." I'm ESL, but for me "current" in this context sounds like

Re: Review Request 46935: Documented the agent endpoint '/metrics/snapshot'.

> On May 4, 2016, 8:34 p.m., Alexander Rukletsov wrote: > > src/slave/http.cpp, lines 623-624 > > > > > > How about something like: "The principal should be authorized to query > > this endpoint"? > > Neil Conway

Re: Review Request 46501: Updated authorization.md to reflect current changes.

> On April 29, 2016, 4:07 p.m., Neil Conway wrote: > > docs/authorization.md, line 276 > > > > > > For consistency, this needs a newline (``). using table instead. - Alexander

Re: Review Request 46501: Updated authorization.md to reflect current changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46501/ --- (Updated May 9, 2016, 10:49 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 7, 2016, 12:24 p.m., Adam B wrote: > > src/master/http.cpp, line 869 > > > > > > Did you need to pass all ("=") the in-scope variables through to the > > lambda, or just `request`? Yes, I only need to

Re: Review Request 46935: Documented the agent endpoint '/metrics/snapshot'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46935/ --- (Updated May 9, 2016, 11:04 a.m.) Review request for mesos, Alexander

Re: Review Request 46935: Documented the agent endpoint '/metrics/snapshot'.

> On May 4, 2016, 10:34 p.m., Alexander Rukletsov wrote: > > src/slave/http.cpp, lines 623-624 > > > > > > How about something like: "The principal should be authorized to query > > this endpoint"? > > Neil Conway

Review Request 47114: Cleaned up angle bracket style.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47114/ --- Review request for mesos, Alexander Rukletsov and Ben Mahler. Repository:

Review Request 47112: Cleaned up header includes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47112/ --- Review request for mesos, Alexander Rukletsov and Ben Mahler. Repository:

Review Request 47113: Cleaned up `using` statements.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47113/ --- Review request for mesos, Alexander Rukletsov and Ben Mahler. Repository:

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review132228 --- Looks good! One more round and we can commit this. Do you want

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46203/#review132231 --- src/slave/http.cpp (line 814)

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review132232 --- Also, do you want to update configuration.md? - Alexander

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/tests/master_authorization_tests.cpp, line 1033 > > > > > > Doesn't it compile without explicit casting to string? It doesn't. See the discussion

Re: Review Request 46501: Updated authorization.md to reflect current changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46501/#review132234 --- docs/authorization.md (line 12)

Re: Review Request 46935: Documented the agent endpoint '/metrics/snapshot'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46935/ --- (Updated May 9, 2016, 11:34 a.m.) Review request for mesos, Alexander

Re: Review Request 46501: Updated authorization.md to reflect current changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46501/#review132236 --- Patch looks great! Reviews applied: [46501] Passed command:

Review Request 47115: Fixed some issues in newbie-guide.md.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47115/ --- Review request for mesos and Till Toenshoff. Bugs: MESOS-5292

Re: Review Request 46866: Enabled authorization of libprocess HTTP endpoints (libprocess).

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46866/#review132237 --- 3rdparty/libprocess/include/process/http.hpp (line 85)

Re: Review Request 46867: Enabled authorization of libprocess HTTP endpoints (Mesos).

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46867/#review132240 --- src/common/http.cpp (line 587)

Re: Review Request 46869: Allowed tests to authorize libprocess HTTP endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46869/#review132241 --- src/tests/cluster.cpp (lines 424 - 432)

Re: Review Request 46423: Windows: Forked `subprocess.cpp`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46423/#review132242 --- Ship it! Ship It! - Michael Park On May 5, 2016, 3:35 p.m.,

Re: Review Request 46501: Updated authorization.md to reflect current changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46501/ --- (Updated May 9, 2016, 12:49 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 47114: Cleaned up angle bracket style.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47114/#review132243 --- Patch looks great! Reviews applied: [47112, 47113, 47114]

Re: Review Request 47068: Added fine-grained filtering master flag.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47068/#review132245 --- docs/authorization.md (lines 90 - 101)

Re: Review Request 47069: Added `user` field to `Task` protobuf message.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47069/#review132246 --- In the description: `/protobug/protobuf/`, also, I would avoid

Re: Review Request 47068: Added fine-grained filtering master flag.

> On May 9, 2016, 11:12 a.m., Alexander Rojas wrote: > > docs/authorization.md, lines 90-101 > > > > > > Do we really need to distinguish this one as a different case of > > general authorization? I personally

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:14 a.m., Alexander Rukletsov wrote: > > src/slave/http.cpp, line 814 > > > > > > I think we should have used `MethodNotAllowed` here. See my comment in https://reviews.apache.org/r/46784/. I

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

> On May 5, 2016, 7:20 p.m., Till Toenshoff wrote: > > src/slave/http.cpp, line 373 > > > > > > Two blanks please. Good catch! I've created https://reviews.apache.org/r/47116/ to fix that. - Jan

Review Request 47116: Added a missing blank.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47116/ --- Review request for mesos and Till Toenshoff. Repository: mesos Description

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 7, 2016, 10:24 a.m., Adam B wrote: > > src/master/http.cpp, line 869 > > > > > > Did you need to pass all ("=") the in-scope variables through to the > > lambda, or just `request`? > > Jan Schlicht wrote:

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:18 a.m., Alexander Rukletsov wrote: > > Also, do you want to update configuration.md? To add the "get_endpoints" action to the ACL example? Yep, makes sense to do that here, will add it. - Jan --- This is an

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 9:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 2872 > > > > > > Why don't we return `MethodNotAllowed` here? Same applies to the agent, > > actually : ) > > Jan Schlicht

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 7, 2016, 12:24 p.m., Adam B wrote: > > src/master/http.cpp, line 869 > > > > > > Did you need to pass all ("=") the in-scope variables through to the > > lambda, or just `request`? > > Jan Schlicht wrote:

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 879 > > > > > > In r/46203/ you've cached flags and passed them into continuation. Here > > you call `master->flags` in the

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 2872 > > > > > > Why don't we return `MethodNotAllowed` here? Same applies to the agent, > > actually : ) > > Jan Schlicht

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On April 29, 2016, 11:20 a.m., Benjamin Bannier wrote: > > src/master/http.cpp, line 868 > > > > > > Could you make this capture list explicit (`[this, request]`)? > > Jan Schlicht wrote: > The style guide

Re: Review Request 47116: Added a missing blank.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47116/#review132260 --- src/slave/http.cpp (line 375)

Re: Review Request 47115: Fixed some issues in newbie-guide.md.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47115/#review132263 --- Patch looks great! Reviews applied: [47115] Passed command:

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 2872 > > > > > > Why don't we return `MethodNotAllowed` here? Same applies to the agent, > > actually : ) > > Jan Schlicht

Re: Review Request 47116: Added a missing blank in "slave/http.cpp".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47116/ --- (Updated May 9, 2016, 2:38 p.m.) Review request for mesos and Till Toenshoff.

Re: Review Request 47116: Added a missing blank in "slave/http.cpp".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47116/#review132268 --- Ship it! Ship It! - Alexander Rukletsov On May 9, 2016,

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 2872 > > > > > > Why don't we return `MethodNotAllowed` here? Same applies to the agent, > > actually : ) > > Jan Schlicht

Re: Review Request 46501: Updated authorization.md to reflect current changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46501/#review132271 --- Patch looks great! Reviews applied: [46501] Passed command:

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 2872 > > > > > > Why don't we return `MethodNotAllowed` here? Same applies to the agent, > > actually : ) > > Jan Schlicht

Re: Review Request 46935: Documented the agent endpoint '/metrics/snapshot'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46935/#review132273 --- Ship it! I'll slightly change the wording and commit that for

Re: Review Request 46936: Documented the agent endpoint '/flags'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46936/#review132274 --- Ship it! I'll resolve outstading issues and commit this for

Re: Review Request 46370: Introduced linux capabilities API.

> On May 7, 2016, 7:14 p.m., Jie Yu wrote: > > src/linux/capabilities.hpp, line 194 > > > > > > When I review the code, I found very confusing. We have > > `CapabilityInfo, CapabilityInfo::Capability, Capability,

Review Request 47105: Decided to change the default initial task value.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47105/ --- Review request for mesos. Bugs: MESOS-5342

Review Request 47120: Value from f() was used when it needs to be the value from map<> cost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47120/ --- Review request for mesos. Repository: mesos Description --- Value from

Review Request 47103: Added reference count decrments to core histograms.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47103/ --- Review request for mesos. Bugs: MESOS-5342

Review Request 47101: Process binding using greedy, submodular selection algorithm.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47101/ --- Review request for mesos. Bugs: MESOS-5342

Review Request 47100: Process binding using greedy, submodular selection algorithm.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47100/ --- Review request for mesos. Bugs: MESOS-5342

Review Request 47104: Remove pid from pid-core tracking map.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47104/ --- Review request for mesos. Bugs: MESOS-5342

Review Request 47106: Corrected variable names and 'empty' histogram detection logic.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47106/ --- Review request for mesos. Bugs: MESOS-5342

Review Request 47102: Corrected initial utilization value for cores.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47102/ --- Review request for mesos. Bugs: MESOS-5342

Re: Review Request 47071: Added framework/task filtering to /state and /tasks endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47071/#review132290 --- Patch looks great! Reviews applied: [46613, 47068, 47069, 47070,

Re: Review Request 47120: Value from f() was used when it needs to be the value from map<> cost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47120/#review132295 --- Bad review! Reviews applied: [] Error: No reviewers specified.

Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47123/ --- Review request for mesos, Cody Maloney and Till Toenshoff. Bugs: MESOS-5173

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47123/ --- (Updated May 9, 2016, 1:05 p.m.) Review request for mesos, Cody Maloney and

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47123/ --- (Updated May 9, 2016, 1:10 p.m.) Review request for mesos, Cody Maloney and

Re: Review Request 46806: Fixed and illustrated process::initialize ordering.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46806/#review132298 --- Ship it! Ship It! - Joris Van Remoortere On May 2, 2016,

Review Request 47126: Forgot the algorithm is maximizing when it needs to minimize.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47126/ --- Review request for mesos. Repository: mesos Description --- The

Review Request 47127: Forgot the algorithm is maximizing when it needs to minimize.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47127/ --- Review request for mesos. Repository: mesos Description --- The

Re: Review Request 47100: Process binding using greedy, submodular selection algorithm.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47100/#review132299 --- Bad review! Reviews applied: [] Error: No reviewers specified.

Re: Review Request 46491: Ensured subsequent kills are ignored after the task is reaped.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46491/#review132305 --- src/docker/executor.cpp (lines 396 - 397)

Re: Review Request 46920: Added validation hook inside Slave::runTask.

> On May 6, 2016, 12:37 a.m., Adam B wrote: > > Please create a JIRA with motivation for this hook. I don't want a new hook > > to slip into a release without a JIRA in the changelog to document it. > > Kapil Arya wrote: > Good point. I meant to add it to the RR, but it slipped :-). I

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47123/#review132307 --- Patch looks great! Reviews applied: [47123] Passed command:

Re: Review Request 47080: Agent should backoff when establishing a socket.

> On May 9, 2016, 5:20 a.m., Cong Wang wrote: > > src/slave/slave.cpp, line 954 > > > > > > One thing to note is the authentication could need a connect with > > master, so this could impact it. But essentially we

Re: Review Request 46928: Added safety fixes to and tests `os::close`.

> On May 8, 2016, 9:06 p.m., Joris Van Remoortere wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/close.hpp, > > lines 38-47 > > > > > > https://msdn.microsoft.com/en-us/library/s58ftw19.aspx

Re: Review Request 46325: Updated HTTP command executor to support kill policy in Kill event.

> On May 5, 2016, 8:25 p.m., Ben Mahler wrote: > > src/launcher/http_command_executor.cpp, lines 644-645 > > > > > > How about: > > > > ``` > > cout << "Received a new kill policy grace period of <<

Re: Review Request 46325: Updated HTTP command executor to support kill policy in Kill event.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46325/#review132308 --- src/launcher/http_command_executor.cpp (line 620)

Re: Review Request 46491: Ensured subsequent kills are ignored after the task is reaped.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46491/#review132323 --- Ship it! Modulo comments. - Ben Mahler On May 7, 2016, 1:02

Re: Review Request 46325: Updated HTTP command executor to support kill policy in Kill event.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46325/#review132324 --- Ship it! Modulo comments. - Ben Mahler On May 7, 2016, 1:03

Re: Review Request 47127: Forgot the algorithm is maximizing when it needs to minimize.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47127/#review132325 --- Bad review! Reviews applied: [] Error: No reviewers specified.

Re: Review Request 46504: Constructed error string in MethodNotAllowed.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46504/ --- (Updated May 9, 2016, 10:21 p.m.) Review request for mesos and Alexander

Re: Review Request 47092: Removed misleading return values from process::network::.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47092/#review132342 --- Ship it! Ship It! - Anand Mazumdar On May 7, 2016, 11:40

Re: Review Request 47091: Made http::internal::ConnectionProcess a managed Process.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47091/#review132338 --- Fix it, then Ship it! LGTM. Just a couple of micro nit-picks.

Re: Review Request 46874: Enhanced log message when launch mesos-containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46874/ --- (Updated 五月 9, 2016, 10:39 p.m.) Review request for mesos, Gilbert Song and

Re: Review Request 45377: Made "driver" as optional for DockerVolume.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45377/ --- (Updated May 10, 2016, 2:43 a.m.) Review request for mesos, David vonThenen,

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36440/ --- (Updated May 10, 2016, 3:03 a.m.) Review request for mesos, Gilbert Song and

Re: Review Request 46866: Enabled authorization of libprocess HTTP endpoints (libprocess).

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46866/ --- (Updated May 10, 2016, 3:23 a.m.) Review request for mesos, Alexander Rojas

Re: Review Request 46866: Enabled authorization of libprocess HTTP endpoints (libprocess).

> On May 9, 2016, 10:28 a.m., Alexander Rojas wrote: > > 3rdparty/libprocess/src/process.cpp, line 3289 > > > > > > Why no setting this variable close to were it will be used. I always > > liked the concept of

Re: Review Request 45377: Made "driver" as optional for DockerVolume.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45377/ --- (Updated 五月 10, 2016, 3:41 a.m.) Review request for mesos, David vonThenen,

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36440/ --- (Updated 五月 10, 2016, 3:50 a.m.) Review request for mesos, Gilbert Song and

Re: Review Request 46866: Enabled authorization of libprocess HTTP endpoints (libprocess).

> On May 9, 2016, 10:28 a.m., Alexander Rojas wrote: > > 3rdparty/libprocess/include/process/http.hpp, line 85 > > > > > > Is there a design doc or an overall explanation of how authorization > > will be performed

Review Request 47154: Establish TCP connection after backing off.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47154/ --- Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang. Bugs: