mneethiraj commented on code in PR #1120:
URL: https://github.com/apache/ranger/pull/1120#discussion_r3700116276


##########
security-admin/src/main/java/org/apache/ranger/rest/RangerHealthREST.java:
##########
@@ -54,4 +59,26 @@ public RangerServerHealth getRangerServerHealth() {
 
         return rangerServerHealthUtil.getRangerServerHealth(dbVersion);
     }
+
+    @GET
+    @Path("/health/readiness")
+    @Produces("application/json")
+    @Transactional(propagation = Propagation.NOT_SUPPORTED)
+    public RangerServerHealth getRangerServerReadiness() {
+        List<String> serviceDefNames = serviceREST.getServiceDefNames();
+
+        if (serviceDefNames != null && !serviceDefNames.isEmpty()) {
+            return 
rangerServerHealthUtil.serviceUpWithAvailableServiceDefs(serviceDefNames);
+        } else {
+            return rangerServerHealthUtil.serviceDown();

Review Comment:
   "down" doesn't seem appropriate here; consider "initializing" or 
"initialization_failure".



##########
security-admin/src/main/java/org/apache/ranger/biz/SessionMgr.java:
##########
@@ -507,10 +507,15 @@ protected boolean validateUserSession(UserSessionBase 
userSession, String curren
 
     @Transactional(readOnly = false, propagation = Propagation.REQUIRES_NEW)
     protected XXAuthSession storeAuthSession(XXAuthSession gjAuthSession) {
-        // daoManager.getEntityManager().getTransaction().begin();
+        /*
+        Recording an x_auth_sess row for every liveness/readiness probe is not 
required
+         */
+        if (gjAuthSession != null && 
RangerConstants.HEALTH_CHECK_USERNAME.equalsIgnoreCase(gjAuthSession.getLoginId()))
 {

Review Comment:
   Replace repeated references to 
`RangerConstants.HEALTH_CHECK_USERNAME.equalsIgnoreCase` with a method like 
`RangerBizUtil.isHealthCheckUser(`.



##########
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java:
##########
@@ -607,6 +610,46 @@ public RangerServiceDefList getServiceDefs(@Context 
HttpServletRequest request)
         return ret;
     }
 
+    public List<String> getServiceDefNames() {
+        LOG.debug("==> ServiceREST.getServiceDefNames()");
+
+        if 
(!RangerConstants.HEALTH_CHECK_USERNAME.equalsIgnoreCase(resolveAuthenticatedLoginId()))
 {
+            throw 
restErrorUtil.createRESTException(HttpServletResponse.SC_FORBIDDEN,

Review Comment:
   If `ServiceREEST.getServiceDefNames()` meant to be called only from 
`RangerHealthREST`, why not directly have this implementation in 
`RangerHealthREST`, and eliminate the updates in ServiceREST, 
RangerServiceDefService and related classes?



-- 
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]

Reply via email to