aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maxim Khutornenko <ma...@apache.org>
Subject Re: Review Request 47550: AURORA-1492 Improve "aurora update start" command output
Date Mon, 23 May 2016 16:29:18 GMT

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



Looking much better! A few minor suggestions.


src/main/python/apache/aurora/client/cli/jobs.py (lines 171 - 172)
<https://reviews.apache.org/r/47550/#comment199166>

    If the sole purpose of this method is unit test facilitation you can rather use mock patch
instead.



src/main/python/apache/aurora/client/cli/jobs.py (line 212)
<https://reviews.apache.org/r/47550/#comment199167>

    You can import DiffFormatter directly:
    ```
    from apache.aurora.client.cli.diff_formatter import DiffFormatter
    ```



src/main/python/apache/aurora/client/cli/update.py (lines 162 - 163)
<https://reviews.apache.org/r/47550/#comment199169>

    Same as above, use mock patch instead.



src/main/python/apache/aurora/client/cli/update.py (line 177)
<https://reviews.apache.org/r/47550/#comment199168>

    Same as above, import directly.



src/test/python/apache/aurora/client/cli/test_diff.py (line 38)
<https://reviews.apache.org/r/47550/#comment199162>

    Use 'spec=DiffFormatter' for a stronger assertion of mocked functionality.



src/test/python/apache/aurora/client/cli/test_diff_formatter.py (lines 56 - 58)
<https://reviews.apache.org/r/47550/#comment199160>

    You can use it directly from AuroraClientCommandTest. 
    
    Also, to shorten the access points you may want to extend from AuroraClientCommandTest
rather than from unittest.TestCase.



src/test/python/apache/aurora/client/cli/test_diff_formatter.py (lines 60 - 62)
<https://reviews.apache.org/r/47550/#comment199161>

    This is no longer needed if you reference the above from AuroraClientCommandTest.



src/test/python/apache/aurora/client/cli/test_diff_formatter.py (lines 113 - 115)
<https://reviews.apache.org/r/47550/#comment199158>

    This seems to be used in only one place, suggest inlining.



src/test/python/apache/aurora/client/cli/test_diff_formatter.py (line 118)
<https://reviews.apache.org/r/47550/#comment199159>

    Same here



src/test/python/apache/aurora/client/cli/test_supdate.py (line 100)
<https://reviews.apache.org/r/47550/#comment199157>

    Use 'spec=DiffFormatter' for a stronger assertion of mocked functionality.


- Maxim Khutornenko


On May 20, 2016, 10:24 p.m., Mehrdad Nurolahzade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47550/
> -----------------------------------------------------------
> 
> (Updated May 20, 2016, 10:24 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> AURORA-1492 Improve "aurora update start" command output
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/diff_formatter.py PRE-CREATION 
>   src/main/python/apache/aurora/client/cli/jobs.py e8bc38aaff42579419130b116389d9b9e09122a9

>   src/main/python/apache/aurora/client/cli/update.py eb7e9b0799bb25af652a0d5bde231cfca5bc1510

>   src/test/python/apache/aurora/client/cli/test_diff.py b9e91cf3be6ebd6d99e588ba2a2df304e42fd832

>   src/test/python/apache/aurora/client/cli/test_diff_formatter.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 2135ca988ce2f260262145a2168849acd8d14874

> 
> Diff: https://reviews.apache.org/r/47550/diff/
> 
> 
> Testing
> -------
> 
> Automated: ./pants test.pytest --no-fast src/test/python::
> Manual: using vagrant
> e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> Style: ./build-support/hooks/pre-commit
> 
> Example output:
> 
> aurora update start devcluster/www-data/prod/hello ./aurora/examples/jobs/updated_hello_world.aurora

>  INFO] Starting update for: hello
> Job update has started. View your update progress at http://aurora.local:8081/scheduler/www-data/prod/hello/update/df5ba4d7-6a32-42ca-8453-1b172f61a23b
> This job update will:
> add instances: [2-3]
> update instances: [0-1]
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>


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