vikaskr22 commented on code in PR #650:
URL: https://github.com/apache/ranger/pull/650#discussion_r2317945608
##########
kms/src/main/java/org/apache/ranger/kms/dao/BaseDao.java:
##########
@@ -117,6 +117,7 @@ public T create(T obj) {
logger.error("create({}) failed", tClass.getSimpleName(), e);
rollbackTransaction();
+ throw e;
Review Comment:
@vyommani , Thanks again for your time. You have raised two points:
1. **On throwing the exception as it is instead of wrapping inside
RuntimeException:** Yes, the current approach works. And the reason is, there
is not any checked exception being thrown from the method or the API being used
inside that method. EntityManager is being used for DB operations like
commit/rollback etc, if you see this API doesn't throw any checked exception.
In some cases it throws only RuntimeExceptions. And also, if there is any API
being used throws any checked exceptions, it will be a compile time error. So
this approach is working.
2. **Why exception is being thrown after rollbacktransaction() :** Yes,
this is required. And this is the bug.
The current issue is, Key being created with very long attribute ( beyond
column size limits) is being shown as successful on UI but key actually doesn't
exist. Actually, due to DB runtime error, it is being rolled back but service
layer API is not getting any exception or error code, so it was returning 200
OK from the REST. That is incorrect. here I am trying to propagate the runtime
exception (instead of simply logging ) so that API returns 500, internal server
error instead of 200 OK.
I hope it's clear now. Please let me know if there are any other concerns.
--
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: dev-unsubscr...@ranger.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
