You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2017/04/05 17:54:39 UTC
Review Request 58214: Fixed a regression hiding previously exposed
master and agent flags.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58214/
-----------------------------------------------------------
Review request for mesos and Michael Park.
Bugs: MESOS-7316
https://issues.apache.org/jira/browse/MESOS-7316
Repository: mesos
Description
-------
In f441eb9 we in a number of places changed how 'Flag's were added to
'Flags' by moving from ad-hoc invocations of 'FlagsBase::add' on
particular instances to proper 'Flags' member variables. This was needed
to ensure 'Flags' instances could always safely be copied. For that we
introduced local derived 'Flags' classes to support localized parsing
needs. At the same time, this implementation strategy led to these these
local variables not being accessible through instances of the original
class anymore (this was inevitable when making 'Flags' classes properly
copyable), which e.g., causes a regression in the flags displayed in a
master's '/flags' endpoint.
This commit moves the flags into the respective base class removing the
local classes so that all passed flags are exposed to users.
Diffs
-----
src/master/flags.hpp 41a0edf
src/master/flags.cpp d25cfdd
src/master/main.cpp fa7ba13
src/slave/flags.hpp 2c4bd6a
src/slave/flags.cpp 71935de
src/slave/main.cpp a124d2e
Diff: https://reviews.apache.org/r/58214/diff/1/
Testing
-------
`make check` (Fedora 25)
This is a backport to `1.2.x` of the https://reviews.apache.org/r/57994/ which applied against the then `master` branch (1.3 in spe).
Thanks,
Benjamin Bannier
Re: Review Request 58214: Fixed a regression hiding previously exposed
master and agent flags.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58214/#review172153
-----------------------------------------------------------
Ship it!
Ship It!
- Michael Park
On April 5, 2017, 10:54 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58214/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 10:54 a.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Bugs: MESOS-7316
> https://issues.apache.org/jira/browse/MESOS-7316
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In f441eb9 we in a number of places changed how 'Flag's were added to
> 'Flags' by moving from ad-hoc invocations of 'FlagsBase::add' on
> particular instances to proper 'Flags' member variables. This was needed
> to ensure 'Flags' instances could always safely be copied. For that we
> introduced local derived 'Flags' classes to support localized parsing
> needs. At the same time, this implementation strategy led to these these
> local variables not being accessible through instances of the original
> class anymore (this was inevitable when making 'Flags' classes properly
> copyable), which e.g., causes a regression in the flags displayed in a
> master's '/flags' endpoint.
>
> This commit moves the flags into the respective base class removing the
> local classes so that all passed flags are exposed to users.
>
>
> Diffs
> -----
>
> src/master/flags.hpp 41a0edf
> src/master/flags.cpp d25cfdd
> src/master/main.cpp fa7ba13
> src/slave/flags.hpp 2c4bd6a
> src/slave/flags.cpp 71935de
> src/slave/main.cpp a124d2e
>
>
> Diff: https://reviews.apache.org/r/58214/diff/1/
>
>
> Testing
> -------
>
> `make check` (Fedora 25)
>
> This is a backport to `1.2.x` of the https://reviews.apache.org/r/57994/ which applied against the then `master` branch (1.3 in spe).
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 58214: Fixed a regression hiding previously exposed
master and agent flags.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58214/#review171160
-----------------------------------------------------------
Bad patch!
Reviews applied: [58214]
Failed command: python support/apply-reviews.py -n -r 58214
Error:
2017-04-05 18:45:18 URL:https://reviews.apache.org/r/58214/diff/raw/ [11865/11865] -> "58214.patch" [1]
error: patch failed: src/master/flags.hpp:17
error: src/master/flags.hpp: patch does not apply
error: patch failed: src/master/flags.cpp:592
error: src/master/flags.cpp: patch does not apply
error: patch failed: src/master/main.cpp:128
error: src/master/main.cpp: patch does not apply
error: patch failed: src/slave/flags.hpp:17
error: src/slave/flags.hpp: patch does not apply
error: patch failed: src/slave/flags.cpp:953
error: src/slave/flags.cpp: patch does not apply
error: patch failed: src/slave/main.cpp:91
error: src/slave/main.cpp: patch does not apply
Full log: https://builds.apache.org/job/Mesos-Reviewbot/17672/console
- Mesos Reviewbot
On April 5, 2017, 5:54 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58214/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 5:54 p.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Bugs: MESOS-7316
> https://issues.apache.org/jira/browse/MESOS-7316
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In f441eb9 we in a number of places changed how 'Flag's were added to
> 'Flags' by moving from ad-hoc invocations of 'FlagsBase::add' on
> particular instances to proper 'Flags' member variables. This was needed
> to ensure 'Flags' instances could always safely be copied. For that we
> introduced local derived 'Flags' classes to support localized parsing
> needs. At the same time, this implementation strategy led to these these
> local variables not being accessible through instances of the original
> class anymore (this was inevitable when making 'Flags' classes properly
> copyable), which e.g., causes a regression in the flags displayed in a
> master's '/flags' endpoint.
>
> This commit moves the flags into the respective base class removing the
> local classes so that all passed flags are exposed to users.
>
>
> Diffs
> -----
>
> src/master/flags.hpp 41a0edf
> src/master/flags.cpp d25cfdd
> src/master/main.cpp fa7ba13
> src/slave/flags.hpp 2c4bd6a
> src/slave/flags.cpp 71935de
> src/slave/main.cpp a124d2e
>
>
> Diff: https://reviews.apache.org/r/58214/diff/1/
>
>
> Testing
> -------
>
> `make check` (Fedora 25)
>
> This is a backport to `1.2.x` of the https://reviews.apache.org/r/57994/ which applied against the then `master` branch (1.3 in spe).
>
>
> Thanks,
>
> Benjamin Bannier
>
>