On 01/03/17 13:01, Dan Berindei wrote: > On Wed, Mar 1, 2017 at 10:18 AM, Radim Vansa <rva...@redhat.com> wrote: >> I still think that if the cache is already defined, defineConfiguration >> should throw an exception. This javadoc was written 7 years ago [1],
> I agree with throwing an exception in defineConfiguration(...) when > that cache is already defined. +1 > getCache(cache, configurationName) when the cache is already defined, > I'd just ignore the new configuration (as we already ignore it when > the cache is runninng) and maybe log a warning telling people to use > defineConfiguration(cacheName, configurationName, new > ConfigurationBuilder().build()) + getCache(cacheName). +1 Tristan >> R. >> >> [1] >> https://github.com/infinispan/infinispan/commit/73d99d37ebfb8af6b64df6a7579a2448deacbde7#diff-e2b618b97769a45ec42eb5910a8c2119R62 >> >> On 02/28/2017 10:51 PM, William Burns wrote: >>> So while I was trying to work on this, I have to admit I am even more >>> torn in regards to what to do. Looking at [1] it looks like the >>> template should only be applied if the cache configuration is not >>> currently defined. Unfortunately it doesn't work this way and always >>> applies this template to any existing configuration. So I am thinking >>> an alternative is to instead make it work as the documentation states, >>> only using the template if the cache is not currently defined. This >>> seems more logical to me at least. >>> >>> With that change the getCache(String, String) could stay as long as it >>> is documented that a template is only applied if no cache >>> configuration exists. >>> >>> What do you guys think? >>> >>> [1] >>> https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/manager/EmbeddedCacheManager.java#L84 >>> >>> >>> On Mon, Feb 27, 2017 at 10:09 AM William Burns <mudokon...@gmail.com >>> <mailto:mudokon...@gmail.com>> wrote: >>> >>> On Mon, Feb 27, 2017 at 9:55 AM Dan Berindei >>> <dan.berin...@gmail.com <mailto:dan.berin...@gmail.com>> wrote: >>> >>> I would go for option 2. >>> >>> >>> Do you think a WARN message will be enough? I am a bit weary about >>> this option myself. >>> >>> >>> We already started disconnecting the cache definition and >>> retrieval, >>> at least `getCache(name)` doesn't define a new cache based on the >>> default configuration any more. So I don't think it would be >>> too much, >>> even at this point, to deprecate all the overloads of >>> `getCache` that >>> can define a new cache and advise users to use >>> `defineConfiguration` >>> instead. >>> >>> >>> Hrmm I like the idea of deprecating the overloads :) >>> >>> >>> >>> >>> Cheers >>> Dan >>> >>> >>> On Mon, Feb 27, 2017 at 4:31 PM, William Burns >>> <mudokon...@gmail.com <mailto:mudokon...@gmail.com>> wrote: >>> > When working on another project using Infinispan the code >>> being used was a >>> > bit interesting and I don't think our template configuration >>> handling was >>> > expecting it do so in such a way. >>> > >>> > Essentially the code defined a template for a distributed >>> cache as well as >>> > some named caches. Then whenever a cache is retrieved it >>> would pass the >>> > given name and always the distributed cache template. >>> Unfortunately with >>> > the way templates work they essentially redefine a cache >>> first so the actual >>> > cache configuration was wiped out. In this example I was >>> able to get the >>> > code to change to using a default cache instead, which is >>> the behavior that >>> > is needed. >>> > >>> > The issue though at hand is whether we should allow a user >>> to call getCache >>> > in such a way. My initial thought is to have it throw some >>> sort of >>> > configuration exception when this is invoked. But there are >>> some possible >>> > options. >>> > >>> > 1. Throw a configuration exception not allowing a user to >>> use a template >>> > with an already defined cache. This has a slight disconnect >>> between >>> > configuration and runtime, since if a user adds a new >>> definition it could >>> > cause runtime issues. >>> > 2. Log an error/warning message when this occurs. Is this >>> enough though? >>> > Still could have runtime issues that are possibly undetected. >>> > 3. Merge the configurations together applying the template >>> first. This >>> > would be akin to how default cache works currently, but you >>> would get to >>> > define your default template configuration at runtime. This >>> sounded like the >>> > best option to me, but the problem is what if someone calls >>> getCache using >>> > the same cache name but a different template. This could get >>> hairy as well. >>> > >>> > Really thinking about the future, disconnecting the cache >>> definition and >>> > retrieval would be the best option, but we can't do that >>> this late in the >>> > game. >>> > >>> > What do you guys think? >>> > >>> > - Will >>> > >>> > _______________________________________________ >>> > infinispan-dev mailing list >>> > infinispan-dev@lists.jboss.org >>> <mailto:infinispan-dev@lists.jboss.org> >>> > https://lists.jboss.org/mailman/listinfo/infinispan-dev >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev@lists.jboss.org >>> <mailto:infinispan-dev@lists.jboss.org> >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>> >>> >>> >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev@lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> >> -- >> Radim Vansa <rva...@redhat.com> >> JBoss Performance Team >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > _______________________________________________ > infinispan-dev mailing list > infinispan-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev > -- Tristan Tarrant Infinispan Lead JBoss, a division of Red Hat _______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev