psalagnac commented on code in PR #2304:
URL: https://github.com/apache/solr/pull/2304#discussion_r1511341651
##########
solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeCommandStatus.java:
##########
@@ -51,25 +51,24 @@ public GetNodeCommandStatus(
public GetNodeCommandStatusResponse getCommandStatus(String requestId) {
ensureRequiredParameterProvided(CoreAdminParams.REQUESTID, requestId);
var requestStatusResponse = new GetNodeCommandStatusResponse();
- if
(coreAdminAsyncTracker.getRequestStatusMap(RUNNING).containsKey(requestId)) {
- requestStatusResponse.status = RUNNING;
- } else if
(coreAdminAsyncTracker.getRequestStatusMap(COMPLETED).containsKey(requestId)) {
- requestStatusResponse.status = COMPLETED;
- requestStatusResponse.response =
-
coreAdminAsyncTracker.getRequestStatusMap(COMPLETED).get(requestId).getRspObject();
- requestStatusResponse.response =
- coreAdminAsyncTracker
- .getRequestStatusMap(COMPLETED)
- .get(requestId)
- .getOperationRspObject();
- } else if
(coreAdminAsyncTracker.getRequestStatusMap(FAILED).containsKey(requestId)) {
- requestStatusResponse.status = FAILED;
- requestStatusResponse.response =
-
coreAdminAsyncTracker.getRequestStatusMap(FAILED).get(requestId).getRspObject();
- } else {
+
+ TaskObject taskObject =
coreAdminAsyncTracker.getAsyncRequestForStatus(requestId);
+ String status = taskObject != null ? taskObject.getStatus() : null;
+
+ if (status == null) {
requestStatusResponse.status = "notfound";
requestStatusResponse.msg = "No task found in running, completed or
failed tasks";
+ } else {
+ requestStatusResponse.status = status;
+
+ if (taskObject.getStatus().equals(COMPLETED)) {
+ requestStatusResponse.response = taskObject.getRspObject();
+ requestStatusResponse.response = taskObject.getOperationRspObject();
Review Comment:
Oh, good catch!
Something is definitely wrong here, but this is existing code. I did not
notice that.
I think this deserves a a fix by itself. Values for `msg` and `response` in
the V2 API response are not consistent depending on the status. And it seems to
me there not consistent with V1 either.
I think this was introduced with #2144.
Maybe I should put this PR on hold and fix this first?
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
