Re: Review Request 60784: Fixed a bug in operator== for DiskInfo::Source.

2017-07-13 Thread Neil Conway
enerated e-mail. To reply, visit: https://reviews.apache.org/r/60784/#review180384 --- On July 11, 2017, 10:29 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail.

Review Request 60784: Fixed a bug in operator== for DiskInfo::Source.

2017-07-11 Thread Neil Conway
55d493e89114acc94b1524f3f94a47ccea20469a Diff: https://reviews.apache.org/r/60784/diff/1/ Testing --- `make check` I tried to write a unit test for this specific problem but wasn't able to repro :-\ Current coding seems wrong / inconsistent in any case, though. Thanks, Neil Conway

Re: Review Request 59764: Ignore registration attempts by agents with misconfigured domain.

2017-07-11 Thread Neil Conway
--- `make check` Manual testing. Thanks, Neil Conway

Re: Review Request 59766: Changed allocator to offer remote resources to region-aware frameworks.

2017-07-11 Thread Neil Conway
/slave_recovery_tests.cpp f6eafcbe3d89c7a69c03db0fd7bc10ae73d06584 Diff: https://reviews.apache.org/r/59766/diff/6/ Changes: https://reviews.apache.org/r/59766/diff/5-6/ Testing --- `make check`. Thanks, Neil Conway

Re: Review Request 59762: Added domain to MasterInfo and SlaveInfo.

2017-07-10 Thread Neil Conway
/mesos.cpp 423510ef14025dba208ef85edf5305c2ce58f01d Diff: https://reviews.apache.org/r/59762/diff/5/ Changes: https://reviews.apache.org/r/59762/diff/4-5/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 59766: Changed allocator to offer remote resources to region-aware frameworks.

2017-07-10 Thread Neil Conway
/slave_recovery_tests.cpp f6eafcbe3d89c7a69c03db0fd7bc10ae73d06584 Diff: https://reviews.apache.org/r/59766/diff/5/ Changes: https://reviews.apache.org/r/59766/diff/4-5/ Testing --- `make check`. Thanks, Neil Conway

Re: Review Request 59761: Added master and agent flags to specify domain.

2017-07-07 Thread Neil Conway
Diff: https://reviews.apache.org/r/59761/diff/4/ Changes: https://reviews.apache.org/r/59761/diff/3-4/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 60346: Improved the documentation of 'TASK_LOST'.

2017-07-05 Thread Neil Conway
) <https://reviews.apache.org/r/60346/#comment254563> We don't support the "strict-registry" flag anymore, so I removed the reference to this before committing. - Neil Conway On June 21, 2017, 10:17 p.m., Gast

Re: Review Request 59762: Added domain to MasterInfo and SlaveInfo.

2017-07-05 Thread Neil Conway
423510ef14025dba208ef85edf5305c2ce58f01d Diff: https://reviews.apache.org/r/59762/diff/4/ Changes: https://reviews.apache.org/r/59762/diff/3-4/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 59921: Added agent domain to Offer message.

2017-07-05 Thread Neil Conway
://reviews.apache.org/r/59921/diff/2/ Changes: https://reviews.apache.org/r/59921/diff/1-2/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 59764: Ignore registration attempts by agents with misconfigured domain.

