I agree with Dan here. The C++ client took a stab at making
ExecutionService less static but didn't go this far. Rather than pull Cache
out of the either it is passed into each of the static factory methods,
wither as a Region, Pool or Cache itself. I like propose to add
Cache::getFunctionService to cache.
I find UML diagrams to be a bit heavy handed for describing APIs. Can we
just simplify this into so sample code that users will actually interact
with.
So if we were to make this change on the C++ side we would have that (C++
Cache is client cache, confusing yes).
class Cache ... {
...
FunctionService& getFunctionService();
}
class FunctionService ... {
...
Execution& onRegion(const std::string& regionName);
Execution& onRegion(std::shared_ptr<Region> region);
Execution& onServers(std::shared_ptr<Pool> pool);
Execution& onServers(); // was onServer(std::shared_ptr<RegionService>)
Execution& onServer(std::shared_ptr<Pool> pool);
Execution& onServer(); // was onServer(std::shared_ptr<RegionService>)
}
class Region ... {
...
Execution& createExecution(); // nice shortcut
}
void doExecution(Cache& cache) {
auto results = cache.getExecutionService()
.onRegion("myRegion")
.withArgs(MyFunctionArgs{"arg1", 2})
.execute("MyFunction");
...
}
void doExecution(Region& cache) {
auto results = region.createExecution()
.withArgs(MyFunctionArgs{"arg1", 2})
.execute("MyFunction");
...
}
On a related note, I would suggest we use the unqualified names for the
client since that is where real "users" of our API will be spending most of
their time. For them calling everything ClientX and ClientY is a bit
redundant.
On Wed, Mar 7, 2018 at 1:45 PM Dan Smith <dsm...@pivotal.io> wrote:
> Hi Udo,
>
> +1 for making the function service not static and spitting it into client
> and server FunctionService objects!
>
> We do have Cache and ClientCache right now. So I would recommend this API
> rather than putting two methods on Cache. Cache is already the the server
> side API.
>
> Cache {
> ServerFunctionService getFunctionService()
> }
>
> ClientCache {
> ClientFunctionService getFunctionService()
> }
>
> If at some point we split the client side API into a separate jar the API
> shouldn't need to change. If you don't like ClientFunctionService, maybe
> o.a.g.cache.client.execute.FunctionService? We would never want two
> different versions of org.apache.geode.function.FunctionService. People
> wouldn't be able to test a client and server in the same JVM.
>
> Also, you might want to check out this (somewhat stalled) proposal -
> https://cwiki.apache.org/confluence/display/GEODE/
> Function+Service+Usability+Improvements. We had buy in on this on the dev
> list but have not found cycles to actually do it yet. But maybe now is the
> time?
>
> -Dan
>
> On Wed, Mar 7, 2018 at 11:18 AM, Udo Kohlmeyer <u...@apache.org> wrote:
>
> > Hi there Apache Dev's,
> >
> > Please look at the proposal to improve the FunctionService and remove the
> > static invocation of it from within the Cache.
> >
> > https://cwiki.apache.org/confluence/display/GEODE/Function+
> > Service+Refactor+-+Removal+of+static-ness+and+splitting+of+
> > client+and+server-side+FunctionService
> >
> > --Udo
> >
> >
>
