dimas-b commented on code in PR #1989:
URL: https://github.com/apache/polaris/pull/1989#discussion_r2222945900
##########
service/common/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java:
##########
@@ -45,23 +46,19 @@ public class PolarisExceptionMapper implements
ExceptionMapper<PolarisException>
private static final Logger LOGGER =
LoggerFactory.getLogger(PolarisExceptionMapper.class);
private Response.Status getStatus(PolarisException exception) {
- if (exception instanceof AlreadyExistsException) {
- return Response.Status.CONFLICT;
- } else if (exception instanceof InvalidPolicyException) {
- return Response.Status.BAD_REQUEST;
- } else if (exception instanceof PolicyAttachException) {
- return Response.Status.BAD_REQUEST;
- } else if (exception instanceof NoSuchPolicyException) {
- return Response.Status.NOT_FOUND;
- } else if (exception instanceof PolicyVersionMismatchException) {
- return Response.Status.CONFLICT;
- } else if (exception instanceof PolicyMappingAlreadyExistsException) {
- return Response.Status.CONFLICT;
- } else if (exception instanceof PolicyInUseException) {
- return Response.Status.BAD_REQUEST;
- } else {
- return Response.Status.INTERNAL_SERVER_ERROR;
- }
+ return switch (exception) {
+ case AlreadyExistsException alreadyExistsException ->
Response.Status.CONFLICT;
+ case CommitConflictException commitConflictException ->
Response.Status.REQUEST_TIMEOUT;
Review Comment:
With the current state of the PR (no retries on persistence-level conflicts)
I believe the 409 (Conflict) response code is the most relevant.
However, as discussed on the [dev email
thread](https://lists.apache.org/thread/gp4f84ld08z1khrd09klzdzflv7l7fgh), I do
not think 409 is the best possible implementation of the Iceberg REST API. I
believe Polaris should retry on the server side and respond with 429 (Too Many
Requests) if retries do not help.
That said, I understand that implementing retries is a much bigger effort,
I'd be ok with 409 for now (as it is incrementally better than 500).
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
