diqiu50 commented on code in PR #14517:
URL: https://github.com/apache/gravitino/pull/14517#discussion_r4452190214
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueExceptionConverter.java:
##########
@@ -174,46 +320,150 @@ static RuntimeException toSchemaException(GlueException
e, String context) {
* @return a Gravitino or standard Java runtime exception
*/
static RuntimeException toTableException(GlueException e, String context) {
- if (isAuthenticationFailure(e)) {
- return toAuthenticationException(e, context);
- }
if (e instanceof EntityNotFoundException) {
return new NoSuchTableException(e, "%s does not exist", context);
}
if (e instanceof AlreadyExistsException) {
return new TableAlreadyExistsException(e, "%s already exists", context);
}
- if (e instanceof InvalidInputException) {
- return ExceptionMessages.illegalArgument(context, e);
+ return toGenericException(e, context, null);
+ }
+
+ /**
+ * Converts a failure raised by the Iceberg SDK into a Gravitino exception.
Iceberg reports Glue
+ * and S3 errors either as its own exception types or wrapped around the AWS
SDK exception, so the
+ * cause chain is walked from the outside in and the first recognized type
decides the result:
+ * Iceberg not-found, already-exists, validation and forbidden errors map to
the matching
+ * Gravitino types, a commit conflict maps to {@link
OptimisticLockException}, and AWS SDK causes
+ * go through {@link #toGenericException}. A commit whose state is unknown
is reported as such,
+ * because the change may have been applied. {@link
GravitinoRuntimeException}, {@link
+ * IllegalArgumentException} and {@link UnsupportedOperationException} are
returned as is.
+ *
+ * @param e the failure raised by an Iceberg catalog call
+ * @param context description of the operation context for error messages
+ * @return a Gravitino or standard Java runtime exception
+ */
+ static RuntimeException toIcebergException(RuntimeException e, String
context) {
+ if (e instanceof GravitinoRuntimeException
+ || e instanceof IllegalArgumentException
+ || e instanceof UnsupportedOperationException) {
+ return e;
}
- if (e instanceof AccessDeniedException) {
- return new ForbiddenException(e, "Glue error: %s: %s", context,
awsErrorDetail(e));
+ for (Throwable t = e; t != null; t = t.getCause() == t ? null :
t.getCause()) {
+ if (t instanceof CommitStateUnknownException) {
+ // A local validation failure underneath means nothing was sent, so
the commit did not
+ // happen. Any other cause, such as a transport failure, does not mean
the commit failed.
+ Throwable invalid = findCause(t,
org.apache.iceberg.exceptions.ValidationException.class);
+ if (invalid != null) {
+ return new IllegalArgumentException(
+ String.format("Invalid request for %s: %s", context,
describe(invalid)), e);
+ }
+ return new RuntimeException(
+ String.format(
+ "%s: the commit state is unknown and the change may have been
applied; check the"
+ + " table before retrying: %s",
+ context, describe(t.getCause() != null ? t.getCause() : t)),
+ e);
+ }
+ if (t instanceof CommitFailedException) {
+ Throwable awsCause = findCause(t, SdkException.class);
+ return awsCause == null || awsCause instanceof
ConcurrentModificationException
+ ? new OptimisticLockException(
+ e, "%s: the table was changed concurrently, retry: %s",
context, describe(t))
+ : toGenericException((SdkException) awsCause, context, null);
+ }
+ if (t instanceof AlreadyExistsException) {
+ return new TableAlreadyExistsException(
+ e, "%s: %s", context, awsErrorDetail((AlreadyExistsException) t));
+ }
+ if (t instanceof org.apache.iceberg.exceptions.AlreadyExistsException) {
+ return new TableAlreadyExistsException(e, "%s: %s", context,
describe(t));
+ }
+ if (t instanceof NoSuchNamespaceException) {
+ return new NoSuchSchemaException(e, "%s: %s", context, describe(t));
+ }
+ if (t instanceof org.apache.iceberg.exceptions.NoSuchTableException) {
+ return new NoSuchTableException(e, "%s: %s", context, describe(t));
+ }
+ if (t instanceof org.apache.iceberg.exceptions.NotFoundException) {
+ // Iceberg wraps a Glue EntityNotFoundException here; it may be the
table or its database.
+ return new NotFoundException(e, "%s: %s", context, describe(t));
Review Comment:
Thanks, agreed. We'll fix this in the follow-up Glue Iceberg table PR,
preserving `NoSuchSchemaException` for both wrapped and raw Glue exceptions,
with regression tests.
--
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]
