Re: Review Request 38910: Added `-v` flag to `docker rm`.

> On Oct. 1, 2015, 4:31 a.m., Jojy Varghese wrote: > > src/docker/docker.cpp, line 681 > > > > > > wondering this behavior should be defaulted or not. We might be > > overloading stop with more than what it should

Re: Review Request 38910: Added `-v` flag to `docker rm`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38910/#review101743 --- src/docker/docker.cpp (line 681)

Review Request 39007: libprocess: Used thread-safe replacement for strerror.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39007/ --- Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.

Review Request 39006: stout: Used thread-safe replacement for strerror.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39006/ --- Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.

Re: Review Request 39037: Allow description empty in help information.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39037/ --- (Updated Oct. 7, 2015, 3:04 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 39060: Create master detector per url & not per framework

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39060/#review101738 --- src/sched/sched.cpp (line 139)

Re: Review Request 39018: Added JSON parsing for Resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39018/#review101740 --- src/common/resources.cpp (line 354)

Re: Review Request 38570: Change documentation image links to absolute paths.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38570/#review101673 --- Patch looks great! Reviews applied: [38570] All tests passed. -

Re: Review Request 38919: Validation of Docker Image Manifests

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38919/ --- (Updated Oct. 6, 2015, 12:08 p.m.) Review request for mesos, Jojy Varghese and

Review Request 39056: Fix for Mesos master crash due to check failure.

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

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39056/#review101674 --- src/common/resources.cpp (line 870)

Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38527/#review101678 --- Patch looks great! Reviews applied: [38527] All tests passed. -

Re: Review Request 39037: Allow description empty in help information.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39037/#review101677 --- 3rdparty/libprocess/include/process/help.hpp (line 51)

Re: Review Request 37024: Exposes mesos version information in components.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37024/#review101679 --- Would be great to add a unit test for VersionProcess as a follow

Re: Review Request 37993: Add explanatory comments for Allocator interface

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37993/#review101613 --- Patch looks great! Reviews applied: [37993] All tests passed. -

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38877/#review101615 --- src/slave/slave.cpp (line 2420)

Re: Review Request 37024: Exposes mesos version information in components.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37024/#review101617 --- Patch looks great! Reviews applied: [37024] All tests passed. -

Re: Review Request 37853: Overlay filesystem provisioning backend

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37853/ --- (Updated Oct. 6, 2015, 6:36 a.m.) Review request for mesos, Ben Mahler, Chi

Re: Review Request 39018: Added JSON parsing for Resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39018/#review101616 --- src/common/resources.cpp (line 362)

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38877/#review101614 --- src/slave/slave.cpp (line 2479)

Re: Review Request 38779: Use new HTTP status code check in scheduler.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38779/#review101685 --- Ship it! Ship It! - Gilbert Song On Oct. 6, 2015, 1:27 p.m.,

Re: Review Request 38919: Validation of Docker Image Manifests

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38919/#review101682 --- src/slave/containerizer/provisioner/docker/spec.cpp (line 62)

Review Request 39060: Create master detector per url & not per framework

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

Re: Review Request 38779: Use new HTTP status code check in scheduler.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38779/ --- (Updated Oct. 6, 2015, 8:27 p.m.) Review request for mesos, Anand Mazumdar and

Re: Review Request 39013: RegistryClient refactor: Fixed comments style.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39013/#review101680 --- Ship it! Thanks for pulling away the dependencies, I'll commit

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39014/#review101681 --- I would have liked to ship this but it looks like the fsLayers

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

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

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38416/ --- (Updated Oct. 6, 2015, 8:27 p.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39056/ --- (Updated Oct. 6, 2015, 9:33 p.m.) Review request for mesos and Neil Conway.

Re: Review Request 38919: Validation of Docker Image Manifests

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38919/#review101693 --- Patch looks great! Reviews applied: [38901, 38919] All tests

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote: > > src/v1/resources.cpp, lines 880-886 > > > > > > Not yours, but too bad we need to duplicate the logic here. I am now > > sure what will be the long term plan here. If

Re: Review Request 37996: Added property manager

> On Oct. 6, 2015, 12:39 p.m., Bernd Mathiske wrote: > > Ship It! I don't think we should introduce this into stout in its current form. I realize you're planning to use this for authentication stuff, but looking at this on its own, it seems like a confusing abstraction. Why would we couple

Re: Review Request 38570: Change documentation image links to absolute paths.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38570/#review101688 --- docs/external-containerizer.md (line 92)

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39056/#review101690 --- Ship it! Ship It! src/tests/reservation_tests.cpp (line 173)

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39056/#review101691 --- src/common/resources.cpp (line 876)

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

> On Oct. 6, 2015, 6:34 p.m., Neil Conway wrote: > > src/common/resources.cpp, line 874 > > > > > > This change should also be applied to Resources::apply() in > > src/v1/resources.cpp Thanks Neil. - Mandeep

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote: > > src/common/resources.cpp, line 880 > > > > > > Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to > > > >

Re: Review Request 39060: Create master detector per url & not per framework

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

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38416/#review101692 --- 3rdparty/libprocess/include/process/http.hpp (lines 146 - 188)

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39056/ --- (Updated Oct. 6, 2015, 10:07 p.m.) Review request for mesos and Neil Conway.

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote: > > src/common/resources.cpp, line 880 > > > > > > Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to > > > >

Re: Review Request 39042: Updated comment referencing deprecated and removed /master/shutdown endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39042/ --- (Updated Oct. 6, 2015, 8:18 a.m.) Review request for mesos and Bernd Mathiske.

Review Request 39042: Updated comment referencing deprecated and removed /master/shutdown endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39042/ --- Review request for mesos and Bernd Mathiske. Bugs: MESOS-2697

Re: Review Request 37996: Added property manager

> On Oct. 6, 2015, 1:14 a.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp, lines > > 30-52 > > > > > > Hm.. what will this abstraction be used for? I have a hard time

Re: Review Request 38579: Refactored registry client

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38579/#review101621 --- Patch looks great! Reviews applied: [39013, 38443, 38579] All

Re: Review Request 38919: Validation of Docker Image Manifests

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38919/#review101735 --- Patch looks great! Reviews applied: [38901, 38919] All tests

Re: Review Request 37996: Added property manager

> On Oct. 7, 2015, 1:42 a.m., Till Toenshoff wrote: > > Like most other reviewers, I must confess that my first reaction when > > seeing this tree application where mixed and triggered doubts if we really > > needed to introduce it. However, I found Alexander's given points very > >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote: > > One high level suggestion. > > > > After looking at our http code, I realized that we use the same pattern > > again and again, for example: > > ``` > > JSON::Array array; > >

Re: Review Request 37468: Removed allocation types to mesos::master namespace

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37468/ --- (Updated Oct. 7, 2015, 2:04 a.m.) Review request for mesos. Bugs: MESOS-2516

Re: Review Request 37024: Exposes mesos version information in components.

> On Oct. 6, 2015, 7:23 p.m., Ben Mahler wrote: > > Would be great to add a unit test for VersionProcess as a follow up, not in > > this patch. Thank you very much, let me add it later. - haosdent --- This is an automatically

Re: Review Request 37024: Exposes mesos version information in components.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37024/ --- (Updated Oct. 7, 2015, 2:30 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 37024: Exposes mesos version information in components.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37024/ --- (Updated Oct. 7, 2015, 2:47 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 39018: Added JSON parsing for Resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39018/#review101739 --- Patch looks great! Reviews applied: [39018] All tests passed. -

Re: Review Request 37024: Exposes mesos version information in components.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37024/ --- (Updated Oct. 7, 2015, 2:56 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39014/ --- (Updated Oct. 7, 2015, 12:16 a.m.) Review request for mesos and Ben Mahler.

Review Request 39068: RegistryClient refactor: Renamed fsLayerInfoList

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

Re: Review Request 38941: Moved structs outside RegistryClient

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38941/ --- (Updated Oct. 7, 2015, 12:19 a.m.) Review request for mesos, Ben Mahler and

Re: Review Request 37996: Added property manager

> On Oct. 6, 2015, 12:39 p.m., Bernd Mathiske wrote: > > Ship It! > > Ben Mahler wrote: > I don't think we should introduce this into stout in its current form. I > realize you're planning to use this for authentication stuff, but looking at > this on its own, it seems like a confusing

Re: Review Request 38910: Added `-v` flag to `docker rm`.

> On Oct. 1, 2015, 4:31 a.m., Jojy Varghese wrote: > > src/docker/docker.cpp, line 681 > > > > > > wondering this behavior should be defaulted or not. We might be > > overloading stop with more than what it should

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39056/#review101726 --- Patch looks great! Reviews applied: [39056] All tests passed. -

Re: Review Request 39068: RegistryClient refactor: Renamed fsLayerInfoList

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39068/ --- (Updated Oct. 7, 2015, 12:39 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39056/#review101727 --- src/common/resources.cpp (line 875)

Re: Review Request 39015: RegistryClient refactor: expanded abbreviated names.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39015/ --- (Updated Oct. 7, 2015, 12:41 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 39017: RegistryClient refactor: encapsulated Manifest

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39017/ --- (Updated Oct. 7, 2015, 12:41 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 38873: Added helper functions for evolving old style executor messages to V1 Executor Events

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38873/#review101709 --- Ship it! Will get this committed shortly.

Re: Review Request 39016: RegistryClient refactor: refactored lambdas

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39016/ --- (Updated Oct. 7, 2015, 12:41 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 38941: Moved structs outside RegistryClient

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38941/ --- (Updated Oct. 7, 2015, 12:40 a.m.) Review request for mesos, Ben Mahler and

Re: Review Request 39053: RegistryClient refactor: priv method const'ness

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39053/ --- (Updated Oct. 7, 2015, 12:42 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

This patch still depends on https://reviews.apache.org/r/38579/, are you planning to remove the dependency? I can't ship these smaller changes since they depend on the large refactor and your layer id patch: https://reviews.apache.org/r/38443/ On Tue, Oct 6, 2015 at 5:16 PM, Jojy Varghese

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39056/#review101729 --- src/common/resources.cpp (line 875)

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

> On 十月 7, 2015, 1:03 a.m., Guangya Liu wrote: > > src/common/resources.cpp, line 879 > > > > > > The meos is now using 0.01 as the MIN_CPUS As the mesos is using 0.01 as the MIN_CPUS, I think it is OK using 0.01

Re: Review Request 38910: Added `-v` flag to `docker rm`.

> On 十月 1, 2015, 4:31 a.m., Jojy Varghese wrote: > > src/docker/docker.cpp, line 681 > > > > > > wondering this behavior should be defaulted or not. We might be > > overloading stop with more than what it should be

Re: Review Request 37996: Added property manager

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37996/#review101707 --- Ship it! Like most other reviewers, I must confess that my first

Re: Review Request 38919: Validation of Docker Image Manifests

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38919/ --- (Updated Oct. 6, 2015, 3:35 p.m.) Review request for mesos, Jojy Varghese and

Re: Review Request 38779: Use new HTTP status code check in scheduler.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38779/#review101710 --- Patch looks great! Reviews applied: [38416, 38779] All tests

Re: Review Request 39018: Added JSON parsing for Resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39018/ --- (Updated Oct. 6, 2015, 11:02 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 39018: Added JSON parsing for Resources.

> On Oct. 6, 2015, 7:09 a.m., haosdent huang wrote: > > src/common/resources.cpp, line 362 > > > > > > Is it would more clear to split three functions, one is for parse old > > form, one is for parse json object

Re: Review Request 38570: Change documentation image links to absolute paths.

> On Oct. 6, 2015, 2:29 p.m., Ben Mahler wrote: > > docs/external-containerizer.md, line 92 > > > > > > How will this work when we add multiple versions of documents to the > > website? Now this hardcodes "latest"

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

> On Oct. 6, 2015, 7:57 p.m., Ben Mahler wrote: > > src/slave/containerizer/provisioner/docker/registry_client.hpp, line 67 > > > > > > Please let's avoid conflating independent changes in these patches, as > > it

Re: Review Request 38337: Extract gz file in fetcher.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38337/#review101644 --- Ship it! Ship It! - Bernd Mathiske On Oct. 5, 2015, 2:26 a.m.,

Re: Review Request 36180: Avoid multi writers write to same file in PortMappingIsolatorTests.

> On Oct. 6, 2015, 3:20 a.m., Cong Wang wrote: > > src/tests/containerizer/port_mapping_tests.cpp, line 270 > > > > > > traffic_invalid_via_loopback is not good either, because there is no > > traffic from an

Re: Review Request 39019: Windows: Added dirent compat code for non-Unix systems.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39019/#review101663 ---

Re: Review Request 39005: stout: Added thread-safe replacement for strerror.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39005/#review101665 --- 3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp (line

Re: Review Request 39005: stout: Added thread-safe replacement for strerror.

> On Oct. 6, 2015, 5:09 p.m., Cong Wang wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp, line 37 > > > > > > When strerror_r() returns EINVAL, you return an empty string? Also, you should force

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

> On Oct. 6, 2015, 1:04 a.m., haosdent huang wrote: > > src/slave/slave.cpp, line 2389 > > > > > > I think just print a number is not easy to understand when troubleshoot > > error happens. > > Anand Mazumdar

Re: Review Request 39043: Added support for HTTP Authentication in Mesos.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39043/#review101650 --- Patch looks great! Reviews applied: [37714, 37996, 37997, 37998,

Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38527/ --- (Updated Oct. 6, 2015, 4:47 p.m.) Review request for mesos, Ian Downes, Jie

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

> On Oct. 6, 2015, 6:35 a.m., haosdent huang wrote: > > src/slave/slave.cpp, line 2420 > > > > > > Should use return here directly instead of break? What is the advantage of one over the other here ? I was being

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

> On Oct. 6, 2015, 1:04 a.m., haosdent huang wrote: > > src/slave/slave.cpp, line 2389 > > > > > > I think just print a number is not easy to understand when troubleshoot > > error happens. Can you elaborate a

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38901/ --- (Updated Oct. 6, 2015, 10:29 a.m.) Review request for mesos, Jojy Varghese and

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

> On Oct. 6, 2015, 6:20 a.m., haosdent huang wrote: > > src/slave/slave.cpp, line 2479 > > > > > > Seems this line is incomplete. It was intentionally left this way to avoid warning as error issues. This would be