Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Mark Chu-Carroll]

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

(Updated May 15, 2014, 12:08 p.m.)


Review request for Aurora and Joe Smith.


Changes
---

Rebase to master. (Why? I don't know. Git complained.)


Bugs: aurora-253
https://issues.apache.org/jira/browse/aurora-253


Repository: aurora


Description
---

Add an updated version of the clientv2 doc to apache.

(Also make a few changes in the interfaces to match the updated doc.)


Diffs (updated)
-

  docs/clientv2.md PRE-CREATION 

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


Testing
---

n/a


Thanks,

Mark Chu-Carroll

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Mark Chu-Carroll]

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

(Updated May 15, 2014, 12:02 p.m.)


Review request for Aurora and Joe Smith.


Bugs: aurora-253
https://issues.apache.org/jira/browse/aurora-253


Repository: aurora


Description
---

Add an updated version of the clientv2 doc to apache.

(Also make a few changes in the interfaces to match the updated doc.)


Diffs
-

  docs/clientv2.md PRE-CREATION 
  src/main/python/apache/aurora/client/cli/__init__.py 
4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
  src/main/python/apache/aurora/client/cli/jobs.py 
3b327df5f9c5f1d5e7e68863191313921a8dde44 

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


Testing
---

n/a


Thanks,

Mark Chu-Carroll

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Joe Smith]

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

Ship it!


awesome!!!

- Joe Smith


On May 12, 2014, 5:15 a.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18979/
> ---
> 
> (Updated May 12, 2014, 5:15 a.m.)
> 
> 
> Review request for Aurora, Dave Lester and Tom Galloway.
> 
> 
> Bugs: aurora-253
> https://issues.apache.org/jira/browse/aurora-253
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an updated version of the clientv2 doc to apache.
> 
> (Also make a few changes in the interfaces to match the updated doc.)
> 
> 
> Diffs
> -
> 
>   docs/clientv2.md PRE-CREATION 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 3b327df5f9c5f1d5e7e68863191313921a8dde44 
> 
> Diff: https://reviews.apache.org/r/18979/diff/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Mark Chu-Carroll]

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

(Updated May 12, 2014, 8:15 a.m.)


Review request for Aurora, Dave Lester and Tom Galloway.


Changes
---

Address Joe's comments.


Bugs: aurora-253
https://issues.apache.org/jira/browse/aurora-253


Repository: aurora


Description
---

Add an updated version of the clientv2 doc to apache.

(Also make a few changes in the interfaces to match the updated doc.)


Diffs (updated)
-

  docs/clientv2.md PRE-CREATION 
  src/main/python/apache/aurora/client/cli/__init__.py 
4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
  src/main/python/apache/aurora/client/cli/jobs.py 
3b327df5f9c5f1d5e7e68863191313921a8dde44 

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


Testing
---

n/a


Thanks,

Mark Chu-Carroll

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Mark Chu-Carroll]

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



docs/clientv2.md


No, it's part of the paragraph for the previous bullet.



docs/clientv2.md


Good point; killall didn't exist when I wrote this doc!



docs/clientv2.md


It does use updateconfig. The semantics are exactly the same as the 
original update command.



docs/clientv2.md


Strange - that all looks fine in my editor. I'm wiping all the indents and 
retyping them; guessing some tab characters accidentally got inserted.



- Mark Chu-Carroll


On March 21, 2014, 1:58 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18979/
> ---
> 
> (Updated March 21, 2014, 1:58 p.m.)
> 
> 
> Review request for Aurora, Dave Lester and Tom Galloway.
> 
> 
> Bugs: aurora-253
> https://issues.apache.org/jira/browse/aurora-253
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an updated version of the clientv2 doc to apache.
> 
> (Also make a few changes in the interfaces to match the updated doc.)
> 
> 
> Diffs
> -
> 
>   docs/clientv2.md PRE-CREATION 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 3b327df5f9c5f1d5e7e68863191313921a8dde44 
> 
> Diff: https://reviews.apache.org/r/18979/diff/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Joe Smith]

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


Whoa- awesome 


docs/clientv2.md


(maybe instead remove one of the thats?) "The problem we're facing..."



docs/clientv2.md


This section is a great summary of the issues, +1



docs/clientv2.md


should this have a dash?



docs/clientv2.md


oh man. didn't realize this was gonna be a part of this. _awesome_



docs/clientv2.md


maybe include a note to mention killall?



docs/clientv2.md


is there a way to configure/control/specify this process? (I don't recall 
if this uses UpdateConfig or not, happy to check then report back if you don't 
recall either)



docs/clientv2.md


