KKcorps commented on code in PR #19716:
URL: https://github.com/apache/pinot/pull/19716#discussion_r4357006006
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotIngestionRestletResource.java:
##########
@@ -236,6 +239,17 @@ private SuccessResponse ingestData(String
tableNameWithType, String batchConfigM
return fileIngestionHelper.buildSegmentAndPush(payload);
}
+ /// A record failure returns its causes, which describe the uploaded data.
Other failures don't: they can involve
+ /// files on the controller that the batch config names.
+ @VisibleForTesting
+ static ControllerApplicationException newIngestFromFileException(String
message, Response.Status status,
+ Exception e) {
+ if (ExceptionUtils.indexOfType(e, RecordProcessingException.class) >= 0) {
Review Comment:
Minor: the chain starts at `e`, so any wrapper between `e` and the
`RecordProcessingException` also goes into the response. Those messages come
from code that is not reading the record, which is the kind this PR keeps out.
Nothing sensitive sits there today, but a later wrapper in
`FileIngestionHelper` could name the working dir. Could we take the chain from
the `RecordProcessingException` instead?
`ExceptionUtils.getThrowableList(e).get(index)` gives it.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotIngestionRestletResource.java:
##########
@@ -43,6 +44,7 @@
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
+import org.apache.commons.lang3.exception.ExceptionUtils;
Review Comment:
Nit: this is the commons-lang3 `ExceptionUtils`, while the mapper uses
Pinot's `org.apache.pinot.common.utils.ExceptionUtils` for the same feature.
Calling `org.apache.commons.lang3.exception.ExceptionUtils.indexOfType(...)`
inline here would save a reader from mixing them up.
--
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]
