Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 25, 2017, 8:33 a.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 24, 2017, 3:32 p.m., Michael Park wrote: > > src/master/master.hpp, lines 2490-2504 > > > > > > What do you think about pulling the roles retrieval out? > > > > ```cpp > > auto getRoles =

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 14, 2017, 10:04 p.m., Guangya Liu wrote: > > src/tests/master_validation_tests.cpp, line 2616 > > > > > > How about s/RejectRolesChange/RejectRolesChangeWithMutiRole > > Benjamin Bannier wrote: >

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 25, 2017, 12:32 a.m., Michael Park wrote: > > src/master/master.hpp, lines 2490-2504 > > > > > > What do you think about pulling the roles retrieval out? > > > > ```cpp > > auto getRoles =

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 25, 2017, 1:41 a.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review162878 --- Fix it, then Ship it! src/master/master.hpp (lines 2490 -

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 14, 2017, 10:04 p.m., Guangya Liu wrote: > > src/tests/master_validation_tests.cpp, line 2616 > > > > > > How about s/RejectRolesChange/RejectRolesChangeWithMutiRole > > Benjamin Bannier wrote: >

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 14, 2017, 1:04 p.m., Guangya Liu wrote: > > src/tests/master_validation_tests.cpp, line 2616 > > > > > > How about s/RejectRolesChange/RejectRolesChangeWithMutiRole > > Benjamin Bannier wrote: > When

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review162607 --- Patch looks great! Reviews applied: [55381, 55571, 55271]

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 23, 2017, 9 a.m.) Review request for mesos, Benjamin Mahler, Jay

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 20, 2017, 7:30 p.m., Michael Park wrote: > > src/master/master.hpp, lines 2475-2516 > > > > > > We set the `validationError` to `None` just above, so this should > > always be `true`. It looks to me

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 21, 2017, 4:30 a.m., Michael Park wrote: > > src/master/master.hpp, lines 2475-2516 > > > > > > We set the `validationError` to `None` just above, so this should > > always be `true`. It looks to me

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 21, 2017, 1:39 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review162535 --- src/master/master.hpp (lines 2475 - 2516)

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 20, 2017, 6:53 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 19, 2017, 2:01 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review162253 --- src/tests/master_validation_tests.cpp (lines 2774 - 2776)

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review162226 --- Patch looks great! Reviews applied: [55381, 55571, 55271]

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 18, 2017, 8:07 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 18, 2017, 4:25 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 18, 2017, 3:36 a.m., Jay Guo wrote: > > src/master/master.hpp, lines 2522-2528 > > > > > > This check is only valid **iff** both `src` and `dest` frameworkInfo > > are single role, isn't it? maybe > >

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 17, 2017, 10:10 a.m., Jay Guo wrote: > > Also, should we allow user to downgrade from a multi-role framework to > > single-role? I feel it would be very complicated and we should explicitly > > disallow that... > > Benjamin Bannier wrote: > I am not sure this is required. We

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review162021 --- src/master/master.hpp (lines 2522 - 2528)

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 17, 2017, 5:10 p.m., Jay Guo wrote: > > Also, should we allow user to downgrade from a multi-role framework to > > single-role? I feel it would be very complicated and we should explicitly > > disallow that... > > Benjamin Bannier wrote: > I am not sure this is required. We

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 17, 2017, 10:10 a.m., Jay Guo wrote: > > Also, should we allow user to downgrade from a multi-role framework to > > single-role? I feel it would be very complicated and we should explicitly > > disallow that... I am not sure this is required. We already make sure elsewhere that (1)

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 17, 2017, 11:20 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review161909 --- Patch looks great! Reviews applied: [55381, 55571, 55271]

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 17, 2017, 6:14 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review161828 --- Also, should we allow user to downgrade from a multi-role

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review161737 --- Patch looks great! Reviews applied: [55381, 55571, 55271]

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 16, 2017, 5:58 a.m., Jay Guo wrote: > > src/master/master.cpp, line 2478 > > > > > > I think we don't necessarily have `framework` here if framework > > information hasn't been reconstructed from agent

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 16, 2017, 12:28 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 16, 2017, 12:58 p.m., Jay Guo wrote: > > src/master/master.cpp, lines 2491-2492 > > > > > > Note that you could use `framework.capabilities` here since > > https://reviews.apache.org/r/55021/ is landed.

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 16, 2017, 10:54 a.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 16, 2017, 5:58 a.m., Jay Guo wrote: > > I wonder how we deal with default role `*` in multi-role scenario? I > > remember that we require `*` to be explicitly specified for a multi-role > > framework, is that still true? This is not how I read the design doc, but this is only

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

> On Jan. 14, 2017, 10:04 p.m., Guangya Liu wrote: > > src/master/master.cpp, line 2490 > > > > > > I think that you may want to > > ``` > > #include > > ``` This is already added as part of this

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review161678 --- I wonder how we deal with default role `*` in multi-role

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review161648 --- src/master/master.cpp (lines 2473 - 2506)

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review161397 --- Bad patch! Reviews applied: [55271, 55381] Failed command:

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 12, 2017, 4:32 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

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

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 11, 2017, 11:37 a.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

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

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 10, 2017, 3:49 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- (Updated Jan. 10, 2017, 1:29 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/#review160813 --- You may also want a test case in

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

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

Review Request 55271: Disallow multi-role frameworks to change their roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55271/ --- Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu. Bugs: