Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-16 Thread Udo Kohlmeyer
-1 to adding a copy constructor onto any /**Factory*/ classes. I have never seen a copy constructor on a Factory pattern in the wild. That said, having done some Googling, this is something that people talk about, so it cannot be THAT foreign. There is one thing I want to point out here. Ther

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-13 Thread Darrel Schneider
The v2 management api allows regions to be created remotely in cluster configuration and on running servers. But it does not allow you to create a region on a client. Non-spring applications can use RegionFactory to create the region on the client side. On Fri, Dec 13, 2019 at 9:56 AM Dan Smith

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-13 Thread Dan Smith
+1 to adding a way to copy the RegionAttributes. BTW, I really wish this RegionFactory was an interface. I don't know if adding a copy constructor makes it harder to migrate to an interface later, but maybe just having this single public method might be better? + RegionFactory createRegionFacto

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-13 Thread Mark Hanson
Hi Udo, I disagree. I believe the currently published best practice is to use RegionFactory. As a part of the work I have been doing, I have been migrating code from the AttributesFactory pattern to the RegionFactory pattern. To support that, I believe a copy constructor for RegionFactory is n

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-11 Thread Udo Kohlmeyer
I think at this point I'd be looking at the new V2 Management API's for Regions. I think any new "public" effort that we'd be adding to the product should be done through the Management API's for Regions, rather than exposing new public API's that in reality should not be made "public". --Ud

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-11 Thread Mark Hanson
Yup, fixing that. > On Dec 11, 2019, at 3:38 PM, Darrel Schneider wrote: > > Don't expose "InternalCache" on RegionFactory. You probably want it to be > "Cache". > > On Wed, Dec 11, 2019 at 3:35 PM Mark Hanson wrote: > >> >> In Cache.java >> >> + RegionFactory createRegionFactory(RegionFa

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-11 Thread Mark Hanson
Basically the point is to allow a use to copy a RegionFactory because under certain circumstances it is necessary. I found that when migrating from AttributesFactory. Thanks, Mark > On Dec 11, 2019, at 3:48 PM, Anthony Baker wrote: > > Mark, > > Can you share how the API changes will help th

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-11 Thread Mark Hanson
In discussion with Dan, I made a few realizations, so I have new API coming. Basically dropping the use of InternalCache below. Please hold off on reviewing. Thanks, Mark > On Dec 11, 2019, at 3:34 PM, Mark Hanson wrote: > > > In Cache.java > > + RegionFactory createRegionFactory(RegionF

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-11 Thread Anthony Baker
Mark, Can you share how the API changes will help the user? Thanks, Anthony > On Dec 11, 2019, at 2:57 PM, Mark Hanson wrote: > > Hi All, > > There was a suggestion that since I am making a couple user visible API > changes that I might want to ask the dev list. > > Basically I was migra

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-11 Thread Darrel Schneider
Don't expose "InternalCache" on RegionFactory. You probably want it to be "Cache". On Wed, Dec 11, 2019 at 3:35 PM Mark Hanson wrote: > > In Cache.java > > + RegionFactory createRegionFactory(RegionFactory > regionFactory); > > In RegionFactory.java > + public RegionFactory(InternalCache cach

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-11 Thread Mark Hanson
In Cache.java + RegionFactory createRegionFactory(RegionFactory regionFactory); In RegionFactory.java + public RegionFactory(InternalCache cache, RegionFactory regionFactory) and + public RegionFactory(RegionFactory regionFactory) Lastly in GemfireCacheImpl.java + public RegionFactory cre

Re: [DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-11 Thread Dan Smith
I see a lot of PR comments on those PRs. What is the new API you added? -Dan On Wed, Dec 11, 2019 at 2:57 PM Mark Hanson wrote: > Hi All, > > There was a suggestion that since I am making a couple user visible API > changes that I might want to ask the dev list. > > Basically I was migrating co

[DISCUSS] Adding a couple user APIs dealing with RegionFactory.

2019-12-11 Thread Mark Hanson
Hi All, There was a suggestion that since I am making a couple user visible API changes that I might want to ask the dev list. Basically I was migrating code from AttributesFactory to RegionFactory and hit a few snags along the way. Please see https://github.com/apache/geode/pull/4409