The choices are:

(1) Action parameter: "aurora job update
--action=start/pause/resume/abort".
Pros: it's part of the "job" noun and the familiar "update" verb.
Cons:
- the action parameter is ugly,
- we lose automatic parameter checking, since the different actions take
different parameters.
- it's difficult to prevent it from interfering with existing users while
the server-driven update is under
  development and not working. (I see this as an *extremely* serious
blocking problem.)

(2) Multiple verbs on job: "aurora job update_start", "aurora job
update_pause", ...
Pros:
- it's part of the "job" noun,
- parameter checking works.
- it's reasonably easy to keep this separated so that it doesn't show up
until we're ready.
Cons:
- it's hideous.

(3) Update noun: "aurora update start/pause/resume/abort".
Pros:
- parameter checking works.
- easy to keep under wraps without annoying users.
- (arguable) I think it makes sense, because it represents a command that
interacts with a different service.
Cons:
- It's not part of the job noun.


I strongly prefer option 3. But I don't want to waste time bikesheding. So
if you guys strongly disagree with me, fine, just tell me which one you're
willing to agree on.

        -Mark

On Mon, Sep 15, 2014 at 2:28 PM, Bill Farner <wfar...@apache.org> wrote:

> 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 <
> mchucarr...@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
>> >
>> >
>>
>>
>

Reply via email to