sajjad-moradi commented on a change in pull request #7812:
URL: https://github.com/apache/pinot/pull/7812#discussion_r754741458
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java
##########
@@ -543,9 +543,10 @@ private static String getErrorMessage(HttpUriRequest
request, CloseableHttpRespo
StatusLine statusLine = response.getStatusLine();
String reason;
try {
- reason =
JsonUtils.stringToJsonNode(EntityUtils.toString(response.getEntity())).get("_error").asText();
+ String entityStr = EntityUtils.toString(response.getEntity());
+ reason = JsonUtils.stringToObject(entityStr,
SimpleHttpErrorInfo.class).getError();
Review comment:
This is a valid concern and we'll have incompatibility between different
versions for different component. However, as I mentioned before, this PR is
just a refactoring and the original backward incompatible changes in the data
definition shouldn't have been allowed in the first place. Since this is not
in the success path and we'll know about the error by catching the exception, I
believe it makes the code simpler and more maintainable if we don't have a
special code handling "error", "_error", and any future names.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
