aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brian Wickman" <wick...@apache.org>
Subject Re: Review Request 22457: Improve aurora "job diff" command.
Date Mon, 16 Jun 2014 20:20:16 GMT

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



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

    indentation on this comment should be dedented 1



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

    given that DIFF_VIEWER is user-supplied, should we be performing this check?



src/main/python/apache/aurora/client/cli/json_tree_diff.py
<https://reviews.apache.org/r/22457/#comment80804>

    from twitter.common.lang import Compatibility and use Compatibility.string
    
    StringTypes doesn't exist in python 3.x:
    
    mba=~=; python3.3
    Python 3.3.3 (default, Apr 15 2014, 11:17:36) 
    [GCC 4.2.1 Compatible Apple Clang 4.0 ((tags/Apple/clang-421.0.60))] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> from types import StringTypes
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ImportError: cannot import name StringTypes
    >>> 
    



src/main/python/apache/aurora/client/cli/json_tree_diff.py
<https://reviews.apache.org/r/22457/#comment80803>

    isinstance(e, list)



src/main/python/apache/aurora/client/cli/json_tree_diff.py
<https://reviews.apache.org/r/22457/#comment80805>

    isinstance(base_val, dict)
    isinstance(other_val, dict)
    



src/test/python/apache/aurora/client/cli/test_diff.py
<https://reviews.apache.org/r/22457/#comment80802>

    this sort of testing seems fragile as it relies upon the sort order of the python dictionary
hash which is not guaranteed to be the same between VM implementations (or even within minor
versions of VM implementations.)
    
    instead, you'll probably probably need to parse the output rather than checking string
comparisons.


- Brian Wickman


On June 13, 2014, 8:22 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22457/
> -----------------------------------------------------------
> 
> (Updated June 13, 2014, 8:22 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Brian Wickman.
> 
> 
> Bugs: aurora-520
>     https://issues.apache.org/jira/browse/aurora-520
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add a new diff method, which uses field-by-field comparison of JSON trees for comparing
running job configurations to potentially updated configs.
> 
> - Allow exclusion of semantically irrelevant fields.
> - Provide a clearer list of the differences between configs.
> - Provide a scripting-friendly alternative JSON syntax for diffs.
> 
> The old diff behavior is still available under the "--use-shell-diff" option.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/BUILD ebe681a0d1735b7cc695dc3b7a14c4292d87ae32

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

>   src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/BUILD 3c88ed7cf9f654bbbd80d1d44aa1dd1c8655e378

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

>   src/test/python/apache/aurora/client/cli/test_json_diff.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22457/diff/
> 
> 
> Testing
> -------
> 
> New unit tests of the JSON tree diff code, plus a bunch of new "job diff" tests of the
new functionality.
> All tests pass.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


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