Andrey, Can you list all setters that we have on MBeans?
-Val On Mon, Feb 6, 2017 at 2:21 PM, Andrey Mashenkov <andrey.mashen...@gmail.com > wrote: > Folks, > > Changing MBeans setters signature is bad idea. AOP tests failed on TC with > this change. > > On Mon, Feb 6, 2017 at 11:21 AM, Vladimir Ozerov <voze...@gridgain.com> > wrote: > > > Val, > > > > Good catch! Can we try leaving SPIs and base methods untouched? Will it > API > > be consistent in this case? > > > > On Fri, Feb 3, 2017 at 10:22 PM, Valentin Kulichenko < > > valentin.kuliche...@gmail.com> wrote: > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > -- > С уважением, > Машенков Андрей Владимирович > Тел. +7-921-932-61-82 > > Best regards, > Andrey V. Mashenkov > Cerr: +7-921-932-61-82 >
