Alright seems fine to me. On Wed, Mar 1, 2017, 9:27 AM Tristan Tarrant <ttarr...@redhat.com> wrote:
> 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 > I get a lot of tests to change and remove now 😁 > > 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 >
_______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
