Bill, Chris, and I discussed this yesterday, and the consensus was that a
warning message, followed by a pause to give an opportunity to cancel was
the best option. I'd rather not re-argue the issue yet another time!

 -Mark




On Thu, Mar 13, 2014 at 2:56 PM, Kevin Sweeney <kevi...@apache.org> wrote:

>    This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19159/
>    
> src/main/python/apache/aurora/client/commands/core.py<https://reviews.apache.org/r/19159/diff/1/?file=517771#file517771line386>
>  (Diff
> revision 1)
>
> def show_job_pretty(job):
>
>    384
>
>     print('Shards option is required for kill; use killall to kill all 
> shards')
>
>   file=sys.stderr?
>
>
>    
> src/main/python/apache/aurora/client/commands/core.py<https://reviews.apache.org/r/19159/diff/1/?file=517771#file517771line406>
>  (Diff
> revision 1)
>
> 404
>
>   if not options.force:
>
>   405
>
>     print('killall requires the force option to indicate that you really want 
> to kill the job.')
>
>   406
>
>     exit(1)
>
>   In favor of dropping this, but if you don't this should be file=sys.stderr.
>
>
>    
> src/main/python/apache/aurora/client/commands/core.py<https://reviews.apache.org/r/19159/diff/1/?file=517771#file517771line413>
>  (Diff
> revision 1)
>
> 411
>
>   print('**** Warning! This is going to kill all tasks in this job!')
>
>   412
>
>   print('**** Tasks will be killed in 10 seconds. Press ^C to abort!')
>
>   stderr
>
>
>    
> src/main/python/apache/aurora/client/commands/core.py<https://reviews.apache.org/r/19159/diff/2/?file=518936#file518936line413>
>  (Diff
> revision 2)
>
> 411
>
>   print('**** Warning! This is going to kill all tasks in this job!')
>
>   412
>
>   print('**** Tasks will be killed in 10 seconds. Press ^C to abort!')
>
>   413
>
>   time.sleep(10)
>
>   I'm not a fan of embedding this sleep behavior either - this can be 
> accomplished with hooks. For my use case of quick iteration this adds 
> slowness for no apparent reason.
>
>
>    
> src/main/python/apache/aurora/client/commands/core.py<https://reviews.apache.org/r/19159/diff/2/?file=518936#file518936line415>
>  (Diff
> revision 2)
>
> 413
>
>   time.sleep(10)
>
>   You use this value in your test - can you extract a constant?
>
>
> - Kevin Sweeney
>
> On March 13th, 2014, 1:05 p.m. PDT, Mark Chu-Carroll wrote:
>   Review request for Aurora, Kevin Sweeney and Bill Farner.
> By Mark Chu-Carroll.
>
> *Updated March 13, 2014, 1:05 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