You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by Aled Sage <al...@gmail.com> on 2015/02/06 23:17:44 UTC
Backwards compatibility in advanced-networking: package renames
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
Re: Backwards compatibility in advanced-networking: package renames
Posted by Svetoslav Neykov <sv...@cloudsoftcorp.com>.
Aled,
I've updated the transformers at https://github.com/apache/incubator-brooklyn/pull/528 <https://github.com/apache/incubator-brooklyn/pull/528>. The transform to be used now is:
- renameClass:
old_val: brooklyn.networking.subnet.PortForwarder
new_val: brooklyn.networking.common.subnet.PortForwarder
- renameClass:
old_val: brooklyn.networking.subnet.PortForwarderAsync
new_val: brooklyn.networking.common.subnet.PortForwarderAsync
- renameClass:
old_val: brooklyn.networking.subnet.PortForwarderAsyncImpl
new_val: brooklyn.networking.common.subnet.PortForwarderAsyncImpl
- renameClass:
old_val: brooklyn.networking.subnet.PortForwarderAsyncImpl$1
new_val: brooklyn.networking.common.subnet.PortForwarderAsyncImpl$1
- renameClass:
old_val: brooklyn.networking.subnet.PortForwarderAsyncImpl$DeferredExecutor
new_val: brooklyn.networking.common.subnet.PortForwarderAsyncImpl$DeferredExecutor
- renameClass:
old_val: brooklyn.networking.subnet.PortForwarderClient
new_val: brooklyn.networking.common.subnet.PortForwarderClient
- renameClass:
old_val: brooklyn.networking.subnet.PortForwarderClient$1
new_val: brooklyn.networking.common.subnet.PortForwarderClient$1
- renameClass:
old_val: brooklyn.networking.subnet.PortForwarderClient$2
new_val: brooklyn.networking.common.subnet.PortForwarderClient$2
- renameClass:
old_val: brooklyn.networking.subnet.PortForwarderClient$3
new_val: brooklyn.networking.common.subnet.PortForwarderClient$3
Svet.
> On 23.02.2015 г., at 12:31, Aled Sage <al...@gmail.com> wrote:
>
> Svet,
>
> I had a very quick go previously at creating the transformers. They didn't behave as I expected though.
>
> It seems that:
>
> * only the last transform in the file was applied (is it perhaps using
> a YAML map rather than a list?).
> * it didn't transform things like <pf
> class="brooklyn.networking.subnet.PortForwarderClient">
> Presumably we also need a similar "renameType: ..." in the transforms?
>
> Over to you!
>
> ---
> I tried the following transform:
>
> renameClass:
> old_val: brooklyn.networking.subnet.PortForwarder
> new_val: brooklyn.networking.common.subnet.PortForwarder
> renameClass:
> old_val: brooklyn.networking.subnet.PortForwarderAsync
> new_val: brooklyn.networking.common.subnet.PortForwarderAsync
> renameClass:
> old_val: brooklyn.networking.subnet.PortForwarderAsyncImpl
> new_val: brooklyn.networking.common.subnet.PortForwarderAsyncImpl
> renameClass:
> old_val: brooklyn.networking.subnet.PortForwarderAsyncImpl$1
> new_val: brooklyn.networking.common.subnet.PortForwarderAsyncImpl$1
> renameClass:
> old_val:
> brooklyn.networking.subnet.PortForwarderAsyncImpl$DeferredExecutor
> new_val:
> brooklyn.networking.common.subnet.PortForwarderAsyncImpl$DeferredExecutor
> renameClass:
> old_val: brooklyn.networking.subnet.PortForwarderClient
> new_val: brooklyn.networking.common.subnet.PortForwarderClient
> renameClass:
> old_val: brooklyn.networking.subnet.PortForwarderClient$1
> new_val: brooklyn.networking.common.subnet.PortForwarderClient$1
> renameClass:
> old_val: brooklyn.networking.subnet.PortForwarderClient$2
> new_val: brooklyn.networking.common.subnet.PortForwarderClient$2
> renameClass:
> old_val: brooklyn.networking.subnet.PortForwarderClient$3
> new_val: brooklyn.networking.common.subnet.PortForwarderClient$3
>
> With the following command:
>
> copy-state --persistenceDir /path/to/old-data --destinationDir /path/to/transformed-data --transformations /path/to/transforms.yaml
>
> Aled
>
> p.s. I'll send you the a private message with the state that I'm testing this against.
>
>
> On 23/02/2015 10:18, Svetoslav Neykov wrote:
>> Thanks for checking this Aled, I'll create the transformers.
>>
>> Svet.
>>
>>
>>> On 20.02.2015 г., at 22:55, Aled Sage <al...@gmail.com> wrote:
>>>
>>> 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 <https://github.com/brooklyncentral/advanced-networking/pull/38> <https://github.com/brooklyncentral/advanced-networking/pull/38 <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 <https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/CompoundTransformerLoaderTest.java><https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/CompoundTransformerLoaderTest.java <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 <https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/impl/XsltTransformerTest.java#L67-95> <https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/impl/XsltTransformerTest.java#L67-95 <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
Re: Backwards compatibility in advanced-networking: package renames
Posted by Aled Sage <al...@gmail.com>.
Svet,
I had a very quick go previously at creating the transformers. They
didn't behave as I expected though.
It seems that:
* only the last transform in the file was applied (is it perhaps using
a YAML map rather than a list?).
* it didn't transform things like <pf
class="brooklyn.networking.subnet.PortForwarderClient">
Presumably we also need a similar "renameType: ..." in the transforms?
Over to you!
---
I tried the following transform:
renameClass:
old_val: brooklyn.networking.subnet.PortForwarder
new_val: brooklyn.networking.common.subnet.PortForwarder
renameClass:
old_val: brooklyn.networking.subnet.PortForwarderAsync
new_val: brooklyn.networking.common.subnet.PortForwarderAsync
renameClass:
old_val: brooklyn.networking.subnet.PortForwarderAsyncImpl
new_val: brooklyn.networking.common.subnet.PortForwarderAsyncImpl
renameClass:
old_val: brooklyn.networking.subnet.PortForwarderAsyncImpl$1
new_val: brooklyn.networking.common.subnet.PortForwarderAsyncImpl$1
renameClass:
old_val:
brooklyn.networking.subnet.PortForwarderAsyncImpl$DeferredExecutor
new_val:
brooklyn.networking.common.subnet.PortForwarderAsyncImpl$DeferredExecutor
renameClass:
old_val: brooklyn.networking.subnet.PortForwarderClient
new_val: brooklyn.networking.common.subnet.PortForwarderClient
renameClass:
old_val: brooklyn.networking.subnet.PortForwarderClient$1
new_val: brooklyn.networking.common.subnet.PortForwarderClient$1
renameClass:
old_val: brooklyn.networking.subnet.PortForwarderClient$2
new_val: brooklyn.networking.common.subnet.PortForwarderClient$2
renameClass:
old_val: brooklyn.networking.subnet.PortForwarderClient$3
new_val: brooklyn.networking.common.subnet.PortForwarderClient$3
With the following command:
copy-state --persistenceDir /path/to/old-data --destinationDir
/path/to/transformed-data --transformations /path/to/transforms.yaml
Aled
p.s. I'll send you the a private message with the state that I'm testing
this against.
On 23/02/2015 10:18, Svetoslav Neykov wrote:
> Thanks for checking this Aled, I'll create the transformers.
>
> Svet.
>
>
>> On 20.02.2015 г., at 22:55, Aled Sage <al...@gmail.com> wrote:
>>
>> 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 <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 <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 <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
>
Re: Backwards compatibility in advanced-networking: package renames
Posted by Svetoslav Neykov <sv...@cloudsoftcorp.com>.
Thanks for checking this Aled, I'll create the transformers.
Svet.
> On 20.02.2015 г., at 22:55, Aled Sage <al...@gmail.com> wrote:
>
> 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 <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 <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 <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
Re: Backwards compatibility in advanced-networking: package renames
Posted by Aled Sage <al...@gmail.com>.
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
>
>