Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.incubator.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <CAOTkfX7X-EKBdJZO4XH2AFhUHHTrgDVTrZRvZU9ONFq0PEhBSw@mail.gmail.com>
References: <20140312222509.19462.40647@reviews.apache.org>
	<20140313170711.32376.89969@reviews.apache.org>
	<CAFGkSCk64kuoJ9GBeGrS7tWpzPq8acTw58bEmWDX+HctsKVd_Q@mail.gmail.com>
	<7A8C5E1B-CA63-406C-A6AA-5B54E3F46F5C@apache.org>
	<CAOTkfX7X-EKBdJZO4XH2AFhUHHTrgDVTrZRvZU9ONFq0PEhBSw@mail.gmail.com>
Date: Thu, 13 Mar 2014 11:17:03 -0700
Message-ID: 
 <CAFGkSCneZ+Bx6+xiAxOjR0_M3Kv1MS6frNWytL6xeH2T+bZa9A@mail.gmail.com>
Subject: Re: Review Request 19159: Add killall.
From: Mark Chu-Carroll <mchucarroll@apache.org>
To: Maxim Khutornenko <maxim@apache.org>
Cc: Mark Chu-Carroll <mchucarroll@apache.org>,
 Kevin Sweeney <kevints@apache.org>,
	Bill Farner <wfarner@apache.org>,
 Aurora <reviews@aurora.incubator.apache.org>
Content-Type: multipart/alternative; boundary=047d7b10d1edd0906a04f480f531

--047d7b10d1edd0906a04f480f531
Content-Type: text/plain; charset=ISO-8859-1

But in some sense, this isn't a standard command. This is a dangerous
command, and we want it to be out of the ordinary. We want to be sure that
it's not done accidentally, and making its flag behavior be strange is
arguably good.

    -Mark


On Thu, Mar 13, 2014 at 11:14 AM, Maxim Khutornenko <maxim@apache.org>wrote:

> What looks odd though is a command that is unusable without the --force
> flag. The standard optional semantic of that flag feels violated here.
>
>
> On Thu, Mar 13, 2014 at 11:10 AM, Maxim Khutornenko <maxim@apache.org>wrote:
>
>> What looks odd though is a command that is unusable without the --force
>> flag. The standard optional semantic of that flag is violated here.
>>
>> On Mar 13, 2014, at 11:07 AM, Mark Chu-Carroll <mchucarroll@apache.org>
>> wrote:
>>
>> I think making a reasonable effort to make it harder to accidentally do
>> this is worthwhile - there's no harm in it, and it might just help.
>>
>>   -Mark
>>
>>
>> On Thu, Mar 13, 2014 at 10:07 AM, Kevin Sweeney <kevints@apache.org>wrote:
>>
>>>    This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/19159/
>>>
>>> On March 12th, 2014, 3:25 p.m. PDT, *Mark Chu-Carroll* wrote:
>>>
>>>   src/main/python/apache/aurora/client/commands/core.py<https://reviews.apache.org/r/19159/diff/1/?file=517771#file517771line399> (Diff
>>> revision 1)
>>>
>>> def show_job_pretty(job):
>>>
>>>    397
>>>
>>> @app.command_option('--force', default=False, action='store_true',
>>>
>>>   This is deliberate: the "kill" command doesn't have a "force" option. So this ensures that there's more than a search-and-replace killall for kill: you need to deliberately use the killall command, and specify the force option.
>>>
>>> The require --shards is also in this change - see the change above in kill.
>>>
>>>  On March 12th, 2014, 3:29 p.m. PDT, *Maxim Khutornenko* wrote:
>>>
>>> Missed the --shards part, thanks.
>>>
>>>  If someone is going to search-and-replace kill with killall without wanting to do that I'm not sure we can stop them from shooting themselves in the foot. Up to you but I'm in favor of dropping this required option.
>>>
>>>
>>> - Kevin
>>>
>>> On March 12th, 2014, 3:11 p.m. PDT, Mark Chu-Carroll wrote:
>>>   Review request for Aurora, Kevin Sweeney and Bill Farner.
>>> By Mark Chu-Carroll.
>>>
>>> *Updated March 12, 2014, 3:11 p.m.*
>>>  *Bugs: * aurora-260 <https://issues.apache.org/jira/browse/aurora-260>
>>>  *Repository: * aurora
>>> Description
>>>
>>> Add killall.
>>>
>>> - the kill command now requires a shards parameter.
>>> - the new killall command only works when run with "--force".
>>> - killall generates a scary warning message, and pauses to give
>>>   the user a chance to abort.
>>>
>>>   Testing
>>>
>>> Modified the existing kill command's test suite, adding new tests of the new functionality. All pass.
>>>
>>> [sun-wukong incubator-aurora (killall)]$ ./pants src/test/python/apache/aurora/client/commands:core
>>> Build operating on targets: OrderedSet([PythonTests(src/test/python/apache/aurora/client/commands/BUILD:core)])
>>> ============================= test session starts =============================
>>> platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
>>> collected 26 items
>>>
>>> src/test/python/apache/aurora/client/commands/test_cancel_update.py ..
>>> src/test/python/apache/aurora/client/commands/test_create.py ......
>>> src/test/python/apache/aurora/client/commands/test_diff.py ...
>>> src/test/python/apache/aurora/client/commands/test_kill.py .....
>>> src/test/python/apache/aurora/client/commands/test_listjobs.py ..
>>> src/test/python/apache/aurora/client/commands/test_restart.py ...
>>> src/test/python/apache/aurora/client/commands/test_status.py ..
>>> src/test/python/apache/aurora/client/commands/test_update.py ...
>>>
>>> ========================= 26 passed in 11.34 seconds ==========================
>>> src.test.python.apache.aurora.client.commands.core                              .....   SUCCESS
>>>
>>>   Diffs
>>>
>>>    - src/main/python/apache/aurora/client/commands/core.py
>>>    (ff0f1f8668c8c405fa3a41b70cae32004034e223)
>>>    - src/test/python/apache/aurora/client/commands/test_kill.py
>>>    (7639dc98bfea0663461d15e3d46f1aedd13b124f)
>>>
>>> View Diff <https://reviews.apache.org/r/19159/diff/>
>>>
>>
>>
>>
>

--047d7b10d1edd0906a04f480f531--