> 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
noun.
(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
tool.
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
- Mark
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25255/#review53347
-----------------------------------------------------------
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
>
>
