You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2015/10/19 23:33:24 UTC
Review Request 39457: [WIP] Document messages in messages.proto.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/
-----------------------------------------------------------
Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
Bugs: MESOS-3759
https://issues.apache.org/jira/browse/MESOS-3759
Repository: mesos
Description
-------
Due to the heavy overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto, these comments aim to document the backwards compatibility. A brief summary of each message was added, as well as a reference to the appropriate Event/Call API object.
Diffs
-----
src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
Diff: https://reviews.apache.org/r/39457/diff/
Testing
-------
None. (Comment change only)
Thanks,
Joseph Wu
Re: Review Request 39457: Document messages in messages.proto.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/#review103210
-----------------------------------------------------------
Patch looks great!
Reviews applied: [39457]
All tests passed.
- Mesos ReviewBot
On Oct. 20, 2015, 12:42 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39457/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2015, 12:42 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
>
>
> Bugs: MESOS-3759
> https://issues.apache.org/jira/browse/MESOS-3759
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A brief summary of each message was added.
>
> For messages with an associated Event/Call API object, a reference to the object was added.
> Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
>
>
> Diffs
> -----
>
> src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
>
> Diff: https://reviews.apache.org/r/39457/diff/
>
>
> Testing
> -------
>
> None. (Comment change only)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 39457: Document messages in messages.proto.
Posted by Guangya Liu <gy...@gmail.com>.
> On Oct. 20, 2015, 8:23 a.m., Guangya Liu wrote:
> >
>
> Joseph Wu wrote:
> For now, I'll keep "agent". I'm fairly certain we refer to agents as agents now.
Joseph, do you have any reasons for why not update "slave" to "agent" in the comments? When I was fixing https://reviews.apache.org/r/37993/ , I was asked to change all "slave" to "agent" to keep consistent.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/#review103218
-----------------------------------------------------------
On Oct. 20, 2015, 5:26 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39457/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2015, 5:26 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
>
>
> Bugs: MESOS-3759
> https://issues.apache.org/jira/browse/MESOS-3759
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A brief summary of each message was added.
>
> For messages with an associated Event/Call API object, a reference to the object was added.
> Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
>
>
> Diffs
> -----
>
> src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
>
> Diff: https://reviews.apache.org/r/39457/diff/
>
>
> Testing
> -------
>
> None. (Comment change only)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 39457: Document messages in messages.proto.
Posted by Guangya Liu <gy...@gmail.com>.
> On Oct. 20, 2015, 8:23 a.m., Guangya Liu wrote:
> >
>
> Joseph Wu wrote:
> For now, I'll keep "agent". I'm fairly certain we refer to agents as agents now.
>
> Guangya Liu wrote:
> Joseph, do you have any reasons for why not update "slave" to "agent" in the comments? When I was fixing https://reviews.apache.org/r/37993/ , I was asked to change all "slave" to "agent" to keep consistent.
>
> Joseph Wu wrote:
> This is mostly due to the audience of the code. People who read this file are going to be Mesos contributors. And interally, we already use "agent".
>
> The allocator comments you made are external facing (to module writers). We're still in the process (I think) of converting external docs from "slave" to "agent". For those, it's more important to stay consistent between docs than to be politically correct.
I see, thanks Joe!
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/#review103218
-----------------------------------------------------------
On Oct. 20, 2015, 5:26 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39457/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2015, 5:26 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
>
>
> Bugs: MESOS-3759
> https://issues.apache.org/jira/browse/MESOS-3759
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A brief summary of each message was added.
>
> For messages with an associated Event/Call API object, a reference to the object was added.
> Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
>
>
> Diffs
> -----
>
> src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
>
> Diff: https://reviews.apache.org/r/39457/diff/
>
>
> Testing
> -------
>
> None. (Comment change only)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 39457: Document messages in messages.proto.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Oct. 20, 2015, 1:23 a.m., Guangya Liu wrote:
> >
>
> Joseph Wu wrote:
> For now, I'll keep "agent". I'm fairly certain we refer to agents as agents now.
>
> Guangya Liu wrote:
> Joseph, do you have any reasons for why not update "slave" to "agent" in the comments? When I was fixing https://reviews.apache.org/r/37993/ , I was asked to change all "slave" to "agent" to keep consistent.
This is mostly due to the audience of the code. People who read this file are going to be Mesos contributors. And interally, we already use "agent".
The allocator comments you made are external facing (to module writers). We're still in the process (I think) of converting external docs from "slave" to "agent". For those, it's more important to stay consistent between docs than to be politically correct.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/#review103218
-----------------------------------------------------------
On Oct. 20, 2015, 10:26 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39457/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2015, 10:26 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
>
>
> Bugs: MESOS-3759
> https://issues.apache.org/jira/browse/MESOS-3759
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A brief summary of each message was added.
>
> For messages with an associated Event/Call API object, a reference to the object was added.
> Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
>
>
> Diffs
> -----
>
> src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
>
> Diff: https://reviews.apache.org/r/39457/diff/
>
>
> Testing
> -------
>
> None. (Comment change only)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 39457: Document messages in messages.proto.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Oct. 20, 2015, 1:23 a.m., Guangya Liu wrote:
> >
For now, I'll keep "agent". I'm fairly certain we refer to agents as agents now.
> On Oct. 20, 2015, 1:23 a.m., Guangya Liu wrote:
> > src/messages/messages.proto, lines 132-137
> > <https://reviews.apache.org/r/39457/diff/3/?file=1101370#file1101370line132>
> >
> > Seems it is OK to remove this as I did not see anyone who is using it.
> >
> > If this removed, then the master also needs to be updated by removing such message.
There are quite a few messages in this file that no longer *appear* to be used (mentioned in the review description). I need to check with someone (i.e. BenH) to know if it is really not being used.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/#review103218
-----------------------------------------------------------
On Oct. 19, 2015, 5:42 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39457/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2015, 5:42 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
>
>
> Bugs: MESOS-3759
> https://issues.apache.org/jira/browse/MESOS-3759
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A brief summary of each message was added.
>
> For messages with an associated Event/Call API object, a reference to the object was added.
> Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
>
>
> Diffs
> -----
>
> src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
>
> Diff: https://reviews.apache.org/r/39457/diff/
>
>
> Testing
> -------
>
> None. (Comment change only)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 39457: Document messages in messages.proto.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/#review103218
-----------------------------------------------------------
src/messages/messages.proto (line 77)
<https://reviews.apache.org/r/39457/#comment161201>
s/from the agent up to the framework/from the agent to the framework ?
src/messages/messages.proto (lines 123 - 128)
<https://reviews.apache.org/r/39457/#comment161200>
Seems it is OK to remove this as I did not see anyone who is using it.
If this removed, then the master also needs to be updated by removing such message.
src/messages/messages.proto (line 139)
<https://reviews.apache.org/r/39457/#comment161219>
s/sent/sends?
src/messages/messages.proto (line 153)
<https://reviews.apache.org/r/39457/#comment161218>
s/sent/sends?
src/messages/messages.proto (line 413)
<https://reviews.apache.org/r/39457/#comment161203>
s/slave/agent and other places?
src/messages/messages.proto (lines 499 - 500)
<https://reviews.apache.org/r/39457/#comment161211>
s/slave/agent
src/messages/messages.proto (line 508)
<https://reviews.apache.org/r/39457/#comment161210>
s/slave/agent
src/messages/messages.proto (line 527)
<https://reviews.apache.org/r/39457/#comment161209>
s/slave/agent
src/messages/messages.proto (line 546)
<https://reviews.apache.org/r/39457/#comment161208>
s/slave/agent
src/messages/messages.proto (line 603)
<https://reviews.apache.org/r/39457/#comment161206>
What about make all of the comments using same tone?
For example:
Notifies the scheduler about terminated executors.
Ditto as following
src/messages/messages.proto (line 640)
<https://reviews.apache.org/r/39457/#comment161205>
Not only the agent but also all of the frameworks on the agent will be shutdown, can we put this in the document?
src/messages/messages.proto (line 665)
<https://reviews.apache.org/r/39457/#comment161202>
s/faiure/failure
- Guangya Liu
On 十月 20, 2015, 12:42 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39457/
> -----------------------------------------------------------
>
> (Updated 十月 20, 2015, 12:42 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
>
>
> Bugs: MESOS-3759
> https://issues.apache.org/jira/browse/MESOS-3759
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A brief summary of each message was added.
>
> For messages with an associated Event/Call API object, a reference to the object was added.
> Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
>
>
> Diffs
> -----
>
> src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
>
> Diff: https://reviews.apache.org/r/39457/diff/
>
>
> Testing
> -------
>
> None. (Comment change only)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 39457: Document messages in messages.proto.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/#review103297
-----------------------------------------------------------
Patch looks great!
Reviews applied: [39457]
All tests passed.
- Mesos ReviewBot
On Oct. 20, 2015, 5:26 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39457/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2015, 5:26 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
>
>
> Bugs: MESOS-3759
> https://issues.apache.org/jira/browse/MESOS-3759
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A brief summary of each message was added.
>
> For messages with an associated Event/Call API object, a reference to the object was added.
> Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
>
>
> Diffs
> -----
>
> src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
>
> Diff: https://reviews.apache.org/r/39457/diff/
>
>
> Testing
> -------
>
> None. (Comment change only)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 39457: Document messages in messages.proto.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Oct. 20, 2015, 2:13 p.m., Joris Van Remoortere wrote:
> > This looks great Joseph!
> > Can you stay consistent with the scheduler / framework terminology?
> > Which one should we use consistently in the code-base?
(oops, forgot to click publish earlier.)
I'll use "framework" whenever the object has "framework" in its name, or if it is referring to a `FrameworkID` or `FrameworkInfo`. For other objects, especially ones that deal with the scheduler driver/library or with offers/tasks/agents/executors/statuses, I'll refer to "scheduler".
(That's my take on the fuzzy boundary between the two terms.)
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/#review103307
-----------------------------------------------------------
On Oct. 21, 2015, 11:05 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39457/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2015, 11:05 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
>
>
> Bugs: MESOS-3759
> https://issues.apache.org/jira/browse/MESOS-3759
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A brief summary of each message was added.
>
> For messages with an associated Event/Call API object, a reference to the object was added.
> Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
>
>
> Diffs
> -----
>
> src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
>
> Diff: https://reviews.apache.org/r/39457/diff/
>
>
> Testing
> -------
>
> None. (Comment change only)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 39457: Document messages in messages.proto.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/#review103307
-----------------------------------------------------------
Ship it!
This looks great Joseph!
Can you stay consistent with the scheduler / framework terminology?
Which one should we use consistently in the code-base?
- Joris Van Remoortere
On Oct. 20, 2015, 5:26 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39457/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2015, 5:26 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
>
>
> Bugs: MESOS-3759
> https://issues.apache.org/jira/browse/MESOS-3759
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A brief summary of each message was added.
>
> For messages with an associated Event/Call API object, a reference to the object was added.
> Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
>
>
> Diffs
> -----
>
> src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
>
> Diff: https://reviews.apache.org/r/39457/diff/
>
>
> Testing
> -------
>
> None. (Comment change only)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 39457: Document messages in messages.proto.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/
-----------------------------------------------------------
(Updated Oct. 21, 2015, 11:05 a.m.)
Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
Changes
-------
Renamed a few instances of "scheduler" to "framework", and "slave" to "agent".
Bugs: MESOS-3759
https://issues.apache.org/jira/browse/MESOS-3759
Repository: mesos
Description
-------
A brief summary of each message was added.
For messages with an associated Event/Call API object, a reference to the object was added.
Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
Diffs (updated)
-----
src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
Diff: https://reviews.apache.org/r/39457/diff/
Testing
-------
None. (Comment change only)
Thanks,
Joseph Wu
Re: Review Request 39457: Document messages in messages.proto.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/
-----------------------------------------------------------
(Updated Oct. 20, 2015, 10:26 a.m.)
Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
Changes
-------
Standardized to present tense. Addressed comments, except for s/agent/slave/.
Bugs: MESOS-3759
https://issues.apache.org/jira/browse/MESOS-3759
Repository: mesos
Description
-------
A brief summary of each message was added.
For messages with an associated Event/Call API object, a reference to the object was added.
Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
Diffs (updated)
-----
src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
Diff: https://reviews.apache.org/r/39457/diff/
Testing
-------
None. (Comment change only)
Thanks,
Joseph Wu
Re: Review Request 39457: Document messages in messages.proto.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/
-----------------------------------------------------------
(Updated Oct. 19, 2015, 5:42 p.m.)
Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
Changes
-------
Finish first pass.
Summary (updated)
-----------------
Document messages in messages.proto.
Bugs: MESOS-3759
https://issues.apache.org/jira/browse/MESOS-3759
Repository: mesos
Description (updated)
-------
A brief summary of each message was added.
For messages with an associated Event/Call API object, a reference to the object was added.
Additionally, there is a great deal of documentation overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto. Where necessary, some notes were added about the backwards compatibility of messages which are not instantiated in the code base.
Diffs (updated)
-----
src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
Diff: https://reviews.apache.org/r/39457/diff/
Testing
-------
None. (Comment change only)
Thanks,
Joseph Wu
Re: Review Request 39457: [WIP] Document messages in messages.proto.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39457/
-----------------------------------------------------------
(Updated Oct. 19, 2015, 4:07 p.m.)
Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
Changes
-------
Added a bunch more comments.
Bugs: MESOS-3759
https://issues.apache.org/jira/browse/MESOS-3759
Repository: mesos
Description
-------
Due to the heavy overlap between these messages and the comments in mesos/scheduler.hpp and v1/scheduler/scheduler.proto, these comments aim to document the backwards compatibility. A brief summary of each message was added, as well as a reference to the appropriate Event/Call API object.
Diffs (updated)
-----
src/messages/messages.proto ea9a67e169a8a359a12be93b804783c7dcced0b7
Diff: https://reviews.apache.org/r/39457/diff/
Testing
-------
None. (Comment change only)
Thanks,
Joseph Wu