You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Mark Chu-Carroll <mc...@twopensource.com> on 2014/09/09 16:07:03 UTC

Re: Review Request 22457: Improve aurora "job diff" command.

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

(Updated Sept. 9, 2014, 10:07 a.m.)


Review request for Aurora, Maxim Khutornenko and Brian Wickman.


Changes
-------

Tried modifying the test, to see if that fixes the jenkins build issue.


Bugs: aurora-520
    https://issues.apache.org/jira/browse/aurora-520


Repository: aurora


Description (updated)
-------

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 (updated)
-----

  src/main/python/apache/aurora/client/cli/BUILD ebe681a0d1735b7cc695dc3b7a14c4292d87ae32 
  src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
  src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
  src/test/python/apache/aurora/client/cli/BUILD e1f9ebf96774b8f5c75de8570c6ba87d953ab649 
  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


Re: Review Request 22457: Improve aurora "job diff" command.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22457/#review58220
-----------------------------------------------------------


This patch does not apply cleanly on master (53f4e73), do you need to rebase?

- Aurora ReviewBot


On Sept. 9, 2014, 2:07 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22457/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2014, 2:07 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko 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 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
>   src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/BUILD e1f9ebf96774b8f5c75de8570c6ba87d953ab649 
>   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
> 
>


Re: Review Request 22457: Improve aurora "job diff" command.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22457/#review58294
-----------------------------------------------------------


Master (53f4e73) is red with this patch.
  ./build-support/jenkins/build.sh

src.test.python.apache.aurora.client.api.api                                    .....   SUCCESS
src.test.python.apache.aurora.client.api.disambiguator                          .....   SUCCESS
src.test.python.apache.aurora.client.api.instance_watcher                       .....   SUCCESS
src.test.python.apache.aurora.client.api.job_monitor                            .....   SUCCESS
src.test.python.apache.aurora.client.api.mux                                    .....   SUCCESS
src.test.python.apache.aurora.client.api.quota_check                            .....   SUCCESS
src.test.python.apache.aurora.client.api.restarter                              .....   SUCCESS
src.test.python.apache.aurora.client.api.scheduler_client                       .....   SUCCESS
src.test.python.apache.aurora.client.api.sla                                    .....   SUCCESS
src.test.python.apache.aurora.client.api.updater                                .....   SUCCESS
src.test.python.apache.aurora.client.api.updater_util                           .....   SUCCESS
src.test.python.apache.aurora.client.binding_helper                             .....   SUCCESS
src.test.python.apache.aurora.client.cli.api                                    .....   SUCCESS
src.test.python.apache.aurora.client.cli.bridge                                 .....   SUCCESS
src.test.python.apache.aurora.client.cli.command_hooks                          .....   SUCCESS
src.test.python.apache.aurora.client.cli.cron                                   .....   SUCCESS
src.test.python.apache.aurora.client.cli.help                                   .....   SUCCESS
src.test.python.apache.aurora.client.cli.inspect                                .....   SUCCESS
src.test.python.apache.aurora.client.cli.job                                    .....   FAILURE
src.test.python.apache.aurora.client.config                                     .....   SUCCESS

- Aurora ReviewBot


On Oct. 24, 2014, 2:50 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22457/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 2:50 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko 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 995570325bbb09ecbcc2ace5d223760c5d49367f 
>   src/main/python/apache/aurora/client/cli/jobs.py 625cb80a33ae565b403fc71bb9795e4700e1aeb7 
>   src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/BUILD 4692d31a9c128664273f71d15ee217dc060e66f0 
>   src/test/python/apache/aurora/client/cli/test_diff.py 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 
>   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
> 
>


Re: Review Request 22457: Improve aurora "job diff" command.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.

