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


##########
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:
   > or do we want to use a different header
   
   In the long term if we also want to report whether an error is retryable, or 
a client error, then we would have to add a new header.
   
   Ideally the error-codes in QueryException should have followed some 
convention (like all 4xx are user errors, and 5xx are internal errors, etc.).
   
   But given the business logic is coded into Pinot itself, I don't think we 
have any choice other than converting the error-code into the appropriate flag 
in Pinot and returning it in a dedicated header.
   
   e.g. for marking an error as a client-error, we could potentially add 
another header like `X-Pinot-Client-Error: true`.



-- 
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-unsubscr...@pinot.apache.org

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


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

Reply via email to