2017-07-05 Thread Neil Conway
k` Manual testing. Thanks, Neil Conway

Re: Review Request 59766: Changed allocator to offer remote resources to region-aware frameworks.

2017-07-05 Thread Neil Conway
/slave_recovery_tests.cpp f6eafcbe3d89c7a69c03db0fd7bc10ae73d06584 Diff: https://reviews.apache.org/r/59766/diff/4/ Changes: https://reviews.apache.org/r/59766/diff/3-4/ Testing --- `make check`. Thanks, Neil Conway

Re: Review Request 59760: Added REGION_AWARE framework capability.

2017-07-05 Thread Neil Conway
/diff/3-4/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 59763: Caused master to abort when joining a mixed-region cluster.

2017-07-05 Thread Neil Conway
/r/59763/diff/2-3/ Testing --- `make check` Manual testing. Thanks, Neil Conway

Re: Review Request 59759: Added protobuf definitions for fault domains.

2017-07-05 Thread Neil Conway
f/2/ Changes: https://reviews.apache.org/r/59759/diff/1-2/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 59761: Added master and agent flags to specify domain.

2017-07-05 Thread Neil Conway
Diff: https://reviews.apache.org/r/59761/diff/3/ Changes: https://reviews.apache.org/r/59761/diff/2-3/ Testing --- `make check` Thanks, Neil Conway

Review Request 60485: Fixed flaky PersistentVolumeEndpointsTest.ReserveAndSlaveRemoval.

2017-06-27 Thread Neil Conway
/ Testing --- `make check` Thanks, Neil Conway

Review Request 60417: Updated comments and help text in mesos-execute.

2017-06-24 Thread Neil Conway
--- Updated comments and help text in mesos-execute. Diffs - src/cli/execute.cpp 58eaa47bf8388424fd42f7830fdbb7cdecbebb7b Diff: https://reviews.apache.org/r/60417/diff/1/ Testing --- Visual inspection of help output. Thanks, Neil Conway

Review Request 60416: Avoided master crash on re-registration of old agents.

2017-06-24 Thread Neil Conway
://reviews.apache.org/r/60416/diff/1/ Testing --- `make check`, manual testing. Thanks, Neil Conway

Review Request 60415: Added comment to master logic for downgrading checkpointed resources.

2017-06-24 Thread Neil Conway
--- Added comment to master logic for downgrading checkpointed resources. Diffs - src/master/master.cpp 33eca0d17459781fdc2ea915e8f40c78dd306962 Diff: https://reviews.apache.org/r/60415/diff/1/ Testing --- Thanks, Neil Conway

Re: Review Request 60407: Avoided master crash on agent re-registration.

2017-06-24 Thread Neil Conway
added `TODO`s to note this for future improvement. - Neil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60407/#review178836 -----

Re: Review Request 60407: Avoided master crash on agent re-registration.

2017-06-24 Thread Neil Conway
, Neil Conway

Re: Review Request 60405: Documented resource format in agent <-> master protocol.

2017-06-24 Thread Neil Conway
ve.cpp f808458849bb9667a91abe18868751d377d36e0c Diff: https://reviews.apache.org/r/60405/diff/3/ Changes: https://reviews.apache.org/r/60405/diff/2-3/ Testing --- Thanks, Neil Conway

Re: Review Request 60405: Documented resource format in agent <-> master protocol.

2017-06-24 Thread Neil Conway
sources, but that is fine and expected. Should we really be cluttering the logs with this information? - Neil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60405/#review178834 --- On June 24, 2017, 1:48 a.m., Neil Conwa

Re: Review Request 60405: Documented resource format in agent <-> master protocol.

2017-06-24 Thread Neil Conway
ould we really be cluttering the logs with this information? - Neil Conway On June 24, 2017, 1:48 a.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit:

Re: Review Request 60407: Avoided master crash on agent re-registration.

2017-06-23 Thread Neil Conway
it is best-effort anyway, this seems tolerable. Diffs (updated) - src/master/validation.cpp 33e9ff7db9e2789cbb2d6dfd015288dfa1faa7c5 Diff: https://reviews.apache.org/r/60407/diff/2/ Changes: https://reviews.apache.org/r/60407/diff/1-2/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 60404: Documented the content of the `SlaveInfo.resources` field.

2017-06-23 Thread Neil Conway
Diff: https://reviews.apache.org/r/60404/diff/2/ Changes: https://reviews.apache.org/r/60404/diff/1-2/ Testing --- Thanks, Neil Conway

Re: Review Request 60405: Documented resource format in agent <-> master protocol.

2017-06-23 Thread Neil Conway
d36e0c Diff: https://reviews.apache.org/r/60405/diff/2/ Changes: https://reviews.apache.org/r/60405/diff/1-2/ Testing --- Thanks, Neil Conway

Review Request 60407: Avoided master crash on agent re-registration.

2017-06-23 Thread Neil Conway
/validation.cpp 33e9ff7db9e2789cbb2d6dfd015288dfa1faa7c5 Diff: https://reviews.apache.org/r/60407/diff/1/ Testing --- `make check` Thanks, Neil Conway

Review Request 60406: Added sanity check to resource downgrade on agent registration.

2017-06-23 Thread Neil Conway
Testing --- Thanks, Neil Conway

Review Request 60405: Documented resource format in agent <-> master protocol.

2017-06-23 Thread Neil Conway
--- Documented resource format in agent <-> master protocol. Diffs - src/messages/messages.proto 2c086263fdcee4d54a76a61379c2d4dba5271d23 Diff: https://reviews.apache.org/r/60405/diff/1/ Testing --- Thanks, Neil Conway

Review Request 60404: Documented the content of the `SlaveInfo.resources` field.

2017-06-23 Thread Neil Conway
Diff: https://reviews.apache.org/r/60404/diff/1/ Testing --- Thanks, Neil Conway

Re: Review Request 60351: Updated the `operator<<` for repeated resources to use `JSON::protobuf`.

2017-06-22 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60351/#review178758 --- Ship it! Ship It! - Neil Conway On June 22, 2017, 12:08 a.m

Re: Review Request 60352: Relaxed the resource format restriction for frameworks.

2017-06-22 Thread Neil Conway
to commit message. - Neil Conway On June 22, 2017, 12:32 a.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 60283: Fixed `convertResourceFormat` uses with `validateAndUpgradeResources`.

2017-06-22 Thread Neil Conway
(patched) <https://reviews.apache.org/r/60283/#comment252927> We use `EXPECT_EQ` in the other tests here. - Neil Conway On June 22, 2017, 10:22 p.m., Michael Park wrote: > > --- > This is an automatically generated e

Re: Review Request 60284: Removed unused `convertResourceFormat` for `Operation`.

2017-06-22 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60284/#review178752 --- Ship it! Ship It! - Neil Conway On June 21, 2017, 7:08 p.m

Re: Review Request 60255: Prevented reserve/create with refined reservation on non-capable agents.

2017-06-22 Thread Neil Conway
tests for this -- similar to `CreateOperationValidationTest.AgentHierarchicalRoleCapability` - Neil Conway On June 20, 2017, 11:58 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit

Re: Review Request 60283: Fixed `convertResourceFormat` uses with `validateAndUpgradeResources`.

2017-06-22 Thread Neil Conway
g/r/60283/#comment252920> "an UNRESERVE" src/tests/master_tests.cpp Lines 7407 (patched) <https://reviews.apache.org/r/60283/#comment252922> "we test" - Neil Conway On June 21, 2017, 7:08 p.m., Michael Park wrote: > > -

Re: Review Request 60374: Required that resources are always downgraded for a non-capable agent.

2017-06-22 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60374/#review178743 --- Ship it! Ship It! - Neil Conway On June 22, 2017, 7:09 p.m

Review Request 60377: Added test for reservation refinements and old agents.

2017-06-22 Thread Neil Conway
--- `make check`, ran ~5k iterations w/o error. Thanks, Neil Conway

Re: Review Request 60281: Avoided sending an inconsistent `TaskInfo` to the allocator.

2017-06-22 Thread Neil Conway
) <https://reviews.apache.org/r/60281/#comment252901> This comment needs updating. - Neil Conway On June 21, 2017, 7:06 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 60374: Required that resources are always downgraded for a non-capable agent.

2017-06-22 Thread Neil Conway
) <https://reviews.apache.org/r/60374/#comment252897> Can we add a comment explaining why these are `CHECK`s? - Neil Conway On June 22, 2017, 7:09 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 60283: Fixed `convertResourceFormat` uses with `validateAndUpgradeResources`.

2017-06-22 Thread Neil Conway
ched) <https://reviews.apache.org/r/60283/#comment252856> Whitespace. src/tests/master_tests.cpp Lines 7202 (patched) <https://reviews.apache.org/r/60283/#comment252857> Can we remove this? Seems irrelevant to the test's purpose. - Neil Conway On June 21, 2017, 7:08

Re: Review Request 60351: Used `JSON::protobuf` to print resources not validated nor converted.

2017-06-22 Thread Neil Conway
<<` for `RepeatedPtrField`. The current behavior is to silently omit printing invalid resources, which seems very misleading. This would also avoid the risk of random `CHECK` failures if a code path attempts to print a resource before upgrading it. - Neil Conway On June 22, 2017, 12:

