Re: [infinispan-dev] hotrod's RemoteCacheManager

On 25 Mar 2010, at 13:41, Manik Surtani wrote: > > On 25 Mar 2010, at 06:30, Mircea Markus wrote: > >> Thanks for the feedback! >>> >> >> On 25 Mar 2010, at 02:18, Manik Surtani wrote: >> >>> >>> On 24 Mar 2010, at 11:11, Mircea Markus wrote: >>> >>>> I've checked in ongoing work of the hotrod client; it would be great to get some feedback on the public API, especially RemoteCache[1] and RemoteCacheManager[2]. >>>> For now RemoteCacheManager does not implement CacheManager interface, see email bellow. >>>> >>>> [1] http://tinyurl.com/yhb793b >>> >>> Looks good. Shouldn't VersionedEntry be VersionedValue<V> ? >>> >>> Basically Cache.get(K) returns V. So RemoteCache.getVersioned(K) should return VersionedValue<V> ? >> good point. It's a Entry<K,V>, as it extends Map.Entry<K,V> > > That would be at odds with Cache.get(K) which returns a value (V) and not a Map.Entry<K, V> .... I don't have a preference for either one so I'll go for your proposal. > >>> >>> >>>> [2] http://tinyurl.com/yh6j2we >>> >>> Don't you want RemoteCacheManager to be an interface and a separate RemoteCacheManagerImpl or something? >> I've been thinking about that; I wanted to copy the behaviour from core, in which we instantiate the DefaultCacheManagers directly. > > Yes, but DefaultCacheManager implements CacheManager. The reason why the interface is useful is because there are valid use cases where people have delegating or aggregating CacheManagers, exposed as a single CacheManager. It will implement CacheContainer (which CacheManager extends). The rest of the methods are only constructors. Wouldn't that be enough?

> Look at the stuff Brian's doing with JBoss AS 6 for example. > >>> Also, maybe you want to also provide a constructor that takes in (Properties p, int numBossThreads, int numWorkerThreads) and (Properties p, int numBossThreads, int numWorkerThreads, ClientIntelligenceLevel cil) - or are these three (boss threads, worker threads, client intelligence level) all defined in properties? >> these will all be defined in properties. It might make sense having an constructor that takes all the config props though. > > Hmm, maybe just the most common config props. > >> >>> >>> >>>> >>>> Cheers, >>>> Mircea >>>> >>>> >>>> On 23 Mar 2010, at 23:16, Mircea Markus wrote: >>>> >>>>> Hi, >>>>> >>>>> While presenting the hot rod client java API in Brno, we agreed to use a RemoteCacheManager that would implement CacheManager (similar to the way in which DefaultCacheManager implements CacheManager; it is used for clients from the same namespace). >>>>> Looking at the CacheManager interface[1], most of the methods that do not make sense on the RemoteCacheManager: defineConfiguration, getClusterName, getAddress, isCoordinator, getStatus, getGlobalConfiguration, getDefaultConfiguration and getCacheNames. >>>>> The only two methods that really make sense are: getCache() and getCache(cacheName:String). So here are some thoughts I have about having RemoteCacheManager implementing CacheManager. >>>>> 1. Approach 1 - throw exceptions for the unsupported methods. Not a very nice approach IMO, given the fact that most of the methods fall in this category, which would make the API a bit unusable >>>>> 2. Approach 2 - do not extend it. This would result in a clearer API >>>>> 3. Approach 2 - Extract the two getCache methods in an super interface (CacheHolder, CacheRepository, ??) and have both CacheManager and RemoteCacheManager implement it - this one I like the most as it encourages reusability and keeps the API clean. >>>>> >>>>> Thoughts? >>>>> >>>>> Cheers, >>>>> Mircea >>>>> >>>>> [1] http://anonsvn.jboss.org/repos/infinispan/trunk/core/src/main/java/org/in... >>>>> _______________________________________________ >>>>> 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 >>> >>> -- >>> Manik Surtani >>> manik(a)jboss.org >>> Lead, Infinispan >>> Lead, JBoss Cache >>> http://www.infinispan.org >>> http://www.jbosscache.org >>> >>> >>> >>> >>> _______________________________________________ >>> 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 > > -- > Manik Surtani > manik(a)jboss.org > Lead, Infinispan > Lead, JBoss Cache > http://www.infinispan.org > http://www.jbosscache.org > > > > > _______________________________________________ > 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