> On Oct. 24, 2014, 11:58 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/cli/jobs.py, line 311
> > <https://reviews.apache.org/r/22457/diff/10/?file=732192#file732192line311>
> >
> >     This is no longer true. The `populated` field reprsents a single TaskConfig now. You'd need to inflate a task config list according to the local instance count.
> 
> Mark Chu-Carroll wrote:
>     So you changed the semantics of the API call without changing its type signature to reflect that? Really?
> 
> Maxim Khutornenko wrote:
>     We can't change type signatures without breaking backward compatibility. This deprecation was a long overdue change as there is no reason to return a set of completely identical structs over the wire now that all TaskConfigs are identical.

Backward compatibility is only backward compatibility if existing code is unbroken by a change. That is *not* the case here. This is a breaking change - it will break the comparisons of any client code that isn't explicitly updated to reflect the change. Semantic changes break backward compatibility - as this change shows!

The right way to handle something like this is to be explicit about the deprecation, not to sloppily overload an existing structure so that the change will slip by unnoticed because you kept the type signature, but changed the semantics.


- Mark


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


On Oct. 24, 2014, 10:50 a.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22457/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 10:50 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko 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 995570325bbb09ecbcc2ace5d223760c5d49367f 
>   src/main/python/apache/aurora/client/cli/jobs.py 625cb80a33ae565b403fc71bb9795e4700e1aeb7 
>   src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/BUILD 4692d31a9c128664273f71d15ee217dc060e66f0 
>   src/test/python/apache/aurora/client/cli/test_diff.py 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 
>   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
> 
>


Re: Review Request 22457: Improve aurora "job diff" command.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Oct. 24, 2014, 3:58 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/cli/jobs.py, line 311
> > <https://reviews.apache.org/r/22457/diff/10/?file=732192#file732192line311>
> >
> >     This is no longer true. The `populated` field reprsents a single TaskConfig now. You'd need to inflate a task config list according to the local instance count.
> 
> Mark Chu-Carroll wrote:
>     So you changed the semantics of the API call without changing its type signature to reflect that? Really?

We can't change type signatures without breaking backward compatibility. This deprecation was a long overdue change as there is no reason to return a set of completely identical structs over the wire now that all TaskConfigs are identical.


- Maxim


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


On Oct. 24, 2014, 2:50 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22457/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 2:50 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko 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 995570325bbb09ecbcc2ace5d223760c5d49367f 
>   src/main/python/apache/aurora/client/cli/jobs.py 625cb80a33ae565b403fc71bb9795e4700e1aeb7 
>   src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/BUILD 4692d31a9c128664273f71d15ee217dc060e66f0 
>   src/test/python/apache/aurora/client/cli/test_diff.py 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 
>   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
> 
>


Re: Review Request 22457: Improve aurora "job diff" command.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.

> On Oct. 24, 2014, 11:58 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/cli/jobs.py, line 311
> > <https://reviews.apache.org/r/22457/diff/10/?file=732192#file732192line311>
> >
> >     This is no longer true. The `populated` field reprsents a single TaskConfig now. You'd need to inflate a task config list according to the local instance count.

So you changed the semantics of the API call without changing its type signature to reflect that? Really?


- Mark


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


On Oct. 24, 2014, 10:50 a.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22457/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 10:50 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko 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 995570325bbb09ecbcc2ace5d223760c5d49367f 
>   src/main/python/apache/aurora/client/cli/jobs.py 625cb80a33ae565b403fc71bb9795e4700e1aeb7 
>   src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/BUILD 4692d31a9c128664273f71d15ee217dc060e66f0 
>   src/test/python/apache/aurora/client/cli/test_diff.py 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 
>   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
> 
>


