Copilot commented on code in PR #13370:
URL: https://github.com/apache/gravitino/pull/13370#discussion_r4059852022
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueExceptionConverter.java:
##########
@@ -118,6 +192,19 @@ static RuntimeException toTableException(GlueException e,
String context) {
return new RuntimeException("Glue error: " + context + ": " +
awsErrorDetail(e), e);
}
+ private static RuntimeException toAuthenticationException(GlueException e,
String context) {
+ return new RuntimeException(
+ String.format(
+ "Failed to authenticate with AWS Glue for %s. AWS rejected the
configured "
+ + "credentials. Verify the '%s' and '%s' catalog properties,
or the configured "
+ + "default AWS credential source. AWS error: %s",
+ context,
+ GlueConstants.AWS_ACCESS_KEY_ID,
+ GlueConstants.AWS_SECRET_ACCESS_KEY,
+ awsErrorDetail(e)),
+ e);
Review Comment:
`toAuthenticationException` is used for schema/table operations, but it
returns a plain `RuntimeException`. The schema/table REST handlers only
classify `ConnectionFailedException` as a downstream connection failure, so
rejected AWS credentials fall through to the generic internal-error response
instead of the connection-failed response. Return `ConnectionFailedException`
here while retaining `e` as the cause, to preserve the same error category as
`testConnection`.
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueExceptionConverter.java:
##########
@@ -103,6 +174,9 @@ 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);
+ }
Review Comment:
The new table-authentication branch is not exercised by the added tests:
`testRejectedCredentialsIncludePropertyNamesAndAwsError` calls
`toSchemaException`, while the table converter test uses
`AccessDeniedException` and skips this branch. Add a table-operation/converter
test with `UnrecognizedClientException` so table paths are protected from
regressing to raw AWS errors.
--
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]
