-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. There is work currently
being done on the new Management/Configuration V2 API's. One of the
goals of this project is to have a single API to create/update/delete
and management the configuration and creation (realization of
configuration). I'm advocating that new Management/Configuration v2
API's need to be interacting with it, rather than a user directly. This,
adding a "copy constructor" of configuration should be added onto the v2
API's rather than on the /**Factory*/ classes.
That said, having */*Factories/* as public API's that are use to create
a /*Region*/, that is correct for the module. But any utility capability
to copy configuration needs to be addressed by the Management API and
not the /*Factory*/ classes.
--Udo
On 12/13/19 10:01 AM, Darrel Schneider wrote:
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 <dsm...@pivotal.io> wrote:
+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?
+ <K, V> RegionFactory<K, V> createRegionFactory(RegionFactory<K, V>
regionFactory);
On Fri, Dec 13, 2019 at 9:35 AM Mark Hanson <mhan...@pivotal.io> wrote:
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
necessary. And thus a createRegionFactory
Hence my changes.
Thanks,
Mark
On Dec 11, 2019, at 4:41 PM, Udo Kohlmeyer <ukohlme...@pivotal.io>
wrote:
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".
--Udo
On 12/11/19 3:53 PM, Mark Hanson wrote:
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 <aba...@pivotal.io>
wrote:
Mark,
Can you share how the API changes will help the user?
Thanks,
Anthony
On Dec 11, 2019, at 2:57 PM, Mark Hanson <mhan...@pivotal.io>
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 code from AttributesFactory to
RegionFactory and hit a few snags along the way.
Please see https://github.com/apache/geode/pull/4409 <
https://github.com/apache/geode/pull/4409> specifically Cache.java,
RegionFactory.java, and for extra credit GemfireCacheImpl.java
I have commented at the relevant changes.
Thanks,
Mark