Re: Review Request 22457: Improve aurora "job diff" command.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Oct. 24, 2014, 3:58 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/cli/jobs.py, line 311
> > <https://reviews.apache.org/r/22457/diff/10/?file=732192#file732192line311>
> >
> >     This is no longer true. The `populated` field reprsents a single TaskConfig now. You'd need to inflate a task config list according to the local instance count.
> 
> Mark Chu-Carroll wrote:
>     So you changed the semantics of the API call without changing its type signature to reflect that? Really?
> 
> Maxim Khutornenko wrote:
>     We can't change type signatures without breaking backward compatibility. This deprecation was a long overdue change as there is no reason to return a set of completely identical structs over the wire now that all TaskConfigs are identical.
> 
> Mark Chu-Carroll wrote:
>     Backward compatibility is only backward compatibility if existing code is unbroken by a change. That is *not* the case here. This is a breaking change - it will break the comparisons of any client code that isn't explicitly updated to reflect the change. Semantic changes break backward compatibility - as this change shows!
>     
>     The right way to handle something like this is to be explicit about the deprecation, not to sloppily overload an existing structure so that the change will slip by unnoticed because you kept the type signature, but changed the semantics.

Ah, I see what the problem is. I accidentally referred to "populated" in my original comment by what I really meant was "taskConfig". Sorry about that.

You have a choice of using the new "taskConfig" field and expanding the result to the number of instances as I suggested or continue using the populatedDEPRECATED field and keep the current behavior. Given that this is a deep refactoring, I suggest the former.


- Maxim


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


On Oct. 24, 2014, 2:50 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22457/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 2:50 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko 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 995570325bbb09ecbcc2ace5d223760c5d49367f 
>   src/main/python/apache/aurora/client/cli/jobs.py 625cb80a33ae565b403fc71bb9795e4700e1aeb7 
>   src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/BUILD 4692d31a9c128664273f71d15ee217dc060e66f0 
>   src/test/python/apache/aurora/client/cli/test_diff.py 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 
>   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
> 
>


Re: Review Request 22457: Improve aurora "job diff" command.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22457/#review58280
-----------------------------------------------------------



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

    This is no longer true. The `populated` field reprsents a single TaskConfig now. You'd need to inflate a task config list according to the local instance count.



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

    Please, refactor the correspondent method in util.py to give you a single scheduled task. There is nothing unique here that cannot be reused.



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

    A ScheduledTask does not have a `key` field. Please, avoid using dynamic properties as they are very confusing in test.



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

    These are not mocks anymore, please update method name.



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

    same here



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

    Missing `job` field as a future replacment to owner/environment/jobName.



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

    Is this still needed? If so, would be great to have a bit of formatting around it to really help debugging when needed.


- Maxim Khutornenko


On Oct. 24, 2014, 2:50 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22457/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 2:50 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko 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 995570325bbb09ecbcc2ace5d223760c5d49367f 
>   src/main/python/apache/aurora/client/cli/jobs.py 625cb80a33ae565b403fc71bb9795e4700e1aeb7 
>   src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/BUILD 4692d31a9c128664273f71d15ee217dc060e66f0 
>   src/test/python/apache/aurora/client/cli/test_diff.py 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 
>   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
> 
>


Re: Review Request 22457: Improve aurora "job diff" command.

Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22457/
-----------------------------------------------------------

(Updated Oct. 24, 2014, 10:50 a.m.)


Review request for Aurora, Maxim Khutornenko and Brian Wickman.


Changes
-------

I think I finally found the problem that was causing the jenkins failures. Since this rebase was a bit messy, can reviewers please take another look before I try pushing it?


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 (updated)
-----

  src/main/python/apache/aurora/client/cli/BUILD 995570325bbb09ecbcc2ace5d223760c5d49367f 
  src/main/python/apache/aurora/client/cli/jobs.py 625cb80a33ae565b403fc71bb9795e4700e1aeb7 
  src/main/python/apache/aurora/client/cli/json_tree_diff.py PRE-CREATION 
  src/test/python/apache/aurora/client/cli/BUILD 4692d31a9c128664273f71d15ee217dc060e66f0 
  src/test/python/apache/aurora/client/cli/test_diff.py 78694d7559f2041f27cd2a7e4cb81ca467f63ac2 
  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