You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2022/11/14 19:58:55 UTC
[GitHub] [cassandra-sidecar] jyothsnakonisa 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
jyothsnakonisa commented on code in PR #42:
URL: https://github.com/apache/cassandra-sidecar/pull/42#discussion_r1022000034
##########
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:
If the instanceId is incorrect then an IllegalArgumentException is being thrown. You can refer the test case `validRequestWithInvalidInstanceId `. Without this, we would throw internal server error to the client, which I think is incorrect.
##########
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:
@frankgh If the instanceId is incorrect then an IllegalArgumentException is being thrown. You can refer the test case `validRequestWithInvalidInstanceId `. Without this, we would throw internal server error to the client, which I think is incorrect.
--
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