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 < [email protected]> 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 > > > > > >
