Hi, I really like the approach mentioned under '3'. While the SPIs might undergo changes sometime, this is probably too early, especially now that we have promised for its stability from Release 0.90. Thats just about my opinion. Thanks
- Venkat On 7/2/07, Simon Nash <[EMAIL PROTECTED]> wrote:
I'd like to get some other views on this before I spend a lot of time reworking this patch. Please see my comments inline below. Simon ant elder wrote: > On 7/2/07, Simon Nash <[EMAIL PROTECTED]> wrote: > >> >> I am reconsidering the changes made in stage 2 of this patch, which >> added two new methods to the Binding SPI interface. >> >> I made these changes for the reasons stated in >> http://www.mail-archive.com/[email protected]/msg19305.html >> and >> http://www.mail-archive.com/[email protected]/msg19308.html >> These methods allow bindings to be marked as either callback or >> forward bindings. This allows callback binding semantics to be >> supported correctly with relatively little change to the core runtime. >> >> Unfortunately these is a flip side to these benefits. Adding these >> methods changes the current published stable Binding SPI, and >> (probably worse) introduces "pollution" of the Binding SPI to carry >> information that is there for the convenience of the core runtime, >> rather than an intrinsic part of the binding semantics. >> >> The alternative is to keep the Binding SPI as it was, and make more >> extensive changes in the core runtime to use the more correct version >> of the model as proposed in >> http://www.mail-archive.com/[email protected]/msg19305.html >> >> I will look at the impact to the core runtime of the alternative >> approach that keeps the Binding SPI unchanged and I will post my >> findings back to this list. >> >> This discussion only affects the Binding SPI changes in stage 2 of >> the patch. It does not affect the provider SPI changes in stage 1 >> of the patch. >> >> I'd be interested in any comments on the above or on any other aspects >> of this patch. > > > > It sounds good the binding SPI can remain unchanged, it also sounded like > from the other thread that it should be possible to avoid some of the other > SPI changes in "stage 1" as well which would be good. > I'm getting on quite well with reworking the code that currently depends on the changes to the binding SPI. There's one slightly ugly area, but I think I'll be able to get around it without too much hackery. The provider SPI changes can't be avoided, though they can be deferred. In the short term this may seem attractive because it gets something working with less new code. However, the total amount of work will be greater since my patch already contains all the necessary collateral changes for code that's impacted by the provider SPI changes. I would have to do significant rework on the patch to remove these for now, then repeat much of this work to recreate a smilar set of collateral changes when we finally adopt these new SPIs. > How about trying to get as much as this going with as few changes to the > existing SPI as possible for now even if doing that requires some less then > perfect code in the runtime impl and axis2 binding, and then once we have > call backs and async working well with axis2 and ws-addressing and ideally > at least another extension or two then look at what the best way to change > the SPIs might be? > I think there's pretty much a straight trade-off between how many of these SPI changes are deferred and the amount of temporary workaround code that would be needed in the core to compensate for this. Let's take the provider SPI changes one by one, in approximate order of increasing difficulty for working around not having them. 1. Not including the change to remove the redundant isCallback parameter from ReferenceBindingProvider.createInvoker() doesn't cause any significant problems in the core. The core can always pass the meaningless extra parameter, and the providers can always ignore it. This does incur the collateral rework costs that I mentioned above for when we get around to cleaning this one up. 2. Not including the supportsAsyncOneWayInvocation() methods on ReferenceBindingProvider and ServiceBindingProvider presents a bit more of a challenge. One possibility is to create a new marker interface that can be implemented by providers that support asynchronous one-way invocation. The core could test for this marker interface and omit the thread switch if the provider implements it. I'm not really convinced that this is cleaner (it's probably a bit more tricky to explain than just having methods for this as we do everywhere else in the SPI), but it would get around the need for a breaking SPI change now. What do people think about this approach? 3. Not including the createCallbackInvoker() method on ServiceBindingProvider. seems at first sight to make it impossible for callbacks to work. But there is (as always with software) another way. We could introduce a new interface ServiceBindingCallbackProvider that extends ServiceBindingProvider and adds the createCallbackInvoker() method. Providers that support callbacks would implement this interface, and providers that don't support callbacks could continue to implement ServiceBindingProvider. Again, this seems slightly tricky but it would work and it wouldn't need a breaking SPI change. Now I'll circle back to 1 again. The proposal for 3 has inspired me to imagine a new interface that just contains the new method signature for createInvoker(). New providers can implement this interface and the new signature, and old providers can continue with the old signature, with the core being smart enough to understand both forms and call the signature that is available. There's still the issue of collateral rework when we finally straighten this out. OK, I've woken up now :-) Before I launch into any of the above changes or the associated collateral rework activity, I'd like to get other views on how to handle points 1, 2 and 3 above in the patch for TUSCANY-1341. I'm happy to go with the consensus on this. My opinion is that the long term solution is likely to be the one I have currently implemented, so the work to change other affected code will need to be done anyway at some point. This work has been done already in the current patch, so it's more efficient to apply it now than to defer it. It seems reasonable to do these changes now as they won't impact the 0.91 release and we have time to get comfortable with them before 0.92 and adjust them if necessary. > There's a few other unrelated changes we probably need to do in the SPI as > well, maybe we can save them all up and then have a single separate release > specially for all these breaking changes and deprecate things so its really > clear how/what/why things changed. > I understand the reasons for this. The counter-argument is that we would be deferring a lot of work and it will be hard to motivate doing all these cleanup things all at once when there's so much we still need to do on the functionality. My preference would be to do cleanup incrementally as we discover that it's needed. Simon --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
