andrew4699 commented on code in PR #402:
URL: https://github.com/apache/polaris/pull/402#discussion_r1817249080
##########
polaris-service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -79,6 +89,7 @@ public Response toResponse(RuntimeException runtimeException)
{
case RESTException e ->
Response.Status.SERVICE_UNAVAILABLE.getStatusCode();
case IllegalArgumentException e ->
Response.Status.BAD_REQUEST.getStatusCode();
case UnsupportedOperationException e ->
Response.Status.NOT_ACCEPTABLE.getStatusCode();
+ case Exception e when isAccessDenied(e) ->
Response.Status.FORBIDDEN.getStatusCode();
Review Comment:
Good call. I scoped it down to a high level exception type for each
respective SDK, i.e. S3Exception/AzureException/StorageException. I was able to
figure those out by running some test code:
```
@Test
void testXRemoveLater() {
try {
var x =
CatalogUtil.loadFileIO(
"org.apache.iceberg.azure.adlsv2.ADLSFileIO", Map.of(), new
Configuration());
var file = x.newInputFile("abfss://[email protected]/abc");
file.getLength();
} catch (Exception e) {
ExceptionUtils.getThrowableList(e)
.forEach(t -> System.out.println("name:" +
t.getClass().getName()));
throw e;
}
}
```
We could try to figure out even more granular exception types, but I think
these strikes a reasonable balance, and are definitely better than the previous
use of Exception.
--
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]
