You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joerg Schad <jo...@mesosphere.io> on 2016/02/22 04:00:03 UTC
Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/
-----------------------------------------------------------
Review request for mesos, Adam B and Neil Conway.
Repository: mesos
Description
-------
Added Scheduler-Driver API to app-framework-development-guide.md.
Diffs
-----
docs/app-framework-development-guide.md e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d
Diff: https://reviews.apache.org/r/43819/diff/
Testing
-------
Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
Thanks,
Joerg Schad
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Adam B <ad...@mesosphere.io>.
> On Feb. 21, 2016, 7:20 p.m., Guangya Liu wrote:
> > docs/app-framework-development-guide.md, line 135
> > <https://reviews.apache.org/r/43819/diff/1/?file=1263766#file1263766line135>
> >
> > Why not merge this and above `Scheduler API` into one section?
>
> Joerg Schad wrote:
> I wanted to keep the Scheduler API i.e. the callbacks the scheduler has to implement and the calls it can make (i.e. SchedulerDriver API) seperate. Does that make sense?
>
> Guangya Liu wrote:
> What about name this part as Scheduler Driver API?
+100 to "Scheduler Driver API" rather than two "Scheduler API" headings
// Abstract interface for connecting a scheduler to Mesos. This
// interface is used both to manage the scheduler's lifecycle (start
// it, stop it, or wait for it to finish) and to interact with Mesos
// (e.g., launch tasks, kill tasks, etc.).
class SchedulerDriver
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/#review120091
-----------------------------------------------------------
On Feb. 21, 2016, 7 p.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2016, 7 p.m.)
>
>
> Review request for mesos, Adam B and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Scheduler-Driver API to app-framework-development-guide.md.
>
>
> Diffs
> -----
>
> docs/app-framework-development-guide.md e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d
>
> Diff: https://reviews.apache.org/r/43819/diff/
>
>
> Testing
> -------
>
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Joerg Schad <jo...@mesosphere.io>.
> On Feb. 22, 2016, 3:20 a.m., Guangya Liu wrote:
> > docs/app-framework-development-guide.md, line 135
> > <https://reviews.apache.org/r/43819/diff/1/?file=1263766#file1263766line135>
> >
> > Why not merge this and above `Scheduler API` into one section?
I wanted to keep the Scheduler API i.e. the callbacks the scheduler has to implement and the calls it can make (i.e. SchedulerDriver API) seperate. Does that make sense?
> On Feb. 22, 2016, 3:20 a.m., Guangya Liu wrote:
> > docs/app-framework-development-guide.md, lines 174-178
> > <https://reviews.apache.org/r/43819/diff/1/?file=1263766#file1263766line174>
> >
> > Do we need to mention this here? The API was not implemented by the built-in allocator, so the framework cannot leverage this if using built-in allocator.
It is part of the interface so I believe yes, but I added a note https://reviews.apache.org/r/43838/ stating that the default allocator currently does not implement this call.
> On Feb. 22, 2016, 3:20 a.m., Guangya Liu wrote:
> > docs/app-framework-development-guide.md, line 17
> > <https://reviews.apache.org/r/43819/diff/1/?file=1263766#file1263766line17>
> >
> > s/interface/interfaces
a) this is equivalent to the description in scheduler.hpp.
b) For me this is a single interface (specifying several callbacks), what makes you feel this should be plural?
> On Feb. 22, 2016, 3:20 a.m., Guangya Liu wrote:
> > docs/app-framework-development-guide.md, line 137
> > <https://reviews.apache.org/r/43819/diff/1/?file=1263766#file1263766line137>
> >
> > s/Interface/Interfaces
see above.
- Joerg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/#review120091
-----------------------------------------------------------
On Feb. 22, 2016, 3 a.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 3 a.m.)
>
>
> Review request for mesos, Adam B and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Scheduler-Driver API to app-framework-development-guide.md.
>
>
> Diffs
> -----
>
> docs/app-framework-development-guide.md e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d
>
> Diff: https://reviews.apache.org/r/43819/diff/
>
>
> Testing
> -------
>
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Guangya Liu <gy...@gmail.com>.
> On 二月 22, 2016, 3:20 a.m., Guangya Liu wrote:
> > docs/app-framework-development-guide.md, line 135
> > <https://reviews.apache.org/r/43819/diff/1/?file=1263766#file1263766line135>
> >
> > Why not merge this and above `Scheduler API` into one section?
>
> Joerg Schad wrote:
> I wanted to keep the Scheduler API i.e. the callbacks the scheduler has to implement and the calls it can make (i.e. SchedulerDriver API) seperate. Does that make sense?
What about name this part as Scheduler Driver API?
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/#review120091
-----------------------------------------------------------
On 二月 22, 2016, 3 a.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -----------------------------------------------------------
>
> (Updated 二月 22, 2016, 3 a.m.)
>
>
> Review request for mesos, Adam B and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Scheduler-Driver API to app-framework-development-guide.md.
>
>
> Diffs
> -----
>
> docs/app-framework-development-guide.md e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d
>
> Diff: https://reviews.apache.org/r/43819/diff/
>
>
> Testing
> -------
>
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/#review120091
-----------------------------------------------------------
docs/app-framework-development-guide.md (line 17)
<https://reviews.apache.org/r/43819/#comment181463>
s/interface/interfaces
docs/app-framework-development-guide.md (line 129)
<https://reviews.apache.org/r/43819/#comment181467>
Why not merge this and above `Scheduler API` into one section?
docs/app-framework-development-guide.md (line 131)
<https://reviews.apache.org/r/43819/#comment181469>
s/Interface/Interfaces
docs/app-framework-development-guide.md (lines 168 - 172)
<https://reviews.apache.org/r/43819/#comment181466>
Do we need to mention this here? The API was not implemented by the built-in allocator, so the framework cannot leverage this if using built-in allocator.
- Guangya Liu
On 二月 22, 2016, 3 a.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -----------------------------------------------------------
>
> (Updated 二月 22, 2016, 3 a.m.)
>
>
> Review request for mesos, Adam B and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Scheduler-Driver API to app-framework-development-guide.md.
>
>
> Diffs
> -----
>
> docs/app-framework-development-guide.md e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d
>
> Diff: https://reviews.apache.org/r/43819/diff/
>
>
> Testing
> -------
>
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Joerg Schad <jo...@mesosphere.io>.
> On Feb. 24, 2016, 7:57 a.m., Adam B wrote:
> > docs/app-framework-development-guide.md, lines 137-139
> > <https://reviews.apache.org/r/43819/diff/1/?file=1263766#file1263766line137>
> >
> > Let's be very explicit that they'll need to instantiate a MesosSchedulerDriver and call these methods on it. Probably also want to document the MesosSchedulerDriver constructors so they know how to instantiate one.
Refered to the test_framework.cpp instead for proper use of the the schedulerDriver...
- Joerg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/#review120465
-----------------------------------------------------------
On March 7, 2016, 8:05 p.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 8:05 p.m.)
>
>
> Review request for mesos, Adam B and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously the app-framework-development-guide only explained the scheduler callback interface. Equally important when developing frameworks is the knowledge if potential actions a scheduler can trigger via the SchedulerDriver.
>
>
> Diffs
> -----
>
> docs/app-framework-development-guide.md 55f09c7cf2262dda98dcc8b63b1e36ab78296141
>
> Diff: https://reviews.apache.org/r/43819/diff/
>
>
> Testing
> -------
>
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Joerg Schad <jo...@mesosphere.io>.
> On Feb. 24, 2016, 7:57 a.m., Adam B wrote:
> > docs/app-framework-development-guide.md, line 144
> > <https://reviews.apache.org/r/43819/diff/1/?file=1263766#file1263766line144>
> >
> > Get rid of the `virtual` and `= 0` since that's not a part of the API that their scheduler will call. They're not implementing a new SchedulerDriver, they're just instantiating one and calling its methods.
I actually just wanted it to be consistent with the below part. Will remove `= 0` from both parts
- Joerg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/#review120465
-----------------------------------------------------------
On Feb. 22, 2016, 3 a.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 3 a.m.)
>
>
> Review request for mesos, Adam B and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Scheduler-Driver API to app-framework-development-guide.md.
>
>
> Diffs
> -----
>
> docs/app-framework-development-guide.md e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d
>
> Diff: https://reviews.apache.org/r/43819/diff/
>
>
> Testing
> -------
>
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/#review120465
-----------------------------------------------------------
Great work. Still missing a bit of context about what a SchedulerDriver is though
docs/app-framework-development-guide.md (lines 131 - 133)
<https://reviews.apache.org/r/43819/#comment181927>
Let's be very explicit that they'll need to instantiate a MesosSchedulerDriver and call these methods on it. Probably also want to document the MesosSchedulerDriver constructors so they know how to instantiate one.
docs/app-framework-development-guide.md (line 138)
<https://reviews.apache.org/r/43819/#comment181924>
Get rid of the `virtual` and `= 0` since that's not a part of the API that their scheduler will call. They're not implementing a new SchedulerDriver, they're just instantiating one and calling its methods.
docs/app-framework-development-guide.md (lines 187 - 191)
<https://reviews.apache.org/r/43819/#comment181925>
Remove this from the guide since it's deprecated.
- Adam B
On Feb. 21, 2016, 7 p.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2016, 7 p.m.)
>
>
> Review request for mesos, Adam B and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Scheduler-Driver API to app-framework-development-guide.md.
>
>
> Diffs
> -----
>
> docs/app-framework-development-guide.md e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d
>
> Diff: https://reviews.apache.org/r/43819/diff/
>
>
> Testing
> -------
>
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/#review120130
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43819]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 22, 2016, 3 a.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 3 a.m.)
>
>
> Review request for mesos, Adam B and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Scheduler-Driver API to app-framework-development-guide.md.
>
>
> Diffs
> -----
>
> docs/app-framework-development-guide.md e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d
>
> Diff: https://reviews.apache.org/r/43819/diff/
>
>
> Testing
> -------
>
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/#review123309
-----------------------------------------------------------
Fix it, then Ship it!
Looks great. Fix it, then I'll commit.
docs/app-framework-development-guide.md (line 136)
<https://reviews.apache.org/r/43819/#comment185526>
s/necessarily/usually/ It's only implemented by a framework if it's not using libmesos, right?
docs/app-framework-development-guide.md (lines 324 - 325)
<https://reviews.apache.org/r/43819/#comment185527>
Looks like this fits on one line now. I wonder if others will too.
- Adam B
On March 7, 2016, 12:14 p.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 12:14 p.m.)
>
>
> Review request for mesos, Adam B and Neil Conway.
>
>
> Bugs: MESOS-4726
> https://issues.apache.org/jira/browse/MESOS-4726
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously the app-framework-development-guide only explained the scheduler callback interface. Equally important when developing frameworks is the knowledge if potential actions a scheduler can trigger via the SchedulerDriver.
>
>
> Diffs
> -----
>
> docs/app-framework-development-guide.md 55f09c7cf2262dda98dcc8b63b1e36ab78296141
>
> Diff: https://reviews.apache.org/r/43819/diff/
>
>
> Testing
> -------
>
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
>
>
> Thanks,
>
> Joerg Schad
>
>
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/
-----------------------------------------------------------
(Updated March 13, 2016, 1:29 p.m.)
Review request for mesos, Adam B and Neil Conway.
Changes
-------
Addressed comments.
Bugs: MESOS-4726
https://issues.apache.org/jira/browse/MESOS-4726
Repository: mesos
Description
-------
Previously the app-framework-development-guide only explained the scheduler callback interface. Equally important when developing frameworks is the knowledge if potential actions a scheduler can trigger via the SchedulerDriver.
Diffs (updated)
-----
docs/app-framework-development-guide.md 55f09c7cf2262dda98dcc8b63b1e36ab78296141
Diff: https://reviews.apache.org/r/43819/diff/
Testing
-------
Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
Thanks,
Joerg Schad
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/
-----------------------------------------------------------
(Updated March 7, 2016, 8:14 p.m.)
Review request for mesos, Adam B and Neil Conway.
Bugs: MESOS-4726
https://issues.apache.org/jira/browse/MESOS-4726
Repository: mesos
Description
-------
Previously the app-framework-development-guide only explained the scheduler callback interface. Equally important when developing frameworks is the knowledge if potential actions a scheduler can trigger via the SchedulerDriver.
Diffs
-----
docs/app-framework-development-guide.md 55f09c7cf2262dda98dcc8b63b1e36ab78296141
Diff: https://reviews.apache.org/r/43819/diff/
Testing
-------
Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
Thanks,
Joerg Schad
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/
-----------------------------------------------------------
(Updated March 7, 2016, 8:13 p.m.)
Review request for mesos, Adam B and Neil Conway.
Changes
-------
Improved description of SchedulerDriver.
Repository: mesos
Description
-------
Previously the app-framework-development-guide only explained the scheduler callback interface. Equally important when developing frameworks is the knowledge if potential actions a scheduler can trigger via the SchedulerDriver.
Diffs (updated)
-----
docs/app-framework-development-guide.md 55f09c7cf2262dda98dcc8b63b1e36ab78296141
Diff: https://reviews.apache.org/r/43819/diff/
Testing
-------
Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
Thanks,
Joerg Schad
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/
-----------------------------------------------------------
(Updated March 7, 2016, 8:05 p.m.)
Review request for mesos, Adam B and Neil Conway.
Changes
-------
addressed reviews.
Repository: mesos
Description (updated)
-------
Previously the app-framework-development-guide only explained the scheduler callback interface. Equally important when developing frameworks is the knowledge if potential actions a scheduler can trigger via the SchedulerDriver.
Diffs (updated)
-----
docs/app-framework-development-guide.md 55f09c7cf2262dda98dcc8b63b1e36ab78296141
Diff: https://reviews.apache.org/r/43819/diff/
Testing
-------
Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
Thanks,
Joerg Schad
Re: Review Request 43819: Added Scheduler-Driver API to
app-framework-development-guide.md.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43819/#review120356
-----------------------------------------------------------
Fix it, then Ship it!
docs/app-framework-development-guide.md (line 17)
<https://reviews.apache.org/r/43819/#comment181787>
I'd say "framework schedulers" for simplicity.
- Neil Conway
On Feb. 22, 2016, 3 a.m., Joerg Schad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 3 a.m.)
>
>
> Review request for mesos, Adam B and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Scheduler-Driver API to app-framework-development-guide.md.
>
>
> Diffs
> -----
>
> docs/app-framework-development-guide.md e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d
>
> Diff: https://reviews.apache.org/r/43819/diff/
>
>
> Testing
> -------
>
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via docker website container.
>
>
> Thanks,
>
> Joerg Schad
>
>