Re: Review Request 60282: Introduced `validateAndUpgradeResources`.

2017-06-22 Thread Neil Conway
tions, and tasks" in commit message: remove the comma. "a `upgradeResources`" in commit message: "a `validateAndUpgradeResources`" src/common/resources_utils.hpp Lines 141 (patched) <https://reviews.apache.org/r/60282/#comment252854> "in their previous format"

Re: Review Request 60281: Added the adjusted `TaskInfo` into the `Operation` to be sent out.

2017-06-22 Thread Neil Conway
`TaskInfo` is actually being sent to the allocator? The agent gets the correct `TaskInfo`. - Neil Conway On June 21, 2017, 7:06 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit

Re: Review Request 60281: Added the adjusted `TaskInfo` into the `Operation` to be sent out.

2017-06-21 Thread Neil Conway
) <https://reviews.apache.org/r/60281/#comment252729> If we're already copying the operation (see `foreach` a few lines up), do we need to make an additional copy of the `TaskInfo`? If we mutate w/o copying, we'd avoid the need to introduce a second variable. - Neil Conway On June 21, 2017

Re: Review Request 60283: Fixed `convertResourceFormat` uses with `validateAndUpgradeResources`.

2017-06-21 Thread Neil Conway
) <https://reviews.apache.org/r/60283/#comment252728> Update this comment. - Neil Conway On June 21, 2017, 7:08 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 60284: Removed unused `convertResourceFormat` for `Operation`.

