You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2014/10/23 21:14:31 UTC
Review Request 27102: Moved framework id validation from scheduler
driver to master.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27102/
-----------------------------------------------------------
Review request for mesos, Ben Mahler and Dominic Hamon.
Bugs: MESOS-1972
https://issues.apache.org/jira/browse/MESOS-1972
Repository: mesos-git
Description
-------
Moved framework id validation from scheduler driver to master.
Note that the check regarding incorrect use of both command info and executor info is already present in the master. Killed it from the driver.
Diffs
-----
src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec
src/sched/sched.cpp e89e5e56b72fec8832e99b90692d2cbd619fbcfb
src/tests/resource_offers_tests.cpp 060039eed0bb8246ac034c0fb8560bfc7b25ab98
Diff: https://reviews.apache.org/r/27102/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 27102: Moved framework id validation from scheduler
driver to master.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27102/#review58320
-----------------------------------------------------------
Ship it!
src/master/master.cpp
<https://reviews.apache.org/r/27102/#comment99275>
Can you update the existing ExecutorInfo checker instead? We already check presence there, but not equality.
- Ben Mahler
On Oct. 23, 2014, 11:32 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27102/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 11:32 p.m.)
>
>
> Review request for mesos, Ben Mahler and Dominic Hamon.
>
>
> Bugs: MESOS-1972
> https://issues.apache.org/jira/browse/MESOS-1972
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Moved framework id validation from scheduler driver to master.
>
> Note that the check regarding incorrect use of both command info and executor info is already present in the master. Killed it from the driver.
>
>
> Diffs
> -----
>
> src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec
> src/sched/sched.cpp e89e5e56b72fec8832e99b90692d2cbd619fbcfb
> src/tests/resource_offers_tests.cpp 060039eed0bb8246ac034c0fb8560bfc7b25ab98
>
> Diff: https://reviews.apache.org/r/27102/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 27102: Moved framework id validation from scheduler
driver to master.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27102/#review58249
-----------------------------------------------------------
Patch looks great!
Reviews applied: [27102]
All tests passed.
- Mesos ReviewBot
On Oct. 23, 2014, 11:32 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27102/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 11:32 p.m.)
>
>
> Review request for mesos, Ben Mahler and Dominic Hamon.
>
>
> Bugs: MESOS-1972
> https://issues.apache.org/jira/browse/MESOS-1972
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Moved framework id validation from scheduler driver to master.
>
> Note that the check regarding incorrect use of both command info and executor info is already present in the master. Killed it from the driver.
>
>
> Diffs
> -----
>
> src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec
> src/sched/sched.cpp e89e5e56b72fec8832e99b90692d2cbd619fbcfb
> src/tests/resource_offers_tests.cpp 060039eed0bb8246ac034c0fb8560bfc7b25ab98
>
> Diff: https://reviews.apache.org/r/27102/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 27102: Moved framework id validation from scheduler
driver to master.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27102/
-----------------------------------------------------------
(Updated Oct. 23, 2014, 11:32 p.m.)
Review request for mesos, Ben Mahler and Dominic Hamon.
Changes
-------
comments.
Bugs: MESOS-1972
https://issues.apache.org/jira/browse/MESOS-1972
Repository: mesos-git
Description
-------
Moved framework id validation from scheduler driver to master.
Note that the check regarding incorrect use of both command info and executor info is already present in the master. Killed it from the driver.
Diffs (updated)
-----
src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec
src/sched/sched.cpp e89e5e56b72fec8832e99b90692d2cbd619fbcfb
src/tests/resource_offers_tests.cpp 060039eed0bb8246ac034c0fb8560bfc7b25ab98
Diff: https://reviews.apache.org/r/27102/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 27102: Moved framework id validation from scheduler
driver to master.
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 23, 2014, 9:57 p.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 1906
> > <https://reviews.apache.org/r/27102/diff/2/?file=731201#file731201line1906>
> >
> > is it ok for an executor to not have a framework id attached?
yea, it's an optional field.
> On Oct. 23, 2014, 9:57 p.m., Dominic Hamon wrote:
> > src/tests/resource_offers_tests.cpp, line 116
> > <https://reviews.apache.org/r/27102/diff/2/?file=731203#file731203line116>
> >
> > nit: it's not random, it's deterministic. Maybe "unknown" instead to indicate that you're testing that it's an unknown framework?
ok. used uuid to make it random :)
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27102/#review58122
-----------------------------------------------------------
On Oct. 23, 2014, 11:32 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27102/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 11:32 p.m.)
>
>
> Review request for mesos, Ben Mahler and Dominic Hamon.
>
>
> Bugs: MESOS-1972
> https://issues.apache.org/jira/browse/MESOS-1972
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Moved framework id validation from scheduler driver to master.
>
> Note that the check regarding incorrect use of both command info and executor info is already present in the master. Killed it from the driver.
>
>
> Diffs
> -----
>
> src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec
> src/sched/sched.cpp e89e5e56b72fec8832e99b90692d2cbd619fbcfb
> src/tests/resource_offers_tests.cpp 060039eed0bb8246ac034c0fb8560bfc7b25ab98
>
> Diff: https://reviews.apache.org/r/27102/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 27102: Moved framework id validation from scheduler
driver to master.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27102/#review58122
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/27102/#comment99026>
is it ok for an executor to not have a framework id attached?
src/tests/resource_offers_tests.cpp
<https://reviews.apache.org/r/27102/#comment99027>
nit: it's not random, it's deterministic. Maybe "unknown" instead to indicate that you're testing that it's an unknown framework?
- Dominic Hamon
On Oct. 23, 2014, 1:57 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27102/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 1:57 p.m.)
>
>
> Review request for mesos, Ben Mahler and Dominic Hamon.
>
>
> Bugs: MESOS-1972
> https://issues.apache.org/jira/browse/MESOS-1972
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Moved framework id validation from scheduler driver to master.
>
> Note that the check regarding incorrect use of both command info and executor info is already present in the master. Killed it from the driver.
>
>
> Diffs
> -----
>
> src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec
> src/sched/sched.cpp e89e5e56b72fec8832e99b90692d2cbd619fbcfb
> src/tests/resource_offers_tests.cpp 060039eed0bb8246ac034c0fb8560bfc7b25ab98
>
> Diff: https://reviews.apache.org/r/27102/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 27102: Moved framework id validation from scheduler
driver to master.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27102/#review58101
-----------------------------------------------------------
Patch looks great!
Reviews applied: [27102]
All tests passed.
- Mesos ReviewBot
On Oct. 23, 2014, 8:57 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27102/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 8:57 p.m.)
>
>
> Review request for mesos, Ben Mahler and Dominic Hamon.
>
>
> Bugs: MESOS-1972
> https://issues.apache.org/jira/browse/MESOS-1972
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Moved framework id validation from scheduler driver to master.
>
> Note that the check regarding incorrect use of both command info and executor info is already present in the master. Killed it from the driver.
>
>
> Diffs
> -----
>
> src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec
> src/sched/sched.cpp e89e5e56b72fec8832e99b90692d2cbd619fbcfb
> src/tests/resource_offers_tests.cpp 060039eed0bb8246ac034c0fb8560bfc7b25ab98
>
> Diff: https://reviews.apache.org/r/27102/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 27102: Moved framework id validation from scheduler
driver to master.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27102/
-----------------------------------------------------------
(Updated Oct. 23, 2014, 8:57 p.m.)
Review request for mesos, Ben Mahler and Dominic Hamon.
Changes
-------
dominic's.
Bugs: MESOS-1972
https://issues.apache.org/jira/browse/MESOS-1972
Repository: mesos-git
Description
-------
Moved framework id validation from scheduler driver to master.
Note that the check regarding incorrect use of both command info and executor info is already present in the master. Killed it from the driver.
Diffs (updated)
-----
src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec
src/sched/sched.cpp e89e5e56b72fec8832e99b90692d2cbd619fbcfb
src/tests/resource_offers_tests.cpp 060039eed0bb8246ac034c0fb8560bfc7b25ab98
Diff: https://reviews.apache.org/r/27102/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 27102: Moved framework id validation from scheduler
driver to master.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27102/#review58060
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/27102/#comment98958>
it's a shame this can't be blocked by https://reviews.apache.org/r/20423/ :)
src/sched/sched.cpp
<https://reviews.apache.org/r/27102/#comment98957>
can you change this to:
foreach (TaskInfo task, tasks) {
and avoid the explicit copy internally?
- Dominic Hamon
On Oct. 23, 2014, 12:14 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27102/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 12:14 p.m.)
>
>
> Review request for mesos, Ben Mahler and Dominic Hamon.
>
>
> Bugs: MESOS-1972
> https://issues.apache.org/jira/browse/MESOS-1972
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Moved framework id validation from scheduler driver to master.
>
> Note that the check regarding incorrect use of both command info and executor info is already present in the master. Killed it from the driver.
>
>
> Diffs
> -----
>
> src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec
> src/sched/sched.cpp e89e5e56b72fec8832e99b90692d2cbd619fbcfb
> src/tests/resource_offers_tests.cpp 060039eed0bb8246ac034c0fb8560bfc7b25ab98
>
> Diff: https://reviews.apache.org/r/27102/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>