Re: Review Request 61495: Add documentation for possible task reasons.

2017-09-07 Thread Alexander Rukletsov 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61495/#review184810
---


Fix it, then Ship it!




I'll fix the last issue and commit this for you.


docs/task-state-reasons.md
Lines 20 (patched)


Change "which" back to "that"


- Alexander Rukletsov


On Sept. 6, 2017, 9:29 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61495/
> ---
> 
> (Updated Sept. 6, 2017, 9:29 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, James Peach, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5078
> https://issues.apache.org/jira/browse/MESOS-5078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for possible task reasons.
> 
> 
> Diffs
> -
> 
>   docs/home.md ad91f2fe23b39d59dc021428899069531fce9970 
>   docs/task-state-reasons.md PRE-CREATION 
>   include/mesos/mesos.proto eede0827fc0da5f2fa3cfb432fde29b95a8c644c 
>   include/mesos/v1/mesos.proto 8c6246ee824f28d93dc5b57d25939d1ba76c986b 
> 
> 
> Diff: https://reviews.apache.org/r/61495/diff/8/
> 
> 
> Testing
> ---
> 
> Built website with site/build.sh and verified it renders ok.
> 
> HTML preview: 
> http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-state-reasons/index.html
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 61495: Add documentation for possible task reasons.

2017-09-06 Thread Benno Evers 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61495/
---

(Updated Sept. 6, 2017, 9:29 a.m.)


Review request for mesos, Alexander Rukletsov, James Peach, and Till Toenshoff.


Changes
---

Grammar changes based on feedback by Nicholas Parker.


Bugs: MESOS-5078
https://issues.apache.org/jira/browse/MESOS-5078


Repository: mesos


Description
---

Add documentation for possible task reasons.


Diffs (updated)
-

  docs/home.md ad91f2fe23b39d59dc021428899069531fce9970 
  docs/task-state-reasons.md PRE-CREATION 
  include/mesos/mesos.proto eede0827fc0da5f2fa3cfb432fde29b95a8c644c 
  include/mesos/v1/mesos.proto 8c6246ee824f28d93dc5b57d25939d1ba76c986b 


Diff: https://reviews.apache.org/r/61495/diff/8/

Changes: https://reviews.apache.org/r/61495/diff/7-8/


Testing
---

Built website with site/build.sh and verified it renders ok.

HTML preview: 
http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-state-reasons/index.html


Thanks,

Benno Evers

Re: Review Request 61495: Add documentation for possible task reasons.

2017-09-04 Thread Benno Evers 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61495/
---

(Updated Sept. 4, 2017, 10:47 a.m.)


Review request for mesos, Alexander Rukletsov, James Peach, and Till Toenshoff.


Bugs: MESOS-5078
https://issues.apache.org/jira/browse/MESOS-5078


Repository: mesos


Description
---

Add documentation for possible task reasons.


Diffs (updated)
-

  docs/home.md ad91f2fe23b39d59dc021428899069531fce9970 
  docs/task-state-reasons.md PRE-CREATION 
  include/mesos/mesos.proto eede0827fc0da5f2fa3cfb432fde29b95a8c644c 
  include/mesos/v1/mesos.proto 8c6246ee824f28d93dc5b57d25939d1ba76c986b 


Diff: https://reviews.apache.org/r/61495/diff/7/

Changes: https://reviews.apache.org/r/61495/diff/6-7/


Testing
---

Built website with site/build.sh and verified it renders ok.

HTML preview: 
http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-state-reasons/index.html


Thanks,

Benno Evers

Re: Review Request 61495: Add documentation for possible task reasons.

2017-09-04 Thread Benno Evers 


> On Aug. 31, 2017, 2:08 p.m., Alexander Rukletsov wrote:
> > docs/task-state-reasons.md
> > Lines 162-163 (patched)
> > 
> >
> > You use 3rd and 4th caption level inconsistently: sometimes you repeat 
> > caption for the state, and sometimes you do not.
> > 
> > I would suggest to get rid of the 4th level altogether and have 3rd 
> > level captions in form "For `TASK_KILLED` updates with `SOURCE_SLAVE`"

I removed the additional 3rd-level captions instead, because they look very 
noisy in the generated HTML


> On Aug. 31, 2017, 2:08 p.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto
> > Lines 2149-2150 (original)
> > 
> >
> > I thought you remove the comment in the next patch?

I thought so too. I'll try to revert this again, if reviewboard will let me.


- Benno


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61495/#review184244
---


On Sept. 4, 2017, 10:47 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61495/
> ---
> 
> (Updated Sept. 4, 2017, 10:47 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, James Peach, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5078
> https://issues.apache.org/jira/browse/MESOS-5078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for possible task reasons.
> 
> 
> Diffs
> -
> 
>   docs/home.md ad91f2fe23b39d59dc021428899069531fce9970 
>   docs/task-state-reasons.md PRE-CREATION 
>   include/mesos/mesos.proto eede0827fc0da5f2fa3cfb432fde29b95a8c644c 
>   include/mesos/v1/mesos.proto 8c6246ee824f28d93dc5b57d25939d1ba76c986b 
> 
> 
> Diff: https://reviews.apache.org/r/61495/diff/7/
> 
> 
> Testing
> ---
> 
> Built website with site/build.sh and verified it renders ok.
> 
> HTML preview: 
> http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-state-reasons/index.html
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 61495: Add documentation for possible task reasons.

2017-08-31 Thread Alexander Rukletsov 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61495/#review184244
---


Fix it, then Ship it!




Looks very good to me. Please have a native speaker look through the write-up 
before we ship it: I'm not sure whether appropriate tenses are used in some 
descriptions. Also please wrap at 80 chars where possible.


docs/task-state-reasons.md
Lines 18 (patched)


I'd rather say "authors" or "writers".



docs/task-state-reasons.md
Lines 20 (patched)


s/which/that



docs/task-state-reasons.md
Lines 25 (patched)


remove one "the"



docs/task-state-reasons.md
Lines 47-52 (patched)


Please add a note saying that default executors currently additionally send 
all unhealthy updates.



docs/task-state-reasons.md
Lines 80-82 (patched)


"As of Mesos 1.4 ..." instead of "currently"?



docs/task-state-reasons.md
Lines 95 (patched)


Wrap `FrameworkInfo` in backticks?



docs/task-state-reasons.md
Lines 98 (patched)


... / `SOURCE_AGENT`



docs/task-state-reasons.md
Lines 119 (patched)


s/memory/disk



docs/task-state-reasons.md
Lines 134 (patched)


s/re-register/reregister?



docs/task-state-reasons.md
Lines 162-163 (patched)


You use 3rd and 4th caption level inconsistently: sometimes you repeat 
caption for the state, and sometimes you do not.

I would suggest to get rid of the 4th level altogether and have 3rd level 
captions in form "For `TASK_KILLED` updates with `SOURCE_SLAVE`"



docs/task-state-reasons.md
Lines 202 (patched)


s/slave/agent



docs/task-state-reasons.md
Lines 207 (patched)


s/slave/agent



docs/task-state-reasons.md
Lines 212 (patched)


s/slave/agent



docs/task-state-reasons.md
Lines 224 (patched)


s/slave/agent



docs/task-state-reasons.md
Lines 226 (patched)


s/slave/agent



docs/task-state-reasons.md
Lines 230 (patched)


Either remove "to" or add "change". Here and below.



docs/task-state-reasons.md
Lines 234 (patched)


Is this state for v0 API only? If so, please make a note about it.



docs/task-state-reasons.md
Lines 238 (patched)


Ditto



docs/task-state-reasons.md
Lines 248 (patched)


s/slave/agent



docs/task-state-reasons.md
Lines 250 (patched)


s/slave/agent



docs/task-state-reasons.md
Lines 254 (patched)


Ditto



docs/task-state-reasons.md
Lines 256 (patched)


s/slave/agent



docs/task-state-reasons.md
Lines 259 (patched)


Ditto



docs/task-state-reasons.md
Lines 267 (patched)


remove "to"?



docs/task-state-reasons.md
Lines 273 (patched)


s/slave/agent



docs/task-state-reasons.md
Lines 282 (patched)


Ditto



docs/task-state-reasons.md
Lines 291 (patched)


Ditto



docs/task-state-reasons.md
Lines 305 (patched)


s/slave/agent



docs/task-state-reasons.md
Lines 309 (patched)


Ditto



docs/task-state-reasons.md
Lines 318 (patched)


Ditto



docs/task-state-reasons.md
Lines 326 (patched)


Ditto



docs/task-state-reasons.md
Lines 413 (patched)


"originally sent", probably? Instead, you can say "the original ones"



include/mesos/mesos.proto
Lines 2149-2150 (original)


I thought you re

Re: Review Request 61495: Add documentation for possible task reasons.

2017-08-17 Thread Alexander Rukletsov 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61495/#review183161
---




docs/task-state-reasons.md
Lines 55-58 (patched)


Duplicate of the above?



docs/task-state-reasons.md
Lines 158-163 (patched)


Maybe merge it with the previous one? Otherwise people might not notice 
that there is another reason documented in a separate section.



docs/task-state-reasons.md
Lines 178-197 (patched)


Ditto as above.



docs/task-state-reasons.md
Lines 358-369 (patched)


I think a custom executor can set these reasons even for terminal updates. 
However, this is true for all executor-induced updates.

I don't think we ever will set these reasons for `TASK_STAGING`, because 
this is sent before the task is launched by an agent and never by an executor.

Let's mention that currently (i.e. Mesos 1.4.0) all built-in executors set 
these reasons only for `TASK_RUNNING`.



docs/task-state-reasons.md
Lines 360 (patched)


Not sure this is true.



docs/task-state-reasons.md
Lines 367 (patched)


Ditto.



include/mesos/mesos.proto
Lines 2148 (patched)


task-state-reasons.md?


- Alexander Rukletsov


On Aug. 10, 2017, 2:01 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61495/
> ---
> 
> (Updated Aug. 10, 2017, 2:01 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, James Peach, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5078
> https://issues.apache.org/jira/browse/MESOS-5078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for possible task reasons.
> 
> 
> Diffs
> -
> 
>   docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
>   docs/task-state-reasons.md PRE-CREATION 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
> 
> 
> Diff: https://reviews.apache.org/r/61495/diff/4/
> 
> 
> Testing
> ---
> 
> Built website with site/build.sh and verified it renders ok.
> 
> HTML preview: 
> http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-state-reasons/index.html
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 61495: Add documentation for possible task reasons.

2017-08-10 Thread Benno Evers 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61495/
---

(Updated Aug. 10, 2017, 2:01 p.m.)


Review request for mesos, Alexander Rukletsov, James Peach, and Till Toenshoff.


Changes
---

Refactor moving of comment into a separate review.


Bugs: MESOS-5078
https://issues.apache.org/jira/browse/MESOS-5078


Repository: mesos


Description
---

Add documentation for possible task reasons.


Diffs (updated)
-

  docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
  docs/task-state-reasons.md PRE-CREATION 
  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 


Diff: https://reviews.apache.org/r/61495/diff/4/

Changes: https://reviews.apache.org/r/61495/diff/3-4/


Testing
---

Built website with site/build.sh and verified it renders ok.

HTML preview: 
http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-state-reasons/index.html


Thanks,

Benno Evers

Re: Review Request 61495: Add documentation for possible task reasons.

2017-08-10 Thread Benno Evers 


> On Aug. 10, 2017, 1:56 a.m., Till Toenshoff wrote:
> > docs/task-reasons.md
> > Lines 49 (patched)
> > 
> >
> > Can we avoid HTML code here? We typically used HTML for getting tables 
> > properly formatted as the apache site rendering otherwise caused issues in 
> > the resulting HTML code.
> > 
> > If not done already, I would suggest you to play with the site renderer 
> > a bit to see if the results are actually what you are hoping for.
> 
> Till Toenshoff wrote:
> Just noticed you actually had that in the testing done section - sorry 
> for not seeing that earlier.

I had a look through the RDiscount docs (the markdown rendering engine we use) 
but I didn't find any way to force a line break.
I switched to vertical bars here, but the same issue remains in the 
`**Note:**`-lines in some of the descriptions, which are rendered as a single 
paragraph in HTML, but I don't know how to fix this.


- Benno


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61495/#review182549
---


On Aug. 10, 2017, 10:43 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61495/
> ---
> 
> (Updated Aug. 10, 2017, 10:43 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-5078
> https://issues.apache.org/jira/browse/MESOS-5078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for possible task reasons.
> 
> 
> Diffs
> -
> 
>   docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
>   docs/task-state-reasons.md PRE-CREATION 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
> 
> 
> Diff: https://reviews.apache.org/r/61495/diff/3/
> 
> 
> Testing
> ---
> 
> Built website with site/build.sh and verified it renders ok.
> 
> HTML preview: 
> http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-reasons/index.html
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 61495: Add documentation for possible task reasons.

2017-08-10 Thread Benno Evers 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61495/
---

(Updated Aug. 10, 2017, 10:43 a.m.)


Review request for mesos, Alexander Rukletsov and Till Toenshoff.


Changes
---

Change -tags to vertical bars


Summary (updated)
-

Add documentation for possible task reasons.


Bugs: MESOS-5078
https://issues.apache.org/jira/browse/MESOS-5078


Repository: mesos


Description (updated)
---

Add documentation for possible task reasons.


Diffs (updated)
-

  docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
  docs/task-state-reasons.md PRE-CREATION 
  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 


Diff: https://reviews.apache.org/r/61495/diff/3/

Changes: https://reviews.apache.org/r/61495/diff/2-3/


Testing
---

Built website with site/build.sh and verified it renders ok.

HTML preview: 
http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-reasons/index.html


Thanks,

Benno Evers