2017-06-21 Thread Neil Conway
tps://reviews.apache.org/r/60284/#comment252588> Also need to update the header file. - Neil Conway On June 21, 2017, 7:08 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 60283: Fixed the uses of `convertResourceFormat` with `upgradeResources`.

2017-06-21 Thread Neil Conway
ws.apache.org/r/60283/#comment252587> If we can use `Option` for `upgradeResources`, can we avoid duplicating all this error handling code? - Neil Conway On June 21, 2017, 7:08 p.m., Michael Park wrote: > > --- > This is

Re: Review Request 60282: Introduced `upgradeResources` which also validates the resources.

2017-06-21 Thread Neil Conway
src/common/resources_utils.cpp Lines 274 (patched) <https://reviews.apache.org/r/60282/#comment252583> Hmmm -- this means that we're going to validate most resources twice in most code paths, right? That seems unfortunate, although I guess it isn't easy to avoid. - Neil Conway On J

Re: Review Request 60281: Added the adjusted `TaskInfo` into the `Operation` to be sent out.

2017-06-21 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60281/#review178533 --- Can we write a unit test for this change? - Neil Conway

Re: Review Request 60254: Added missing `RESERVATION_REFINEMENT` capability to examples and tests.

2017-06-20 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60254/#review178446 --- Ship it! Ship It! - Neil Conway On June 20, 2017, 11:35 p.m

Re: Review Request 60253: Placed the `convertResourceFormat` calls after resource validation.

2017-06-20 Thread Neil Conway
, we could consider combining validation and format conversion into a single helper; or perhaps have an "try adding `Resource` to `Resources`" that validates the `Resource`, converts it to the right format, and then either returns the updated `Resources` or an error if validation. - N

Re: Review Request 60038: Prevented allocating reservation refinements to non-capable frameworks.

2017-06-20 Thread Neil Conway
sible to translate the refined reservations into the old format" -- it seems to me it is explicitly _not_ possible to accurately translate refined reservations into the old format, which is the reason for this change. - Neil Conway On June 19, 2017, 9:52 p.m., Michael

