jongyoul commented on code in PR #4631:
URL: https://github.com/apache/zeppelin/pull/4631#discussion_r1258448696


##########
zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java:
##########
@@ -871,17 +871,60 @@ public void updateNote(String noteId,
           return null;
         }
 
-        if (!(Boolean) note.getConfig().get("isZeppelinNotebookCronEnable")) {
+        if (!zConf.isZeppelinNotebookCronEnable()) {
+          boolean hasCronSettings = false;
           if (config.get("cron") != null) {
-            config.remove("cron");
+            LOGGER.warn("cron should be null when cron is disabled");
+            hasCronSettings = true;
+          }
+          if (config.get("cronExecutingUser") != null) {
+            LOGGER.warn("cronExecutingUser should be null when cron is 
disabled");
+            hasCronSettings = true;
+          }
+          if (config.get("cronExecutingRoles") != null) {
+            LOGGER.warn("cronExecutingRoles should be null when cron is 
disabled");
+            hasCronSettings = true;
+          }
+          if (hasCronSettings) {
+            callback.onFailure(new IllegalArgumentException("Wrong configs"), 
context);
+            return null;
+          }
+        } else {
+          String requestCronUser = (String) config.get("cronExecutingUser");
+          List<String> requestCronRoles = (List<String>) 
config.get("cronExecutingRoles");
+
+          if 
(!authorizationService.hasRunPermission(Collections.singleton(requestCronUser), 
note.getId())) {
+            LOGGER.error("Wrong cronExecutingUser: {}", requestCronUser);
+            callback.onFailure(new IllegalArgumentException(requestCronUser), 
context);
+            return null;
+          } else {
+            // This part should be restarted but we need to prepare to notice 
who can be a cron user in advance
+            if (!context.getUserAndRoles().contains(requestCronUser)) {
+              LOGGER.error("Wrong cronExecutingUser: {}", requestCronUser);
+              callback.onFailure(new 
IllegalArgumentException(requestCronUser), context);
+              return null;
+            }
+
+            if (context.getUserAndRoles().containsAll(requestCronRoles)) {

Review Comment:
   Oh... logic is wrong ;;;;



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