[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #42: CASSANDRASC-48: Adding new endpoint that gives information about the release version & partitioner name of a node

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

frankgh commented on code in PR #42:
URL: https://github.com/apache/cassandra-sidecar/pull/42#discussion_r1020523157


##########
src/main/java/org/apache/cassandra/sidecar/routes/CassandraHealthService.java:
##########
@@ -68,16 +71,35 @@ public CassandraHealthService(InstanceMetadataFetcher metadataFetcher)
     public Response getCassandraHealth(@Context HttpServerRequest req,
                                        @QueryParam(AbstractHandler.INSTANCE_ID) Integer instanceId)
     {
-        CassandraAdapterDelegate cassandra;
-        if (instanceId != null)
+        CassandraAdapterDelegate cassandra = getCassandraAdapterDelegate(req.host(), instanceId);
+        return getHealthResponse(cassandra);
+    }
+
+    @Operation(summary = "Cassandra's node status",
+    description = "Returns HTTP 200 if Cassandra is available, 503 otherwise",
+    responses = {
+    @ApiResponse(responseCode = "200", description = "Cassandra status is available"),
+    @ApiResponse(responseCode = "503", description = "Cassandra status is not available")
+    })
+    @GET
+    @Path("/v1/cassandra/status")

Review Comment:
   does settings make more sense?
   ```suggestion
       @Path("/v1/cassandra/settings")
   ```



##########
src/main/java/org/apache/cassandra/sidecar/routes/CassandraHealthService.java:
##########
@@ -68,16 +71,35 @@ public CassandraHealthService(InstanceMetadataFetcher metadataFetcher)
     public Response getCassandraHealth(@Context HttpServerRequest req,
                                        @QueryParam(AbstractHandler.INSTANCE_ID) Integer instanceId)
     {
-        CassandraAdapterDelegate cassandra;
-        if (instanceId != null)
+        CassandraAdapterDelegate cassandra = getCassandraAdapterDelegate(req.host(), instanceId);
+        return getHealthResponse(cassandra);
+    }
+
+    @Operation(summary = "Cassandra's node status",

Review Comment:
   the name of the class `CassandraHealthService` doesn't make sense anymore if we add another endpoint here. We should either rename the class, or add a new class for the new endpoint.



##########
src/main/java/org/apache/cassandra/sidecar/routes/CassandraHealthService.java:
##########
@@ -68,16 +71,35 @@ public CassandraHealthService(InstanceMetadataFetcher metadataFetcher)
     public Response getCassandraHealth(@Context HttpServerRequest req,
                                        @QueryParam(AbstractHandler.INSTANCE_ID) Integer instanceId)
     {
-        CassandraAdapterDelegate cassandra;
-        if (instanceId != null)
+        CassandraAdapterDelegate cassandra = getCassandraAdapterDelegate(req.host(), instanceId);
+        return getHealthResponse(cassandra);
+    }
+
+    @Operation(summary = "Cassandra's node status",
+    description = "Returns HTTP 200 if Cassandra is available, 503 otherwise",
+    responses = {
+    @ApiResponse(responseCode = "200", description = "Cassandra status is available"),
+    @ApiResponse(responseCode = "503", description = "Cassandra status is not available")
+    })
+    @GET
+    @Path("/v1/cassandra/status")
+    public Response status(@Context HttpServerRequest req,
+                           @QueryParam(AbstractHandler.INSTANCE_ID) Integer instanceId)
+    {
+        NodeStatus nodeStatus;
+        try
         {
-            cassandra = metadataFetcher.getDelegate(instanceId);
+            CassandraAdapterDelegate cassandra = getCassandraAdapterDelegate(req.host(), instanceId);
+            nodeStatus = cassandra.getStatus();
         }
-        else
+        catch (IllegalArgumentException e)

Review Comment:
   I don't know if this try/catch block is necessary, I am not able to pin point exactly where the `IllegalArgumentException` would come from



-- 
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: pr-unsubscribe@cassandra.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org