Re: Review Request 60244: Tidied up lambda capture list.

2017-06-20 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60244/#review178401 --- Ship it! Ship It! - Neil Conway On June 20, 2017, 2:22 p.m

Re: Review Request 60218: Added a couple of missing validation logic + minor improvements.

2017-06-19 Thread Neil Conway
(patched) <https://reviews.apache.org/r/60218/#comment252238> Also need to update v1 - Neil Conway On June 20, 2017, 12:57 a.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 60219: Introduced `downgradeResources` to facilitate out-bound resources.

2017-06-19 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60219/#review178334 --- Ship it! - Neil Conway On June 20, 2017, 12:59 a.m., Michael

Re: Review Request 60070: Adjusted the agent to the new resources format [11/N].

2017-06-19 Thread Neil Conway
ps://reviews.apache.org/r/60070/#comment252202> is "send the result" accurate here? "checkpoint the result" instead? - Neil Conway On June 14, 2017, 9:56 a.m., Michael Park wrote: > > --- > Thi

Re: Review Request 60190: Added the agent `RESERVATION_REFINEMENT` capability.

2017-06-19 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60190/#review178315 --- Ship it! Ship It! - Neil Conway On June 19, 2017, 6:43 a.m

Re: Review Request 60210: Disabled emitting unset fields with default values in tests.

2017-06-19 Thread Neil Conway
., are we always omitting default values to ensure we roundtrip, or is it just good practice in these situations, etc.? - Neil Conway On June 19, 2017, 8:43 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 60207: Added `emit_default_value` option to `JSON::protobuf`.

2017-06-19 Thread Neil Conway
a boolean. e.g., `OMIT_DEFAULT_VALUES`, `INCLUDE_DEFAULT_VALUES`. - Neil Conway On June 19, 2017, 8:40 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 60209: Fixed the `serialize` function to respect the protobuf format.

2017-06-19 Thread Neil Conway
omment explaining what is going on here? - Neil Conway On June 19, 2017, 8:40 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 60211: Added RESERVATION_REFINEMENT capability for example frameworks.

2017-06-19 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60211/#review178298 --- Ship it! Ship It! - Neil Conway On June 19, 2017, 8:45 p.m

Re: Review Request 60207: Added `emit_default_value` option to `JSON::protobuf`.

2017-06-19 Thread Neil Conway
/include/stout/protobuf.hpp Line 835 (original), 835 (patched) <https://reviews.apache.org/r/60207/#comment252183> Update this comment. - Neil Conway On June 19, 2017, 8:40 p.m., Michael Park wrote: > > --- > This is a

Re: Review Request 59760: Added REGION_AWARE framework capability.

2017-06-19 Thread Neil Conway
/59760/diff/2-3/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 59760: Added REGION_AWARE framework capability.

2017-06-19 Thread Neil Conway
--- `make check` Thanks, Neil Conway

Re: Review Request 60070: Adjusted the agent to the new resources format [11/N].

2017-06-19 Thread Neil Conway
<https://reviews.apache.org/r/60070/#comment252063> Wrong enum name? src/slave/state.cpp Lines 785 (patched) <https://reviews.apache.org/r/60070/#comment252064> Fix enum name - Neil Conway On Ju

Re: Review Request 60189: Prevented allocating non-capable agent resources to capable frameworks.

2017-06-19 Thread Neil Conway
rk filters these resources". Similar with the comment for `isFiltered` in hierarchical.hpp - Neil Conway On June 19, 2017, 6:43 a.m., Michael Park wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 60041: Converted the resources to the "endpoint" format for V0 endpoints.

2017-06-19 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60041/#review178191 --- Ship it! Ship It! - Neil Conway On June 13, 2017, 8:43 a.m

Re: Review Request 60187: Converted the resources to the "endpoint" format for V1 endpoints.

2017-06-19 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60187/#review178192 --- Ship it! Ship It! - Neil Conway On June 19, 2017, 6:39 a.m

