You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Guangya Liu <gy...@apache.org> on 2017/02/09 15:50:23 UTC
Review Request 56499: Added roles field to framework.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56499/
-----------------------------------------------------------
Review request for mesos, Benjamin Mahler and Jay Guo.
Bugs: MESOS-6638
https://issues.apache.org/jira/browse/MESOS-6638
Repository: mesos
Description
-------
Added roles field to framework.
Diffs
-----
src/master/master.hpp 767ac48f2a65514a19c4a3f8ca7b35f0259f9aeb
src/master/master.cpp 620919ecfe85367b5c1281afc5216cc20e5e2e3c
Diff: https://reviews.apache.org/r/56499/diff/
Testing
-------
make
make check
Thanks,
Guangya Liu
Re: Review Request 56499: Added roles field to framework.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56499/#review165002
-----------------------------------------------------------
Patch looks great!
Reviews applied: [56499]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Feb. 9, 2017, 3:50 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56499/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2017, 3:50 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jay Guo.
>
>
> Bugs: MESOS-6638
> https://issues.apache.org/jira/browse/MESOS-6638
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added roles field to framework.
>
>
> Diffs
> -----
>
> src/master/master.hpp 767ac48f2a65514a19c4a3f8ca7b35f0259f9aeb
> src/master/master.cpp 620919ecfe85367b5c1281afc5216cc20e5e2e3c
>
> Diff: https://reviews.apache.org/r/56499/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 56499: Added roles field to framework.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Feb. 9, 2017, 9:38 p.m., Benjamin Mahler wrote:
> > src/master/master.hpp, lines 2425-2451
> > <https://reviews.apache.org/r/56499/diff/1/?file=1628557#file1628557line2425>
> >
> > We need to update the newly introduced `roles` field here. Also, oldRoles can be removed in favor of directly accessing `roles`.
>
> Guangya Liu wrote:
> Since we do not allow updating `roles` for frameworks and the `roles` was already initialized when construct the framework, so do we still need to update the `roles` here?
Ah right, dropping this.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56499/#review165008
-----------------------------------------------------------
On Feb. 10, 2017, 12:24 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56499/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2017, 12:24 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jay Guo.
>
>
> Bugs: MESOS-6638
> https://issues.apache.org/jira/browse/MESOS-6638
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added roles field to framework.
>
>
> Diffs
> -----
>
> src/master/master.hpp 767ac48f2a65514a19c4a3f8ca7b35f0259f9aeb
> src/master/master.cpp 620919ecfe85367b5c1281afc5216cc20e5e2e3c
>
> Diff: https://reviews.apache.org/r/56499/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 56499: Added roles field to framework.
Posted by Guangya Liu <gy...@apache.org>.
> On \u4e8c\u6708 9, 2017, 9:38 p.m., Benjamin Mahler wrote:
> > src/master/master.hpp, lines 2425-2451
> > <https://reviews.apache.org/r/56499/diff/1/?file=1628557#file1628557line2425>
> >
> > We need to update the newly introduced `roles` field here. Also, oldRoles can be removed in favor of directly accessing `roles`.
Since we do not allow updating `roles` for frameworks and the `roles` was already initialized when construct the framework, so do we still need to update the `roles` here?
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56499/#review165008
-----------------------------------------------------------
On \u4e8c\u6708 10, 2017, 12:24 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56499/
> -----------------------------------------------------------
>
> (Updated \u4e8c\u6708 10, 2017, 12:24 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jay Guo.
>
>
> Bugs: MESOS-6638
> https://issues.apache.org/jira/browse/MESOS-6638
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added roles field to framework.
>
>
> Diffs
> -----
>
> src/master/master.hpp 767ac48f2a65514a19c4a3f8ca7b35f0259f9aeb
> src/master/master.cpp 620919ecfe85367b5c1281afc5216cc20e5e2e3c
>
> Diff: https://reviews.apache.org/r/56499/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 56499: Added roles field to framework.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56499/#review165008
-----------------------------------------------------------
Thanks! We need to ensure we update this field when updating the framework info (see comment below).
Also, since we can construct this via the FrameworkInfo, we don't need to take this as a constructor argument to the framework struct, since this just introduces the potential for a caller to get this wrong.
src/master/master.hpp (line 2163)
<https://reviews.apache.org/r/56499/#comment236831>
No need to pass it as an argument, we can just construct it from the already provided FrameworkInfo.
src/master/master.hpp (lines 2425 - 2451)
<https://reviews.apache.org/r/56499/#comment236835>
We need to update the newly introduced `roles` field here. Also, oldRoles can be removed in favor of directly accessing `roles`.
src/master/master.cpp (lines 2548 - 2553)
<https://reviews.apache.org/r/56499/#comment236832>
No change needed here, since we can just construct the roles based on the framework info.
src/master/master.cpp (lines 2842 - 2847)
<https://reviews.apache.org/r/56499/#comment236833>
Ditto here, no change needed.
src/master/master.cpp (lines 7475 - 7479)
<https://reviews.apache.org/r/56499/#comment236834>
Ditto here, no change needed.
- Benjamin Mahler
On Feb. 9, 2017, 3:50 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56499/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2017, 3:50 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jay Guo.
>
>
> Bugs: MESOS-6638
> https://issues.apache.org/jira/browse/MESOS-6638
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added roles field to framework.
>
>
> Diffs
> -----
>
> src/master/master.hpp 767ac48f2a65514a19c4a3f8ca7b35f0259f9aeb
> src/master/master.cpp 620919ecfe85367b5c1281afc5216cc20e5e2e3c
>
> Diff: https://reviews.apache.org/r/56499/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 56499: Added roles field to framework.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56499/#review165175
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Mahler
On Feb. 10, 2017, 12:24 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56499/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2017, 12:24 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jay Guo.
>
>
> Bugs: MESOS-6638
> https://issues.apache.org/jira/browse/MESOS-6638
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added roles field to framework.
>
>
> Diffs
> -----
>
> src/master/master.hpp 767ac48f2a65514a19c4a3f8ca7b35f0259f9aeb
> src/master/master.cpp 620919ecfe85367b5c1281afc5216cc20e5e2e3c
>
> Diff: https://reviews.apache.org/r/56499/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 56499: Added roles field to framework.
Posted by Guangya Liu <gy...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56499/
-----------------------------------------------------------
(Updated \u4e8c\u6708 10, 2017, 12:24 a.m.)
Review request for mesos, Benjamin Mahler and Jay Guo.
Bugs: MESOS-6638
https://issues.apache.org/jira/browse/MESOS-6638
Repository: mesos
Description
-------
Added roles field to framework.
Diffs (updated)
-----
src/master/master.hpp 767ac48f2a65514a19c4a3f8ca7b35f0259f9aeb
src/master/master.cpp 620919ecfe85367b5c1281afc5216cc20e5e2e3c
Diff: https://reviews.apache.org/r/56499/diff/
Testing
-------
make
make check
Thanks,
Guangya Liu