isn't cron the noun? Looks like it at both line 177 and 227, but I'm kinda 
confused by 228+229. If not, would appreciate some clarification if you don't 
mind



docs/clientv2.md


looks like the indenting got a lil funky here



docs/clientv2.md


hoorah!


- Joe Smith


On March 21, 2014, 10:58 a.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18979/
> ---
> 
> (Updated March 21, 2014, 10:58 a.m.)
> 
> 
> Review request for Aurora, Dave Lester and Tom Galloway.
> 
> 
> Bugs: aurora-253
> https://issues.apache.org/jira/browse/aurora-253
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an updated version of the clientv2 doc to apache.
> 
> (Also make a few changes in the interfaces to match the updated doc.)
> 
> 
> Diffs
> -
> 
>   docs/clientv2.md PRE-CREATION 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 3b327df5f9c5f1d5e7e68863191313921a8dde44 
> 
> Diff: https://reviews.apache.org/r/18979/diff/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Mark Chu-Carroll]

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

(Updated March 21, 2014, 1:58 p.m.)


Review request for Aurora, Dave Lester and Tom Galloway.


Changes
---

Address Tom's latest review comments.


Bugs: aurora-253
https://issues.apache.org/jira/browse/aurora-253


Repository: aurora


Description
---

Add an updated version of the clientv2 doc to apache.

(Also make a few changes in the interfaces to match the updated doc.)


Diffs (updated)
-

  docs/clientv2.md PRE-CREATION 
  src/main/python/apache/aurora/client/cli/__init__.py 
4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
  src/main/python/apache/aurora/client/cli/jobs.py 
3b327df5f9c5f1d5e7e68863191313921a8dde44 

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


Testing
---

n/a


Thanks,

Mark Chu-Carroll

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Tom Galloway]

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


Under "Schedule Noun (Cron)" (line 218 on) you didn't add "aurora cron" before 
the command examples.

line 330-1: "The context is created dynamically by the noun object owning the 
verb being executed."
change to "The noun object owning the executing verb dynamically creates the 
context."

- Tom Galloway


On March 12, 2014, 9:44 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18979/
> ---
> 
> (Updated March 12, 2014, 9:44 p.m.)
> 
> 
> Review request for Aurora, Dave Lester and Tom Galloway.
> 
> 
> Bugs: aurora-253
> https://issues.apache.org/jira/browse/aurora-253
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an updated version of the clientv2 doc to apache.
> 
> (Also make a few changes in the interfaces to match the updated doc.)
> 
> 
> Diffs
> -
> 
>   docs/clientv2.md PRE-CREATION 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 3b327df5f9c5f1d5e7e68863191313921a8dde44 
> 
> Diff: https://reviews.apache.org/r/18979/diff/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Mark Chu-Carroll]

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

(Updated March 12, 2014, 5:44 p.m.)


Review request for Aurora, Dave Lester and Tom Galloway.


Changes
---

review changes.


Bugs: aurora-253
https://issues.apache.org/jira/browse/aurora-253


Repository: aurora


Description
---

Add an updated version of the clientv2 doc to apache.

(Also make a few changes in the interfaces to match the updated doc.)


Diffs (updated)
-

  docs/clientv2.md PRE-CREATION 
  src/main/python/apache/aurora/client/cli/__init__.py 
4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
  src/main/python/apache/aurora/client/cli/jobs.py 
3b327df5f9c5f1d5e7e68863191313921a8dde44 

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


Testing
---

n/a


Thanks,

Mark Chu-Carroll

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Mark Chu-Carroll]