Re: Review Request 60070: Adjusted the agent to the new resources format [11/N].

2017-06-19 Thread Neil Conway
) <https://reviews.apache.org/r/60070/#comment252058> Seems like this is in the wrong RR? - Neil Conway On June 14, 2017, 9:56 a.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 60069: Adjusted the master to the new resources format [10/N].

2017-06-19 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60069/#review178189 --- Ship it! Ship It! - Neil Conway On June 14, 2017, 9:56 a.m

Re: Review Request 60040: Resources: Adjusted `parse` to produce the new resource format [6/N].

2017-06-19 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60040/#review178188 --- Ship it! Ship It! - Neil Conway On June 13, 2017, 8:42 a.m

Re: Review Request 60017: Resources: Adjusted the utilities to the new resource format [5/N].

2017-06-19 Thread Neil Conway
(original), 1501 (patched) <https://reviews.apache.org/r/60017/#comment252057> Per discussion, should instead result in a statically reserved resource whose role is the last role in the reservation stack. - Neil Conway On June 13, 2017, 8:35 a.m., Michael Park

Re: Review Request 60185: Adjusted the master validation to the new resource format [9/N].

2017-06-19 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60185/#review178186 --- Ship it! Ship It! - Neil Conway On June 19, 2017, 6:19 a.m

Re: Review Request 60182: Resources: Adjusted the predicates to the new resource format [2/N].

2017-06-19 Thread Neil Conway
Line 267 (original), 267 (patched) <https://reviews.apache.org/r/60182/#comment252056> "is validated, and is in the ..." - Neil Conway On June 19, 2017, 5:23 a.m., Michael Park wrote: > > --- > This is an autom

Re: Review Request 60184: Resources: Adjusted the `operator<<` to the new resource format [4/N].

2017-06-19 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60184/#review178185 --- Ship it! Ship It! - Neil Conway On June 19, 2017, 5:26 a.m

Re: Review Request 60016: Resources: Adjusted the comparators to new resource format [3/N].

2017-06-19 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60016/#review178183 --- Ship it! Ship It! - Neil Conway On June 13, 2017, 8:35 a.m

Re: Review Request 60017: Resources: Adjusted the utilities to the new resource format [5/N].

2017-06-19 Thread Neil Conway
change for reservation refinements, not a change to just adapt to the new format? In any case, seems like a comment would be helpful. - Neil Conway On June 13, 2017, 8:35 a.m., Michael Park wrote: > > --- > This is an automatically ge

Re: Review Request 60040: Resources: Adjusted `parse` to produce the new resource format [6/N].

2017-06-19 Thread Neil Conway
ough. src/v1/resources.cpp Lines 663 (patched) <https://reviews.apache.org/r/60040/#comment252053> Should the `!resource.has_reservation()` be a `CHECK` instead? src/v1/resources.cpp Lines 672 (patched) <https://reviews.apache.org/r/60040/#comment252052> `CopyFrom`, I'

Re: Review Request 60181: Resources: Updated the comment to mention the resource format [1/N].

2017-06-18 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60181/#review178178 --- Ship it! Ship It! - Neil Conway On June 19, 2017, 5:17 a.m

Re: Review Request 60015: Introduced a utility function `Resources::reservationRole`.

2017-06-18 Thread Neil Conway
(patched) <https://reviews.apache.org/r/60015/#comment252046> Personally, I think this is a bit more readable: ``` if (isUnreserved(resource)) { return false; } return role.isNone() || role.get() == reservationRole(resource); ``` - Neil Conway On J

Re: Review Request 60178: Added test for endpoint resource format.

2017-06-18 Thread Neil Conway
/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 60036: Avoided exposing `role` in JSON for resources with refined reservations.

2017-06-18 Thread Neil Conway
che.org/r/60036/#comment252038> Backticks not double quotes? src/common/http.cpp Lines 526 (patched) <https://reviews.apache.org/r/60036/#comment252039> Can we add `is` checks here, for safety's sake? - Neil Conway On June 15, 2017, 7:39

