Hi, #1 and #2 are ok to me but, IMO, the filter package should be in commons module

Cheers, Pedro On 03/13/2014 12:07 PM, William Burns wrote: > Recently while working on some ISPN 7 features, there were some public > API inconsistencies. I wanted to bring these up just in case if > someone had concerns. > > The first few are pretty trivial, but can cause compilation errors > between versions if user code implements these interfaces and defines > types. > > 1. The CacheWriter interface currently defines a delete(K key) method. > To be more inline with JCache and java.util.collections interfaces I > was hoping to change this to be delete(Object key) instead. > 2. The CacheLoader interface currently defines load(K key) and > contains(K key) methods. Similar to above I was hoping to change the > K type to be Object to be more inline with JCache and > java.util.collections interfaces. > > This last one is a bit more major, but currently we have 2 classes > that are named KeyFilter. One that resides in the > org.infinispan.notifications package and another that resides in the > org.infinispan.persistence.spi.AdvancedCacheLoader interface. > > 3. My plan is instead to consolidate these classes into 1 into a new > core org.infinispan.filter package. I would also move the new > KeyValueFilter class that was added for cluster listeners into this > package and their accompanying implementations. > > The first 2 is currently implemented as changes in > https://github.com/infinispan/infinispan/pull/2423. The latter I was > going to add into changes for > https://issues.jboss.org/browse/ISPN-4068. > > Let me know what you guys think. > > - Will > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev > _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On 03/13/2014 12:35 PM, William Burns wrote: > On Thu, Mar 13, 2014 at 8:31 AM, Pedro Ruivo <pedro(a)infinispan.org&gt; wrote: >> Hi, >> >> #1 and #2 are ok to me but, IMO, the filter package should be in commons >> module > > Sorry I forgot to detail why I said core. I originally planned for > commons package as well, however the KeyValueFilter class needs the > Metadata class, which doesn't live in the commons package. I didn't > want to separate the 2 filter classes. And unfortunately the Metadata > class relies on other classes in core, so that isn't easy to move over > either, but doable :( WDYT? can you explain why the metadata is needed? I assumed that the key and the value were the only objects needed.

> >> >> Cheers, >> Pedro >> >> On 03/13/2014 12:07 PM, William Burns wrote: >>> Recently while working on some ISPN 7 features, there were some public >>> API inconsistencies. I wanted to bring these up just in case if >>> someone had concerns. >>> >>> The first few are pretty trivial, but can cause compilation errors >>> between versions if user code implements these interfaces and defines >>> types. >>> >>> 1. The CacheWriter interface currently defines a delete(K key) method. >>> To be more inline with JCache and java.util.collections interfaces I >>> was hoping to change this to be delete(Object key) instead. >>> 2. The CacheLoader interface currently defines load(K key) and >>> contains(K key) methods. Similar to above I was hoping to change the >>> K type to be Object to be more inline with JCache and >>> java.util.collections interfaces. >>> >>> This last one is a bit more major, but currently we have 2 classes >>> that are named KeyFilter. One that resides in the >>> org.infinispan.notifications package and another that resides in the >>> org.infinispan.persistence.spi.AdvancedCacheLoader interface. >>> >>> 3. My plan is instead to consolidate these classes into 1 into a new >>> core org.infinispan.filter package. I would also move the new >>> KeyValueFilter class that was added for cluster listeners into this >>> package and their accompanying implementations. >>> >>> The first 2 is currently implemented as changes in >>> https://github.com/infinispan/infinispan/pull/2423. The latter I was >>> going to add into changes for >>> https://issues.jboss.org/browse/ISPN-4068. >>> >>> Let me know what you guys think. >>> >>> - Will >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev > _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

Is there a migration guide? This should be documented in some wiki page similar to - https://community.jboss.org/wiki/HibernateSearchMigrationGuide