> On March 11, 2014, 3:16 p.m., Tom Galloway wrote:
> > Line 148: Each noun will be separate component -> Each noun will be a 
> > separate component
> > 
> > noun verb syntax; are you wedded to this? "aurora get quota" and "aurora 
> > create job" are more English (if Tarzan sounding) like than "aurora quota 
> > get" and "aurora job create" and just seem to read better as verb noun to 
> > me.
> > 
> > Line 200: "* `submit jobkey config`:  submits a job to a cluster, launching 
> > the jobs task. "  I'm not clear here on whether this should be "job's 
> > tasks", "job's task", or "task named 'jobs'" (for the last, I'm thinking 
> > going through the "jobs" list in the config file, could be a task, although 
> > a bit different from the defined ones.)
> > 
> > Line 205: "they'llbe" -> "they'll be"
> > 
> > In the noun sections, I'd strongly suggest including at least the noun, and 
> > possibly "aurora "  in the examples for clarity since you've made a 
> > big deal of the noun/verb syntax thing. It took me a minute to realize 
> > "submit jobkey config" was short for "aurora job submit jobkey config"
> > 
> > Line 217-218: "cluster/role, all jobs running under the role will be 
> > listed". I'd add "on the cluster" between "running" and "under the role" 
> > for claity
> > 
> > Line 224: "The cron commands all manipulate cron schedule entries" Maybe 
> > just me, but I'd change this to "All cron commands manipulate..."; 
> > initially I was reading this as "The cron commands all..." with "commands" 
> > as a verb. 
> > 
> > Line 227: Again probably just me, but "schedule a job to run by cron." has 
> > me doing flashbacks to Conan the Barbarian going "By Crom!" and is a bit 
> > awkward. How about "schedule a cron-run job"
> > 
> > Line 234: "Quotas are a data object maintained by the scheduler that 
> > specifies the maximum  
> > resources that may be consumed by jobs owned by a particular role."  
> > Definitely change "Quotas are a data object" to "A quota is a data object". 
> > I'd suggest breaking up this four chained concepts sentence info multiples; 
> > "A quota is a data object maintained by the scheduler. It specifies the 
> > maximum resources that jobs owned by a particular role may consume." Maybe 
> > also emphasize that it does *not* work on a single job as specified by a 
> > jobkey, but on all jobs sharing the specified cluster and role values.
> > 
> > Line 247: "command-line" -> "command line"
> > 
> > Line 323: remove extra whitespace.
> > 
> > Lines 326 and 328: "command-line" -> "command line"
> > 
> > Line 331: "When commands are executed" -> "When commands execute"
> > 
> > Lines 331-2: "When commands are executed, they're given an instance of a 
> > *context object*.  
> > The context object must be an instance of a subclass of 
> > `AuroraCommandContext`."   This is confusing, as you seem to be using 
> > "object" and "instance" interchangably. It seems that the command gets an 
> > instance of an instance? 
> > 
> > Two things I'd suggest mentioning;
> > 
> > 1) Is every current open source command line command and option going to be 
> > available in some way "out of the box" with the new client? In other words, 
> > is there any existing functionality being removed (or, for that matter, 
> > added) in this new client?
> > 
> > 2) Will the new client still have the ability to add hooks to the existing 
> > commands? I'd think so, since as 
> > the hooks stuff operates on the underlying Aurora API which the client 
> > commands call.
> > 
> > 3) Not for open source, but will the Twitter-internal aurora packer and 
> > aurora deploy commands still be implemented in the new client?

Fixed most of this. For the remainder:
- scheduling a job to run by cron is (IME) the common phrasing.
- we are committed to noun/verb order. That got nailed down during design 
reviews.
- all of  the functionality of the v1 client will, at least eventually, be 
supported by v2. Not necessarily every option
  in exactly the same way - but equivalent functionality. I think that that's 
beyond the scope of this document.
- Hooks work exactly the way that they do in the existing client - no changes.
- We want to keep discussions of twitter-internal stuff out of apache - it just 
isn't relevant here. (But for a quick answer, yes.)


- Mark


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


On March 11, 2014, 12:34 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18979/
> ---
> 
> (Updated March 11, 2014, 12:34 p.m.)
> 
> 
> Review request for Aurora, Dave Lester and Tom Galloway.
> 
> 
> Bugs: aurora-253
> https://issues.apache.

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Tom Galloway]

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


Line 148: Each noun will be separate component -> Each noun will be a separate 
component

noun verb syntax; are you wedded to this? "aurora get quota" and "aurora create 
job" are more English (if Tarzan sounding) like than "aurora quota get" and 
"aurora job create" and just seem to read better as verb noun to me.

Line 200: "* `submit jobkey config`:  submits a job to a cluster, launching the 
jobs task. "  I'm not clear here on whether this should be "job's tasks", 
"job's task", or "task named 'jobs'" (for the last, I'm thinking going through 
the "jobs" list in the config file, could be a task, although a bit different 
from the defined ones.)

Line 205: "they'llbe" -> "they'll be"

In the noun sections, I'd strongly suggest including at least the noun, and 
possibly "aurora "  in the examples for clarity since you've made a big 
deal of the noun/verb syntax thing. It took me a minute to realize "submit 
jobkey config" was short for "aurora job submit jobkey config"

Line 217-218: "cluster/role, all jobs running under the role will be listed". 
I'd add "on the cluster" between "running" and "under the role" for claity

Line 224: "The cron commands all manipulate cron schedule entries" Maybe just 
me, but I'd change this to "All cron commands manipulate..."; initially I was 
reading this as "The cron commands all..." with "commands" as a verb. 

Line 227: Again probably just me, but "schedule a job to run by cron." has me 
doing flashbacks to Conan the Barbarian going "By Crom!" and is a bit awkward. 
How about "schedule a cron-run job"