Re: Review Request 60036: Avoided exposing `role` in JSON for resources with refined reservations.

2017-06-18 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60036/#review178173 --- Ship it! Ship It! - Neil Conway On June 15, 2017, 7:39 a.m

Re: Review Request 60022: Introduced `convertResourceFormat` to convert between resource formats.

2017-06-18 Thread Neil Conway
eviews.apache.org/r/60022/#comment252033> I feel like `reservation->CopyFrom(resource->reservation())` would be more idiomatic...? - Neil Conway On June 15, 2017, 8:48 a.m., Michael Park wrote: > >

Re: Review Request 60178: Added test for endpoint resource format.

2017-06-18 Thread Neil Conway
/ Changes: https://reviews.apache.org/r/60178/diff/1-2/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 60174: Updated the resources validation logic to allow for the endpoint format.

2017-06-18 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60174/#review178168 --- Ship it! Ship It! - Neil Conway On June 18, 2017, 10:02 p.m

Review Request 60178: Added test for endpoint resource format.

2017-06-18 Thread Neil Conway
--- Added test for endpoint resource format. Diffs - src/tests/master_validation_tests.cpp 83370fa5653fe5da666ec577b05001c4a5499848 Diff: https://reviews.apache.org/r/60178/diff/1/ Testing --- `make check` Thanks, Neil Conway

Re: Review Request 60174: Updated the resources validation logic to allow for the endpoint format.

2017-06-18 Thread Neil Conway
validation to support this new format? i.e., the commit message says the master is going to _emit_ this weird format, but it doesn't necessarily follow why the master needs to _accept_ inputs in this format. Seems like we should update the tests for this change, right? - Neil Conway On June 18

Re: Review Request 60175: Introduced a utility function `Resources::hasRefinedReservations`.

2017-06-18 Thread Neil Conway
? - Neil Conway On June 18, 2017, 10:03 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 59454: Adjust tests to account for GCC 7.1 fix in bytes.hpp.

2017-06-18 Thread Neil Conway
-- there isn't anything specific about GCC 7.1 in this fix. - Neil Conway On May 24, 2017, 5:53 p.m., Aaron Wood wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 59861: Added protobuf changes for reservation refinement.

2017-06-16 Thread Neil Conway
v1/mesos.proto Lines 367 (patched) <https://reviews.apache.org/r/59861/#comment251967> Typo include/mesos/v1/mesos.proto Lines 1151 (patched) <https://reviews.apache.org/r/59861/#comment251969> Remove comma: "reservation's role," - Neil Conway On June 16, 2017, 10

Re: Review Request 60062: Prevented reserve/create with hierarchical roles on a non-capable agent.

2017-06-16 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60062/#review178122 --- Ship it! Ship It! - Neil Conway On June 16, 2017, 7:48 p.m

Re: Review Request 60139: Avoided needless copy of `Owned` in libprocess.

2017-06-16 Thread Neil Conway
I'll just give up on this, and we can try to clean it up properly as part of fixing `Owned`. - Neil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60139/#review178098 --

Review Request 60162: Fixed bug in GroupTest.ConnectTimer, GroupTest.TimerCleanup.

2017-06-16 Thread Neil Conway
, `GroupTest.ConnectTimer` fails. With this change, the test passes (both with and without the fix for MESOS-5886 applied). Thanks, Neil Conway

Review Request 60161: Cleaned up zookeeper binding code slightly.

2017-06-16 Thread Neil Conway
, Neil Conway

Re: Review Request 60070: Updated the agent to account for the new resources format.

2017-06-15 Thread Neil Conway
://reviews.apache.org/r/60070/#comment251772> Are we using `const` here but not elsewhere in the RR for a reason? - Neil Conway On June 14, 2017, 9:56 a.m., Michael Park wrote: > > --- > This is an automatically generated

Review Request 60139: Avoided needless copy of `Owned` in libprocess.

2017-06-15 Thread Neil Conway
--- `make check` Thanks, Neil Conway

  1   2   3   4   5   6   7   8   9   10   >