> On Sept. 15, 2014, 12:04 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/cli/standalone_client.py, line 121
> > <https://reviews.apache.org/r/25255/diff/2/?file=685554#file685554line121>
> >
> >     I am still not sure why we are going with the top level noun for job 
> > updates. It's quite strange to see create/restart under "job" but "update" 
> > living on its own. Is this only to avoid collision with the existing client 
> > update? If so, it's only there until we iron out the async updates. We 
> > could clearly mark new server side update options as "BETA" in command help 
> > to avoid any confusion. I think the long term consistency here is more 
> > important than temporary ambiguity.

I thought that it made sense to be a separate noun.

There's two main reasons why I think it makes sense for this to be a distinct 

(1) All of the "job" commands on the client are client-driven logic. This 
isn't. This is server-side logic,
  but not truly scheduler logic. It's creating an update process on a server 
somewhere, and then allowing
  you to interact with it. You're not using these commands to interact with a 
job - you're interacting with
  an update process.  You're interacting with a different kind of *thing*, and 
each kind of thing is 
  handled as a different noun.  (Think about the "task" noun. A task is a piece 
of a job, but we handle
  it differently. We still use jobkeys to specify them - but for operations 
that work on the running
  task in a mesos slave, we put them in a different noun than the operations 
that talk to the scheduler
  about the job itself. I think this is a similar case.)

(2) There's a collection of sub-commands, and the subcommands are behaviorally 
very much the kinds of
  operations you find on nouns elsewhere in the client. Putting all of this 
under the "update" verb
  on jobs results in a lot of ugliness - you end up with the "action" parameter 
from the earlier version
  of this review, and you lose hte ability to have the option parser do 
parameter checking for the 
  different actions. You end up with a better syntax, and clearer semantics 
when you treat start/pause/resume/abort/status as verbs on an update noun.
Finally, I very strongly disagree that it's OK to introduce the server-driven 
updates as options to the existing "update" command. Disrupting users with 
unsupported, non-working test functionality is *not* acceptable in a production 

One fringe benefit of this approach is that it's really easy to leave it out of 
the production executable until it's ready to go.


- Mark

This is an automatically generated e-mail. To reply, visit:

On Sept. 11, 2014, 10:55 a.m., Mark Chu-Carroll wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25255/
> -----------------------------------------------------------
> (Updated Sept. 11, 2014, 10:55 a.m.)
> Review request for Aurora.
> Repository: aurora
> Description
> -------
> This change contains the basic commands needed to work with the
> scheduler-driven updater. (It does not yet cover querying for the status
> of the update; that will come in a subsequent change.)
> Diffs
> -----
>   src/main/python/apache/aurora/client/cli/BUILD 
> ebe681a0d1735b7cc695dc3b7a14c4292d87ae32 
>   src/main/python/apache/aurora/client/cli/context.py 
> 51c7d24dca664e476e62f1864d095416dfab70e4 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> ebc22aaa5a8aed311897b3ce9632b6f7175b6080 
>   src/main/python/apache/aurora/client/cli/standalone_client.py 
> b7c8de66d6e4664b536911f826e36a984e8d0fef 
>   src/main/python/apache/aurora/client/cli/update.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/BUILD 
> e1f9ebf96774b8f5c75de8570c6ba87d953ab649 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> a1e7a5a94a2d336239df98e2600658b97c546901 
>   src/test/python/apache/aurora/client/cli/test_supdate.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/util.py 
> 95a2123e127c9811fd2305e71cfc5c7c4376f904 
> Diff: https://reviews.apache.org/r/25255/diff/
> Testing
> -------
> New suite of tests for the new command; all unit tests pass.
> Thanks,
> Mark Chu-Carroll

Reply via email to