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

Reply via email to