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 <ma...@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 <mchucarr...@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 <kevi...@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/>
>>
>
>
>

Reply via email to