On 05/03/18 16:52, Robert Varga wrote: > Hello everyone, Hello again,
as per today's TSC call, with regards to upcoming weather items from MD-SAL, here is an update to the plan: > we are still ways off of having a smooth transition plan from Binding V1 > to Binding V2 and a large body of models are getting standardized, some > of which trigger V1 deficiencies. > > Contingent on favorable outcome of the discussion here: > https://lists.opendaylight.org/pipermail/mdsal-dev/2018-March/001517.html, This was presented and discussed at a TSC call, with overall decision to move forward and has been fully executed. > it would be possible to fix majority of these with very few changes, > which will break pretty much everyone out there, but is ways that are: > - obvious compilation breakages > - can be fixed mechanically > > Specifically I mean the following changes (warning, technical details > ahead): > > 1) Map identity statements to interfaces, not abstract classes This item has been completed the weekend after ONS. > > RFC6020 specified single inheritance for identities, which meant mapping > them to abstract classes worked as expected. RFC7950 changed the > underlying meta model and allowed identities to have multiple base > statements -- leading to multiple inheritance. Java does not support > multiple inheritance of state, hence we currently cannot represent this > YANG 1.1 construct in generated code. > > The change from abstract class to interface is fully source-compatible > for all use cases that are supported. Invalid uses (like subclassing a > generated identity) will break and will require a mechanical source code > update. > > > 2) Replace org.opendaylight.yangtools.yang.binding.Identifiable.getKey() This is https://jira.opendaylight.org/browse/TSC-101, i.e. this is not immediately pending, until there is will to combine it with TSC-99, below. It is expected to be more invasive than TSC-99 in downstreams. > > This is the most major pain in models, as the name chosen clashes with > the namespace used by generated code for getters. This means any > construct like: > > list foo { > key ...; > leaf key { ... } > } > > leads either to a codegen failure on ClassCastException or results in > code which cannot be compiled. > > The solution is to rename getKey() to key(). As all user-governed names > in generated code is prefixed (with "is", "get" and similar), this will > permanently solve this particular problem. Unfortunately it will also > break all code which deals with getting/putting elements into keyed lists. This also applies to getAugmentation(), where the change is from 'getAugmentation()' to 'augmentation()' (for lack of imagination). An example of downstream impact can be seen in BGPCEP, which is heavily modularized using augmentations, in the unfinished patch: https://git.opendaylight.org/gerrit/#/c/71259/. > The fix is a mechanical one and probably scriptable (but I am not much > for writing scripts). > > > 3) Change the signature of generated RPC invocation methods This is https://jira.opendaylight.org/browse/TSC-99, which we would like to merge soon. The exact date is to be discussed at the TSC call on May 3rd. > This change has two facets, the first of which is convenience to callers > of RPCs: these are currently defined as returning a > java.util.concurrent.Future. The MD-SAL core implementation and all RPC > implementations are using subclasses of ListenableFuture while > asynchronous end users tend to JdkFutureAdapters or straightout casts to > change the Future into a ListenableFuture. > > The other facet is the request and response type when the RPC > declaration in YANG does not have an input or output statement: > > rpc foo { }; > rpc bar { input { } }; > rpc bar { output { } }; > > Such RPCs are mapped to either not having an input argument or returning > a Future<Void>. This is a design mistake, as YANG specification calls > for input/output statements to be implicitly present -- they are valid > targets for "augment" statement irrespective of whether they were > explicitly declared. While we can compile such models, all of the > information in such augmentations is inaccessible to Binding users, as > there is no anchor representation class generated for input/output > statements. > > I propose this to be fixed three changes: > a) always generate Input/Output statements for RPCs > b) always take an Input as the argument of generated method > c) always return ListenableFuture<Output> from generated method After going through the implementation phase, delivery in these three steps would not be feasible due to internal structure of codegen :( Hence the fix is structured as Input+Output and ListenableFuture parts. > The first one is completely compatible, we just end up generating > otherwise useless classes. > > The second one will break both callers and implementations of RPCs which > do not have an input statement in the model. The fix is a mechanical > one, it is fair to send an empty Input and ignore the result. Current > trouble spots can be fixed ahead of time by explicitly changing the > model, which will incur exactly same amount of breakage. > > The third one will break implementations which do not specify > ListenableFuture as their overridden return type or which do not return > a subclass of ListenableFuture. The fix is largely a mechanical one. > > > These changes would be delivered as three separate code drops, the order > being 1, 2, 3. The last code drop would happen before the mid-way > checkpoint in the Fluorine release cycle. The breakage is quite simple for most of the projects. The fixes can be staged: When an RPC already has both inputs and outputs by declaring the method to return ListenableFuture, which means that piece of code will not be affected by the mdsal code drop. This is demonstrated in bgpcep patch: https://git.opendaylight.org/gerrit/#/c/71228/. The fix for RPCs which previously lacked an input or an output is rather simple, for example: https://git.opendaylight.org/gerrit/#/c/71227/4/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BgpPeerRpc.java boils down to three things: - adjusting method declaration - adjusting RpcResult type -- for successes, return an empty Output object, this object can safely be cached as a constant -- for failures, just adjust the parameter from Void to FooOutput https://git.opendaylight.org/gerrit/#/c/71227/4/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/TopologyRPCs.java does the same, but the call site has previously been prepared :) Regards, Robert
signature.asc
Description: OpenPGP digital signature
_______________________________________________ controller-dev mailing list controller-dev@lists.opendaylight.org https://lists.opendaylight.org/mailman/listinfo/controller-dev
