nastra commented on code in PR #16024:
URL: https://github.com/apache/iceberg/pull/16024#discussion_r3117475360
##########
core/src/main/java/org/apache/iceberg/rest/responses/PlanTableScanResponse.java:
##########
@@ -58,6 +61,14 @@ public String planId() {
return planId;
}
+ public ErrorResponse errorResponse() {
Review Comment:
same as in the other class. We need to validate() that this is only set when
we have "failed" status
##########
core/src/main/java/org/apache/iceberg/rest/responses/FetchPlanningResultResponse.java:
##########
@@ -50,6 +53,14 @@ public PlanStatus planStatus() {
return planStatus;
}
+ public ErrorResponse errorResponse() {
Review Comment:
I think we should update validate() to verify that the error is only added
for "failed"? At least that's how I'm reading the spec
##########
core/src/main/java/org/apache/iceberg/rest/responses/PlanTableScanResponse.java:
##########
@@ -58,6 +61,14 @@ public String planId() {
return planId;
}
+ public ErrorResponse errorResponse() {
+ return errorResponse;
+ }
+
+ public String errorMessage() {
Review Comment:
I think the full error msg should be whatever errorResponse#toString()
returns, which includes the HTTP code and type (but maybe without the stack
trace). Maybe we should remove this method completely and use `errorResponse()`
directly everywhere?
##########
core/src/main/java/org/apache/iceberg/rest/responses/ErrorResponseParser.java:
##########
@@ -46,9 +46,13 @@ public static String toJson(ErrorResponse errorResponse,
boolean pretty) {
public static void toJson(ErrorResponse errorResponse, JsonGenerator
generator)
throws IOException {
generator.writeStartObject();
+ writeErrorField(errorResponse, generator);
+ generator.writeEndObject();
+ }
+ public static void writeErrorField(ErrorResponse errorResponse,
JsonGenerator generator)
Review Comment:
nit: maybe `errorFieldToJson` to align with the to/FromJson naming?
--
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]
