aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joshua Cohen" <jco...@twitter.com>
Subject Re: Review Request 25255: Implement server-driven update commands.
Date Tue, 02 Sep 2014 17:32:17 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25255/#review52043
-----------------------------------------------------------


Is the command name "supdate" up for debate? I'm not in love with it ;).


src/main/python/apache/aurora/client/cli/jobs.py
<https://reviews.apache.org/r/25255/#comment90770>

    This doesn't read very well to me.
    
    Maybe something like:
    
    "Update action to start an update or pause, resume or abort an in-progress update."



src/main/python/apache/aurora/client/cli/jobs.py
<https://reviews.apache.org/r/25255/#comment90771>

    This general form (get cluster api, call api method, check response, report, exit) is
repeated for each of the update related methods, the only bits changing are the method to
call on the api and the messages that are printed. Consider refactoring to this logic is shared?



src/test/python/apache/aurora/client/cli/test_supdate.py
<https://reviews.apache.org/r/25255/#comment90773>

    Similarly to the impl above, there's a lot of commonality between these success test cases
that could be refactored to reduce boilerplate.


- Joshua Cohen


On Sept. 2, 2014, 4:36 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25255/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2014, 4:36 p.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/context.py 51c7d24dca664e476e62f1864d095416dfab70e4

>   src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080

>   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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message