As I said - the version with the sub-actions fouls up parameter checking. (That's the "action" parameter option. It doesn't matter whether it's "--action=start" or "--start"; in fact, the --action at least lets the parameter parser do a little bit of checking.)
It's possible to add another layer of dispatch to the framework, but it's a fair bit of work. But that's not a problem. The problem there is that for users, "noun/verb" is a very natural idea. When you start going to extra layers of dispatch, the complexity of the command to users changes pretty significantly. It's no longer "what do I want to interact with, and what do I want to do to it", but "How many layers of commands do I need for this?" I think that adding layers changes the cognitive load of working with the system in a very unfortunate way. I still think that of the options, the "update" noun is still the best. The distinction between nouns is always going to be somewhat arbitrary - why is "ssh" part of task instead of job? Why is quota a noun and user a parameter, instead of user a noun and quota a parameter? There is a sensible distinction in update. The fundamental question behind nouns is always "what kind of thing do I want to interact with?" And there's a reasonable answer: "an update process on a server". It can be explained to users in the one-line usage message, and it's easy to remember. On Mon, Sep 15, 2014 at 5:16 PM, Maxim Khutornenko <[email protected]> wrote: > I guess there is also the option you originally proposed: "aurora job > update --start|--pause|--resume|--abort". > > If you don't like the option syntax, would it be too much work to > support double "verb" (or rather double "noun") syntax, like "aurora > job update start"? > > On Mon, Sep 15, 2014 at 2:07 PM, Mark Chu-Carroll > <[email protected]> wrote: > > 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 <[email protected]> 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 > >> <[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 > >>> > > >>> > > >>> > >> > > >
