[GitHub] [helix] junkaixue commented on a change in pull request #1807: Add REST APIs for management mode

[GitBox <gi...@apache.org>]

junkaixue commented on a change in pull request #1807:
URL: https://github.com/apache/helix/pull/1807#discussion_r659494208



##########
File path: helix-core/src/main/java/org/apache/helix/HelixAdmin.java
##########
@@ -381,6 +381,17 @@ void manuallyEnableMaintenanceMode(String clusterName, boolean enabled, String r
    */
   void setClusterManagementMode(ClusterManagementModeRequest request);
 
+  /**
+   * Gets cluster management status {@link ClusterManagementMode}: what mode the cluster is and
+   * whether the cluster has fully reached to that mode.
+   *
+   * @param clusterName cluster name
+   * @return {@link ClusterManagementMode}
+   */
+  default ClusterManagementMode getClusterManagementMode(String clusterName) {

Review comment:
       Why you need a default implementation here? We can make it in real implementation right? It is not necessary.

##########
File path: helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
##########
@@ -521,6 +522,15 @@ public void setClusterManagementMode(ClusterManagementModeRequest request) {
     }
   }
 
+  @Override
+  public ClusterManagementMode getClusterManagementMode(String clusterName) {
+    HelixDataAccessor accessor =

Review comment:
       I remember we have getHelixDataAccessor()

##########
File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
##########
@@ -297,6 +305,113 @@ public Response updateCluster(@PathParam("clusterId") String clusterId,
     return OK();
   }
 
+  @ResponseMetered(name = HttpConstants.READ_REQUEST)
+  @Timed(name = HttpConstants.READ_REQUEST)
+  @GET
+  @Path("{clusterId}/management-mode")
+  public Response getClusterManagementMode(@PathParam("clusterId") String clusterId,
+      @QueryParam("showDetails") boolean showDetails) {
+    ClusterManagementMode mode = getHelixAdmin().getClusterManagementMode(clusterId);
+    if (mode == null) {
+      return notFound("Cluster " + clusterId + " is not in management mode");

Review comment:
       Better not return notFound. This is 404.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org