On Thu, Aug 18, 2011 at 2:07 PM, Christian Schneider <ch...@die-schneider.net> wrote: > Am 18.08.2011 13:50, schrieb Claus Ibsen: >> >> On Thu, Aug 18, 2011 at 1:25 PM, Christian Schneider >> <ch...@die-schneider.net> wrote: >>> >>> Hi Claus, >>> >>> I just saw that you also changed the ThreadPoolFactory interface to not >>> use >>> the profiles and instead have several methods again. This completely >>> defeats >>> the purpose of having a small factory interface. >>> I will revert that back. >>> >> The ThreadPoolFactory has an API that is fully non Camel specific. >> There is no Camel API at all in that interface. >> >> It relies purely on well known JDK terms for thread pools and thus >> makes it much easier for 3rd party to implement a custom factory if >> they need. >> >> All 4 methods are similar to the JDK Executors API and therefore easy >> for people to understand and implement. >> >> Your API was a Camel thingy with the ThreadPoolProfile (API from >> Camel) and thus people would have to drag Camel API in their custom >> implementations. Likewise when people needed a cached thread pool, >> then the ThreadPoolProfile would not be able to indicate that. >> >> I would oppose against putting Camel's API into the SPI when its not >> necessary. > > The class ThreadPoolProfile is not much more than a c struct to combine the > attributes of a thread pool into a class. > The implementor of a camel spi always has a dependency to the camel API as > the spi interface is part of the API. So I see no real problem in having > this in the interface. > Most spi interfaces reference other camel api classes and many of those > classes are much more problematic. The ThreadPoolProfile is self contained > so I see absolutely no problem in having it there. > The similarity to the jdk is important for the ExecutorServiceManager as > very many people use this. The spi factory will only be used very seldomly. > So I don´t think the similarity is very important. > In fact it rather hurts as the user has to implement more methods. >>
That is a fair argument. I will be okay with that. Also considering ThreadPoolFactory is a new API and thus no backwards issue. Fell free to change it to your needs. Btw even if user have to implement more methods, they are more precise what they are asking for: fixed vs cached etc. Also people can often delegate to another method with defaults. So the implementation is easy. See for example the JDK source code in the Executors which is the JDK factory for thread pools. It have many methods and there is a lot of delegates. >> >>> I worked on these changes quite a long time so the fact that you simply >>> changed things back after we discussed on them and agreed on the changes >>> makes me a bit sad. It also causes me a lot of unplanned work now. I >>> agree >>> with you on some of the things you mentioned. >>> Like for example that it makes sense to offer an API on the >>> ExecutorServiceManager that people are familiar with. So I think using >>> the >>> almost same API as in java is a good thing. I also like the fact that the >>> change on the components is now really small after your change and also >>> that >>> there is a completely compatible ExecutorServiceStrategy as a fallback. >>> That >>> really makes sense for all external components. >>> >>> The problem is that with your changes you also rolled back good things I >>> did >>> that I now have to spend a lot of time bringing in again. >>> >> The trunk is not an experimental branch where people can commit big >> refactorings or changes that break backwards compatible. >> Likewise there is nobody in the community that requires a change in >> the ExecutorServiceStrategy API. This is something you >> came up with because you wanted to adjust the API. This is sometimes >> tempting to do, but is very dangerous. The API should be kept >> stable, especially for Camel 2.x that is 2+ years old, and for a >> feature that has been in there for 18+ months. > > Your arguments are true and I have no problems with the fact that you want > the camel 2.x branch to be quite compatible. The problem I had was rather > with the way you did this. You changed the code and then asked for a heads > up. For my changes I first offered a patch and we discussed it. >> >> >> We have Camel 3.0 where the API can be more open for changes. > > The problem with incompatible changes in major versions only is that they > accumulate and make the major version very fragile and makes the development > on that version take a very long time. So I think we should try to do as > much in minor versions as we can and accept some minor incompatiblities. Of > course it is very diffcult to judge what is minor. > > > Christian > > > -- > -- > Christian Schneider > http://www.liquid-reality.de > > Open Source Architect > Talend Application Integration Division http://www.talend.com > > -- Claus Ibsen ----------------- FuseSource Email: cib...@fusesource.com Web: http://fusesource.com Twitter: davsclaus, fusenews Blog: http://davsclaus.blogspot.com/ Author of Camel in Action: http://www.manning.com/ibsen/
