brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aled Sage <aled.s...@gmail.com>
Subject Re: Backwards compatibility in advanced-networking: package renames
Date Fri, 20 Feb 2015 20:55:26 GMT
Svet, Sam,

I've tested the backwards compatibility, and it works with your 
sub-classing approach, along with PR#38 which has just been merged [1].

I was wrong about the errors when assigning to deserialized fields etc.
The deserialized objects are instances of the old class name, which is a 
sub-type of the new class. These objects are thus usable everywhere - as 
return values, as parameters etc.

Longer term, we do need a transformer [2,3] so that we can delete the 
old classes.

Aled

[1] https://github.com/brooklyncentral/advanced-networking/pull/38
[2] 
https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/CompoundTransformerLoaderTest.java
[3] 
https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/impl/XsltTransformerTest.java#L67-95


On 06/02/2015 22:17, Aled Sage wrote:
> Hi all,
>
> Svet + Sam have been doing great work on Brooklyn (including 
> advanced-networking and clocker) to make it work better with OSGi [1].
>
> This includes ensuring that packages have different names in different 
> OSGi bundles (very important to know if you're thinking about package 
> names in the future!).
>
> In downstream projects that depend on advanced-networking, you may see 
> compilation errors when the import is for PortForwarder in the old 
> package (e.g. brooklyn.networking.subnet.PortForwarder instead of 
> brooklyn.networking.common.subnet.PortForwarder).
>
> ---
> Svet, Sam,
>
> We should discuss backwards compatibility some more.
>
> You've renamed the classes, and created sub-classes with the old names 
> - the aim being to support deserialization of previously persisted state.
>
> However, various fields and method parameters now expect the new 
> class. Therefore the deserialized old class will likely not be usable 
> (e.g. errors when trying to assign the deserialized value to a field, 
> or ClassCastExceptions later).
>
> That's my opinion; I haven't actually tried to produce the error from 
> previously-serialized state.
> So first, do you agree?
>
> I see three options for downstream projects (where PortForwarder 
> instances are persisted):
>
> 1. Write a transformer that will upgrade the persisted state, switching
>    the old package for the new package.
>    (That is what the transformer stuff is designed for [2,3]).
> 2. Be very careful about the types of fields, parameters and return
>    types so that the old class is likely to work.
>    (sounds fiddly to get right; and without doing (1) we are left with
>    that code until the old entities die a natural death!).
> 3. Tell them it's not backwards compatible.
>    (That's a last resort, and is unacceptable for anyone running in
>    production.)
>
> Of these, I favour (1).
>
> If you agree, then we need to delete the shims (i.e. the classes with 
> the old names) and write a transformer. And write better docs 
> describing how to use the transformer.
>
> Thoughts?
>
> Aled
>
> [1] https://github.com/brooklyncentral/advanced-networking/pull/21
> [2] 
> https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/CompoundTransformerLoaderTest.java
> [3] 
> https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/impl/XsltTransformerTest.java#L67-95
>
>


Mime
View raw message