Folks, I tend to think that the problem is that we try to apply 'builder approach' to *ALL* setters. Let's approach this smarter.
This approach is actually applicable only for configuration setters available on public API, i.e. it's only about setters on ***Configuration classes and SPI *implementations*. For SPI interface methods like 'CollisionSpi.setExternalCollisionListener' this makes no sense, I would not touch them. The only thing I still don't like is MBeans. Returning something except void on MBean interfaces look ugly, but without doing this we will break API consistency on the implementation. Any ideas on how to approach this? -Val On Fri, Feb 3, 2017 at 10:29 AM, Denis Magda <dma...@apache.org> wrote: > Sorry, “public modifications” -> “public APIs” > > — > Denis > > > On Feb 3, 2017, at 10:03 AM, Denis Magda <dma...@apache.org> wrote: > > > > Andrey, > > > > If the changes affect public modifications don’t forget to update this > page: > > https://cwiki.apache.org/confluence/display/IGNITE/ > Apache+Ignite+2.0+Migration+Guide <https://cwiki.apache.org/ > confluence/display/IGNITE/Apache+Ignite+2.0+Migration+Guide> > > > > — > > Denis > > > >> On Feb 3, 2017, at 12:24 AM, Andrey Mashenkov < > andrey.mashen...@gmail.com> wrote: > >> > >> Vladimir, > >> > >> Ok. I'll go ahead with changing SPI interfaces and run TC test. > >> I think, it would be better to have this branch merged to master as 2 > >> separate commits at least. > >> And may be we should make changes of SPI interfaces in separate Jira > >> ticket? > >> > >> On Fri, Feb 3, 2017 at 11:08 AM, Vladimir Ozerov <voze...@gridgain.com> > >> wrote: > >> > >>> Andrey, > >>> > >>> This is very important change from usability standpoint. But my main > >>> concern is changes to SPI *interfaces*. If we do so users who > implemented > >>> custom SPIs will have broken compatibility. On the other hand, I doubt > >>> there will be too much affected users, and we break compilation in AI > 2.0 > >>> anyway. So looks like we can go ahead with it. > >>> > >>> Thoughts? > >>> > >>> On Fri, Feb 3, 2017 at 7:46 AM, Valentin Kulichenko < > >>> valentin.kuliche...@gmail.com> wrote: > >>> > >>>> My only concern is MBean interfaces. These are not called from code, > but > >>>> from MBean viewers, and I'm not sure setters not returning voids will > be > >>>> properly treated as setters by these viewers. This needs to be > checked. > >>>> > >>>> -Val > >>>> > >>>> On Thu, Feb 2, 2017 at 8:32 PM, Andrey Mashenkov < > >>>> andrey.mashen...@gmail.com > >>>>> wrote: > >>>> > >>>>> Val, > >>>>> > >>>>> Yes, you are right. I don't think we should care about compilation > >>>>> error on user side, as we break compatibility with previous versions. > >>>>> But we talk about public interfaces and may be someone has some cons > >>>>> or suggestions? > >>>>> > >>>>> On Fri, Feb 3, 2017 at 5:31 AM, Valentin Kulichenko < > >>>>> valentin.kuliche...@gmail.com> wrote: > >>>>> > >>>>>> Andrey, > >>>>>> > >>>>>> In which case compatibility is broken? If there is a method that > >>>> returns > >>>>>> void and you change it to return some type, it doesn't break > >>> anything, > >>>>>> because currently nobody can assign the result of this method to a > >>>>>> variable. I.e. in old code the returned value will be always > ignored, > >>>>>> therefore it can be of any type. > >>>>>> > >>>>>> Is there anything else that I'm missing? > >>>>>> > >>>>>> -Val > >>>>>> > >>>>>> On Thu, Feb 2, 2017 at 3:49 AM, Andrey Mashenkov < > >>>>>> andrey.mashen...@gmail.com > >>>>>>> wrote: > >>>>>> > >>>>>>> Hi Igniters, > >>>>>>> > >>>>>>> > >>>>>>> I'm working on IGNITE-4564 [1] to make our configuration classes > >>> and > >>>>> SPI > >>>>>>> classes more convenient. > >>>>>>> > >>>>>>> There is no problem to change return type in setter method > >>> signatures > >>>>>>> and override methods in child child classes to make them return > >>> more > >>>>>>> accurate type. > >>>>>>> > >>>>>>> But, I found that we have set methods in some of our interfaces and > >>>>>>> changing its signature may broke compatibility with user > >>>>> implementations. > >>>>>>> > >>>>>>> Here are example interfaces with setters: > >>>>>>> org.apache.ignite.cache.eviction.fifo.FifoEvictionPolicyMBean > >>>>>>> org.apache.ignite.cache.eviction.igfs. > >>> IgfsPerBlockLruEvictionPolicyM > >>>>>> XBean > >>>>>>> org.apache.ignite.cache.eviction.lru.LruEvictionPolicyMBean > >>>>>>> org.apache.ignite.cache.eviction.sorted.SortedEvictionPolicyMBean > >>>>>>> org.apache.ignite.spi.checkpoint.CheckpointSpi > >>>>>>> org.apache.ignite.spi.collision.CollisionSpi > >>>>>>> org.apache.ignite.spi.collision.fifoqueue. > >>> FifoQueueCollisionSpiMBean > >>>>>>> > >>>>>>> However we have interfaces with NO setters > >>>>>>> org.apache.ignite.spi.loadbalancing.adaptive. > >>>>>>> AdaptiveLoadBalancingSpiMBean. > >>>>>>> > >>>>>>> What can we do with it? > >>>>>>> Change signature of setters without regarding compatibility? Or may > >>>> be > >>>>> it > >>>>>>> is possible to remove setters from some interfaces? > >>>>>>> > >>>>>>> Thought? > >>>>>>> > >>>>>>> > >>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-4564 > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> С уважением, > >>>>> Машенков Андрей Владимирович > >>>>> Тел. +7-921-932-61-82 > >>>>> > >>>>> Best regards, > >>>>> Andrey V. Mashenkov > >>>>> Cerr: +7-921-932-61-82 > >>>>> > >>>> > >>> > >> > >> > >> > >> -- > >> С уважением, > >> Машенков Андрей Владимирович > >> Тел. +7-921-932-61-82 > >> > >> Best regards, > >> Andrey V. Mashenkov > >> Cerr: +7-921-932-61-82 > > > >
