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 Mon, 23 Feb 2015 10:31:36 GMT
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 <aled.sage@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
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message