Line 234: "Quotas are a data object maintained by the scheduler that specifies 
the maximum  
resources that may be consumed by jobs owned by a particular role."  Definitely 
change "Quotas are a data object" to "A quota is a data object". I'd suggest 
breaking up this four chained concepts sentence info multiples; "A quota is a 
data object maintained by the scheduler. It specifies the maximum resources 
that jobs owned by a particular role may consume." Maybe also emphasize that it 
does *not* work on a single job as specified by a jobkey, but on all jobs 
sharing the specified cluster and role values.

Line 247: "command-line" -> "command line"

Line 323: remove extra whitespace.

Lines 326 and 328: "command-line" -> "command line"

Line 331: "When commands are executed" -> "When commands execute"

Lines 331-2: "When commands are executed, they're given an instance of a 
*context object*.  
The context object must be an instance of a subclass of 
`AuroraCommandContext`."   This is confusing, as you seem to be using "object" 
and "instance" interchangably. It seems that the command gets an instance of an 
instance? 

Two things I'd suggest mentioning;

1) Is every current open source command line command and option going to be 
available in some way "out of the box" with the new client? In other words, is 
there any existing functionality being removed (or, for that matter, added) in 
this new client?

2) Will the new client still have the ability to add hooks to the existing 
commands? I'd think so, since as 
the hooks stuff operates on the underlying Aurora API which the client commands 
call.

3) Not for open source, but will the Twitter-internal aurora packer and aurora 
deploy commands still be implemented in the new client?

- Tom Galloway


On March 11, 2014, 4:34 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18979/
> ---
> 
> (Updated March 11, 2014, 4:34 p.m.)
> 
> 
> Review request for Aurora, Dave Lester and Tom Galloway.
> 
> 
> Bugs: aurora-253
> https://issues.apache.org/jira/browse/aurora-253
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an updated version of the clientv2 doc to apache.
> 
> (Also make a few changes in the interfaces to match the updated doc.)
> 
> 
> Diffs
> -
> 
>   docs/clientv2.md PRE-CREATION 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 3b327df5f9c5f1d5e7e68863191313921a8dde44 
> 
> Diff: https://reviews.apache.org/r/18979/diff/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Mark Chu-Carroll]

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

(Updated March 11, 2014, 12:34 p.m.)


Review request for Aurora, Dave Lester and Tom Galloway.


Changes
---

Got rid of the lingering packer references, and added a bit more implementation 
detail.


Bugs: aurora-253
https://issues.apache.org/jira/browse/aurora-253


Repository: aurora


Description
---

Add an updated version of the clientv2 doc to apache.

(Also make a few changes in the interfaces to match the updated doc.)


Diffs (updated)
-

  docs/clientv2.md PRE-CREATION 
  src/main/python/apache/aurora/client/cli/__init__.py 
4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
  src/main/python/apache/aurora/client/cli/jobs.py 
3b327df5f9c5f1d5e7e68863191313921a8dde44 

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


Testing
---

n/a


Thanks,

Mark Chu-Carroll

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Tom Galloway]

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


Need to read in more detail, but one quick point; you mention Packer several 
times. To my knowledge, Packer's not been open sourced and I don't think there 
are plans to do so. If that's correct, probably should remove references to it 
in a doc that's to be posted on Apache.

- Tom Galloway


On March 10, 2014, 10:24 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18979/
> ---
> 
> (Updated March 10, 2014, 10:24 p.m.)
> 
> 
> Review request for Aurora, Dave Lester and Tom Galloway.
> 
> 
> Bugs: aurora-253
> https://issues.apache.org/jira/browse/aurora-253
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add an updated version of the clientv2 doc to apache.
> 
> (Also make a few changes in the interfaces to match the updated doc.)
> 
> 
> Diffs
> -
> 
>   docs/clientv2.md PRE-CREATION 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 
> 
> Diff: https://reviews.apache.org/r/18979/diff/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 18979: Add an updated version of the clientv2 doc to apache.

[Mark Chu-Carroll]

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

(Updated March 10, 2014, 6:24 p.m.)


Review request for Aurora, Dave Lester and Tom Galloway.


Changes
---

Forgot to put Tom on the people line.


Bugs: aurora-253
https://issues.apache.org/jira/browse/aurora-253


Repository: aurora


Description
---

Add an updated version of the clientv2 doc to apache.

(Also make a few changes in the interfaces to match the updated doc.)


Diffs
-

  docs/clientv2.md PRE-CREATION 
  src/main/python/apache/aurora/client/cli/__init__.py 
4a6a7eef7b781be79a3d40776a3bd6f0c6e8c4c0 

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


Testing
---

n/a


Thanks,

Mark Chu-Carroll