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