Re: Review Request 43220: Added script to generate docs from endpoint help strings.

[Ben Mahler]

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


Ship it!




Thanks! I made some minor comments but took care of those for you prior to 
committing.


support/generate-endpoint-help.py (lines 35 - 37)


I noticed mixed use of single quotes and double quotes, any method to the 
madness?



support/generate-endpoint-help.py (lines 46 - 47)


Seems simpler to just append \n here and use a regular string literal?



support/generate-endpoint-help.py (lines 77 - 93)


Could we make the line wrapping formatting consistent here?


- Ben Mahler


On Feb. 8, 2016, 7:06 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43220/
> ---
> 
> (Updated Feb. 8, 2016, 7:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Invoke this script from anywhere within the mesos source tree
> (including in the build directory) to autogenerate documentation for
> all endpoint strings and install them into the docs/endpoint folder.
> In a subsequent commit we commit all of these autogenerated files back
> to the source repo and add a link in home.md to find them.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43220/diff/
> 
> 
> Testing
> ---
> 
> Ran and generated files from it.  Then rebuilt the website and verified that 
> everything looked as it should.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 43220: Added script to generate docs from endpoint help strings.

[Kevin Klues]

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

(Updated Feb. 8, 2016, 7:06 a.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Updated to address bmahler's newest comments.


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


Repository: mesos


Description
---

Invoke this script from anywhere within the mesos source tree
(including in the build directory) to autogenerate documentation for
all endpoint strings and install them into the docs/endpoint folder.
In a subsequent commit we commit all of these autogenerated files back
to the source repo and add a link in home.md to find them.


Diffs (updated)
-

  support/generate-endpoint-help.py PRE-CREATION 

Diff: https://reviews.apache.org/r/43220/diff/


Testing
---

Ran and generated files from it.  Then rebuilt the website and verified that 
everything looked as it should.


Thanks,

Kevin Klues

Re: Review Request 43220: Added script to generate docs from endpoint help strings.

[Kevin Klues]

> On Feb. 5, 2016, 5:17 a.m., Ben Mahler wrote:
> > support/generate-endpoint-help.py, lines 225-227
> > 
> >
> > Do you need the empty check? Also I think we've been doing `if 
> > new_name` in our python style.

Unfortunately, I do need the empty check because runing join() with an empty 
string at the end produces an extra, unwanted "/"


- Kevin


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


On Feb. 5, 2016, 3:30 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43220/
> ---
> 
> (Updated Feb. 5, 2016, 3:30 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Invoke this script from anywhere within the mesos source tree
> (including in the build directory) to autogenerate documentation for
> all endpoint strings and install them into the docs/endpoint folder.
> In a subsequent commit we commit all of these autogenerated files back
> to the source repo and add a link in home.md to find them.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43220/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 43220: Added script to generate docs from endpoint help strings.

[Kevin Klues]

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

(Updated Feb. 5, 2016, 11:17 p.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Updated to address bmahler's comments.


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


Repository: mesos


Description
---

Invoke this script from anywhere within the mesos source tree
(including in the build directory) to autogenerate documentation for
all endpoint strings and install them into the docs/endpoint folder.
In a subsequent commit we commit all of these autogenerated files back
to the source repo and add a link in home.md to find them.


Diffs (updated)
-

  support/generate-endpoint-help.py PRE-CREATION 

Diff: https://reviews.apache.org/r/43220/diff/


Testing (updated)
---

Ran and generated files from it.  Then rebuilt the website and verified that 
everything looked as it should.


Thanks,

Kevin Klues

Re: Review Request 43220: Added script to generate docs from endpoint help strings.

[Ben Mahler]

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



Looking great, just some minor thoughts around ways to make this a bit easier 
on the reader of the code. Curious to hear your thoughts!


support/generate-endpoint-help.py (line 35)


typo: cause



support/generate-endpoint-help.py (line 36)


typo: we



support/generate-endpoint-help.py (lines 55 - 60)


Do we need the name indexing? It looks like we could just store the Popen 
objects in a list for cleanup purposes. Could start_master just return the 
Popen object instead of the index into this map?

Since we only create the master followed by the agent, we don't have both 
subprocesses alive at the same time, could we simplify if we assume we won't 
have more than a single active subprocess for now?



support/generate-endpoint-help.py (lines 76 - 92)


Would be nice to indent consistently here if possible, should we wrap the 
add_argument calls on a newline?



support/generate-endpoint-help.py (lines 77 - 79)


Should this be a 4 space indent?



support/generate-endpoint-help.py (lines 100 - 111)


Do we need these? They seem to exist only to rename os.makedirs and 
shutil.rmtree?

For the os.makedirs error handling, we could just do the following in the 
caller code to make the 'already exists' case explicit:

```
if not os.path.exists(directory):
os.makedirs(directory)
```



support/generate-endpoint-help.py (line 103)


Should this be a 2 space indent?



support/generate-endpoint-help.py (lines 114 - 127)


Could we avoid the string -> list conversion by just starting with the 
command in list form?

It looks like the caller code doesn't need to use a string.

After that change, this could take s/cmd/args/



support/generate-endpoint-help.py (lines 133 - 134)


Where does this happen? Looks like we don't delete the key from the map 
after the kill?



support/generate-endpoint-help.py (lines 186 - 188)


A maintenance endpoint might be a good example to include in general, since 
these endpoints have a slash in the name?



support/generate-endpoint-help.py (lines 242 - 243)


Could we prefix this sentence with a bold NOTE?



support/generate-endpoint-help.py (line 255)


Let's omit any reference to mesosphere ;)



support/generate-endpoint-help.py (line 264)


Ditto here ;)



support/generate-endpoint-help.py (lines 270 - 280)


Still a little tricky for me to figure this out, could the comment include 
an example?

One thought that comes to mind is, should this just return a 'list' of 
links for a given process, and the caller loops over processes calling this? 
Then the caller loops over the link list and stringifies as needed. Just 
thinking that perhaps this function should not be doing the overall string 
construction for the caller as that might be a bit tougher to follow.

Curious to see if we can make this easier for dummies like me :)



support/generate-endpoint-help.py (line 304)


relative_path


- Ben Mahler


On Feb. 5, 2016, 11:17 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43220/
> ---
> 
> (Updated Feb. 5, 2016, 11:17 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Invoke this script from anywhere within the mesos source tree
> (including in the build directory) to autogenerate documentation for
> all endpoint strings and install them into the docs/endpoint folder.
> In a subsequent commit we commit all of these autogenerated files back
> to the source repo and add a link in home.md to find them.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py PRE-CREATION 
> 
> Diff: 

Re: Review Request 43220: Added script to generate docs from endpoint help strings.

[Kevin Klues]

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

(Updated Feb. 5, 2016, 3:30 a.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Updated to match new JSON schema.


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


Repository: mesos


Description
---

Invoke this script from anywhere within the mesos source tree
(including in the build directory) to autogenerate documentation for
all endpoint strings and install them into the docs/endpoint folder.
In a subsequent commit we commit all of these autogenerated files back
to the source repo and add a link in home.md to find them.


Diffs (updated)
-

  support/generate-endpoint-help.py PRE-CREATION 

Diff: https://reviews.apache.org/r/43220/diff/


Testing
---


Thanks,

Kevin Klues

Re: Review Request 43220: Added script to generate docs from endpoint help strings.

[Ben Mahler]

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



Awesome!


support/generate-endpoint-help.py (lines 39 - 40)


Hm.. could we make the key, value types a bit more explicit here?



support/generate-endpoint-help.py (lines 42 - 43)


Wonder if this will tell the user nicely that this needs to be run within 
the git repo.



support/generate-endpoint-help.py (lines 53 - 75)


Maybe some newlines here?



support/generate-endpoint-help.py (lines 64 - 75)


I wonder if we want to let users set flags, is the intention that there may 
be some help information that requires different flag values? If that were the 
case, almost seems as though this script should be smart enough to know which 
incantantions to run to produce all of the necessary help files.



support/generate-endpoint-help.py (lines 85 - 143)


Hm.. can we avoid all the `_or_die` wrappers and just let the exceptions 
throw?



support/generate-endpoint-help.py (lines 189 - 197)


Should this loop until a duration elapses? (printing the error if it can't 
pass after duration elapses would be nice)

Might also want a small sleep between each iteration?



support/generate-endpoint-help.py (lines 206 - 208)


Is this how we do constants in the python helpers? Or do we use CAPS and 
variables instead of functions?



support/generate-endpoint-help.py (lines 221 - 227)


Might be nice to show an example transformation?



support/generate-endpoint-help.py (lines 225 - 227)


Do you need the empty check? Also I think we've been doing `if new_name` in 
our python style.



support/generate-endpoint-help.py (line 230)


Maybe top_dir?



support/generate-endpoint-help.py (lines 230 - 241)


Maybe an example transformation would help?



support/generate-endpoint-help.py (line 244)


Should it be turned into json much earlier in the program? I assume we 
don't need to manipulate the raw string?



support/generate-endpoint-help.py (line 253)


generate_index made me think it was the entire index and I didn't initially 
realize that this was a subsection. Perhaps something like generate_links?

Should topdir be something like prefix? Or prefix_path, prefix_dir?



support/generate-endpoint-help.py (lines 269 - 308)


Maybe this could be one single string with percent parameters for the 
different sections that will be filled in?



support/generate-endpoint-help.py (line 339)


/health is generally what we recommend for checking "liveness"



support/generate-endpoint-help.py (line 352)


/health is generally what we recommend for checking "liveness"



support/generate-endpoint-help.py (lines 356 - 363)


Do we need these?



support/generate-endpoint-help.py (lines 366 - 383)


I wonder if we need these? Sounds like master and agent should be 
parameters rather than different function names?


- Ben Mahler


On Feb. 5, 2016, 3:30 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43220/
> ---
> 
> (Updated Feb. 5, 2016, 3:30 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Invoke this script from anywhere within the mesos source tree
> (including in the build directory) to autogenerate documentation for
> all endpoint strings and install them into the docs/endpoint folder.
> In a subsequent commit we commit all of these autogenerated files back
> to the source repo and add a link in home.md to find them.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43220/diff/
>