collado-mike commented on code in PR #1159:
URL: https://github.com/apache/polaris/pull/1159#discussion_r1990724661
##########
service/common/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -223,39 +212,52 @@ public static int
extractHttpCodeFromCloudException(Throwable t) {
};
}
- static int mapCloudExceptionToResponseCode(Throwable t) {
- if (doesAnyThrowableContainAccessDeniedHint(t)) {
- return Status.FORBIDDEN.getStatusCode();
+ /**
+ * Tries mapping a cloud exception to the HTTP code that Polaris should
return
+ *
+ * @param t the throwable/exception
+ * @return the HTTP code Polaris should return, if it was possible to return
a suitable mapping.
+ * Optional.empty() otherwise.
+ */
+ static Optional<Integer> mapCloudExceptionToResponseCode(Throwable t) {
+ if (!(t instanceof S3Exception
+ || t instanceof AzureException
+ || t instanceof StorageException)) {
+ return Optional.empty();
+ }
+
+ if (containsAnyAccessDeniedHint(t.getMessage())) {
Review Comment:
I'm wondering if we shouldn't start making about of this configurable. E.g.,
the access denied text could be updated by config rather than hardcoding text.
And the mapping of cloud vendor status codes to response codes could be useful
to control by config. Doesn't have to block this PR, but it's a thought
--
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]
