GEODE-6384: Create consistent API to retrieve instances of ClusterManagementService #3191
Conversation
jdeppe-pivotal
requested review from
jinmeiliao,
kohlmu-pivotal and
pdxrunner
|
@Petahhh @onichols-pivotal For your review... |
| import org.apache.geode.management.client.ClusterManagementServiceProvider; | ||
| import org.apache.geode.management.spi.ClusterManagementServiceProviderFactory; | ||
|
|
||
| public class SmartClusterManagementServiceProviderFactory implements |
There was a problem hiding this comment.
This needs a javadoc. Also an @Experimental annotation
There was a problem hiding this comment.
I've added some javadoc and also moved the class to internal as it isn't an API a user would directly interact with.
| import org.apache.geode.management.api.ClusterManagementService; | ||
| import org.apache.geode.management.client.ClusterManagementServiceProvider; | ||
|
|
||
| public class GeodeClientClusterManagementService implements ClusterManagementService { |
There was a problem hiding this comment.
As noted above, All the new public API classes need to have javadocs and @Experimental annotations
There was a problem hiding this comment.
I've deleted this class as it's not necessary at the moment.
| * exists. | ||
| */ | ||
| @Experimental | ||
| public class ClusterManagementClient implements ClusterManagementService { |
There was a problem hiding this comment.
Isn't this the same as GeodeClientClusterManagementService and ServerClusterManagementService?
jdeppe-pivotal
force-pushed
the
feature/GEODE-6384
branch
from
15d6a16
to
b66ed41
Compare
| public class ClusterManagementClient implements ClusterManagementService { | ||
|
|
||
| private static final ResponseErrorHandler DEFAULT_ERROR_HANDLER = | ||
| new RestTemplateResponseErrorHandler(); |
There was a problem hiding this comment.
Do we want to change this error handler to something else? can we makeRestTemplateResponseErrorHandler an inner class to contain the error handling in the client?
|
|
||
| ClusterManagementResult result = client.create(region, ""); | ||
|
|
||
| // This all fails in light of running this test repeatedly as a stress test. Until we introduce |
| private static List<ClusterManagementServiceProviderFactory> providerFactories = null; | ||
|
|
||
| public static ClusterManagementService getService() { | ||
| for (String context : new String[] {SERVER_CONTEXT, GEODE_CLIENT_CONTEXT, |
There was a problem hiding this comment.
this list should only be SERVER_CONTEXT, LOCATOR_CONTEXT and GEODE_CLIENT_CONTEXT, because only those context support/will support create() method without parameter.
|
|
||
| @Override | ||
| public List<String> supportedContexts() { | ||
| return Arrays.asList(ClusterManagementServiceProvider.GEODE_CLIENT_CONTEXT, |
There was a problem hiding this comment.
should this also have LOCATOR_CONTEXT? or if the create() method supports both "LOCATOR" and "SERVER" mode, then we only need one string for this.
…agementService - The overall intent of this commit is to provide a consistent means for retrieving a ClusterMangementService instance regardless of the context you are developing in. Currently, the relevant contexts are: 1) a Java application that has no access (or need to access) core Geode code. 2) a typical Geode client application and 3) server or locator-side code. - Introduces a service provider interface (ClusterManagementServiceProviderFactory) whose implementations are context dependent. This commit introduces two implementations, namely: BasicClusterManagementProviderFactory and SmartClusterManagementServiceProviderFactory). The latter is incomplete but will be fully supported in the near future. - The entry point to creating a ClusterManagementService is in ClusterManagementServiceProvider - more documentation can be found there.
- Reduce contexts to just JAVA_CLIENT_CONTEXT and GEODE_CONTEXT - Each ClusterManagementServiceProviderFactory only serves one context now.
jdeppe-pivotal
force-pushed
the
feature/GEODE-6384
branch
from
e15c65f
to
580eb3d
Compare
retrieving a ClusterMangementService instance regardless of the
context you are developing in. Currently, the relevant contexts are:
Geode code. 2) a typical Geode client application and 3) server or
locator-side code.
(ClusterManagementServiceProviderFactory) whose implementations are
context dependent. This commit introduces two implementations, namely:
BasicClusterManagementProviderFactory and
SmartClusterManagementServiceProviderFactory). The latter is
incomplete but will be fully supported in the near future.
ClusterManagementServiceProvider - more documentation can be found
there.
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
Has your PR been rebased against the latest commit within the target branch (typically
develop)?Is your initial contribution a single, squashed commit?
Does
gradlew buildrun cleanly?Have you written or updated unit tests to verify your changes?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.