On 13 March 2014 12:45, William Burns <mudokonman(a)gmail.com&gt; wrote: > On Thu, Mar 13, 2014 at 8:37 AM, Pedro Ruivo <pedro(a)infinispan.org&gt; wrote: >> >> >> On 03/13/2014 12:35 PM, William Burns wrote: >>> On Thu, Mar 13, 2014 at 8:31 AM, Pedro Ruivo <pedro(a)infinispan.org&gt; wrote: >>>> Hi, >>>> >>>> #1 and #2 are ok to me but, IMO, the filter package should be in commons >>>> module >>> >>> Sorry I forgot to detail why I said core. I originally planned for >>> commons package as well, however the KeyValueFilter class needs the >>> Metadata class, which doesn't live in the commons package. I didn't >>> want to separate the 2 filter classes. And unfortunately the Metadata >>> class relies on other classes in core, so that isn't easy to move over >>> either, but doable :( WDYT? >> >> can you explain why the metadata is needed? I assumed that the key and >> the value were the only objects needed. > > That is how the design doc was written up :P My guess is so that > people if needed can filter out versioned entries or to possibly do > some eviction magic since they can try to calculate when the entry > would be removed. Maybe Mircea can shed some additional light. > >> >>> >>>> >>>> Cheers, >>>> Pedro >>>> >>>> On 03/13/2014 12:07 PM, William Burns wrote: >>>>> Recently while working on some ISPN 7 features, there were some public >>>>> API inconsistencies. I wanted to bring these up just in case if >>>>> someone had concerns. >>>>> >>>>> The first few are pretty trivial, but can cause compilation errors >>>>> between versions if user code implements these interfaces and defines >>>>> types. >>>>> >>>>> 1. The CacheWriter interface currently defines a delete(K key) method. >>>>> To be more inline with JCache and java.util.collections interfaces I >>>>> was hoping to change this to be delete(Object key) instead. >>>>> 2. The CacheLoader interface currently defines load(K key) and >>>>> contains(K key) methods. Similar to above I was hoping to change the >>>>> K type to be Object to be more inline with JCache and >>>>> java.util.collections interfaces. >>>>> >>>>> This last one is a bit more major, but currently we have 2 classes >>>>> that are named KeyFilter. One that resides in the >>>>> org.infinispan.notifications package and another that resides in the >>>>> org.infinispan.persistence.spi.AdvancedCacheLoader interface. >>>>> >>>>> 3. My plan is instead to consolidate these classes into 1 into a new >>>>> core org.infinispan.filter package. I would also move the new >>>>> KeyValueFilter class that was added for cluster listeners into this >>>>> package and their accompanying implementations. >>>>> >>>>> The first 2 is currently implemented as changes in >>>>> https://github.com/infinispan/infinispan/pull/2423. The latter I was >>>>> going to add into changes for >>>>> https://issues.jboss.org/browse/ISPN-4068. >>>>> >>>>> Let me know what you guys think. >>>>> >>>>> - Will >>>>> _______________________________________________ >>>>> infinispan-dev mailing list >>>>> infinispan-dev(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>>>> >>>> _______________________________________________ >>>> infinispan-dev mailing list >>>> infinispan-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On Thu, Mar 13, 2014 at 8:31 AM, Pedro Ruivo <pedro(a)infinispan.org&gt; wrote: > Hi, > > #1 and #2 are ok to me but, IMO, the filter package should be in commons > module Sorry I forgot to detail why I said core. I originally planned for commons package as well, however the KeyValueFilter class needs the Metadata class, which doesn't live in the commons package. I didn't want to separate the 2 filter classes. And unfortunately the Metadata class relies on other classes in core, so that isn't easy to move over either, but doable :( WDYT?

On Thu, Mar 13, 2014 at 11:14 AM, Vladimir Blagojevic <vblagoje(a)redhat.com&gt; wrote: > Mircea and I wanted to "promote" AdvancedCacheLoader.KeyFilter or merge > it with one filter we all choose to use because we have so many of these > internal filter classes it is ridiculous. The filter you wrote is THE one? I can't say that for sure. Basically the KeyFilter that exists in the notifications package was what I was going to use as it is more generic since the method is called accept, which I thought was better than shouldLoadKey. Are we thinking we would get rid of the KeyFilter and only use KeyValueFilter though ? - Will

On Thu, Mar 13, 2014 at 11:54 AM, Vladimir Blagojevic <vblagoje(a)redhat.com&gt; wrote: > On 3/13/2014, 11:39 AM, William Burns wrote: >> On Thu, Mar 13, 2014 at 11:14 AM, Vladimir Blagojevic >> <vblagoje(a)redhat.com&gt; wrote: >>> Mircea and I wanted to "promote" AdvancedCacheLoader.KeyFilter or merge >>> it with one filter we all choose to use because we have so many of these >>> internal filter classes it is ridiculous. The filter you wrote is THE one? >> I can't say that for sure. Basically the KeyFilter that exists in the >> notifications package was what I was going to use as it is more >> generic since the method is called accept, which I thought was better >> than shouldLoadKey. >> >> Are we thinking we would get rid of the KeyFilter and only use >> KeyValueFilter though ? >> >> - Will >> > Not sure Will. I would move both KeyFilter and KeyValueFilter in some > major package so we can all use it throughout the codebase! That is exactly what I was hoping to do with the new org.infinispan.filter package in core module.