Re: [PR] Added pinot-error-code header in query response [pinot]

["ankitsultana (via GitHub)" <gi...@apache.org>]

ankitsultana commented on code in PR #12338:
URL: https://github.com/apache/pinot/pull/12338#discussion_r1474714391


##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java:
##########
@@ -348,4 +349,25 @@ private static HttpRequesterIdentity makeHttpIdentity(org.glassfish.grizzly.http
 
     return identity;
   }
+
+  /**
+   * Generate Response object from the BrokerResponse object with 'X-Pinot-Error-Code' header value
+   * @param brokerResponse
+   * @return Response
+   * @throws Exception
+   */
+  private static Response getPinotQueryResponse(BrokerResponse brokerResponse)
+      throws Exception {
+    int headerValue = -1; // default value of the header.
+
+    if (brokerResponse.getExceptionsSize() != 0) {
+      // set the header value as first exception error code value.
+      headerValue = brokerResponse.getProcessingExceptions().get(0).getErrorCode();
+    }
+
+    // returning the Response with OK status and header value.
+    return Response.ok()
+        .header(PINOT_QUERY_ERROR_CODE_HEADER, headerValue)

Review Comment:
   Yeah the issue is that the broker API returns exceptions in the JSON in most scenarios, with the HTTP Status Code set to 200. We can't change that behavior without making it backwards incompatible, hence the proposal was to add an additional response header.



-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org