aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bill Farner <wfar...@apache.org>
Subject Re: Review Request 25255: Implement server-driven update commands.
Date Mon, 15 Sep 2014 18:28:42 GMT
In the world of 'nouns' and 'verbs', i imagine a user would think "i want
to update my job", not "i want to start an update".  So if the goal of
organizing commands by nouns is to make it intuitive, i think "update
start" is less natural than "job update".

-=Bill

On Mon, Sep 15, 2014 at 10:25 AM, Mark Chu-Carroll <
mchucarroll@twopensource.com> wrote:

>
>
> > 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
> >
> >
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message