This is an automated email from the ASF dual-hosted git repository.
tbonelee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 78161e34de [ZEPPELIN-6600] Refresh the cron scheduler only when the
expression changes
78161e34de is described below
commit 78161e34de0f96cb035b573edceec1203d46c516
Author: renechoi <[email protected]>
AuthorDate: Sat Sep 5 00:14:14 2026 +0900
[ZEPPELIN-6600] Refresh the cron scheduler only when the expression changes
### What is this PR for?
`NotebookService.isCronUpdated` reports an update whenever either config
carries a cron
expression, so saving an unrelated setting on a scheduled note unregisters
and re-registers its
Quartz job. The equality branch sets the flag it should clear. Comparing
the values directly
leaves the scheduler alone unless the expression changed.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-6600
### How should this be tested?
`NotebookServiceTest#testCronRefreshedOnlyWhenTheExpressionChanges` walks a
note through add,
unrelated edit, change and removal. Master fails at that edit.
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5442 from renechoi/ZEPPELIN-6600.
Signed-off-by: ChanHo Lee <[email protected]>
---
.../apache/zeppelin/service/NotebookService.java | 12 +++--
.../zeppelin/service/NotebookServiceTest.java | 51 ++++++++++++++++++++--
2 files changed, 53 insertions(+), 10 deletions(-)
diff --git
a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
index 9e5e31aa1c..eacd970eb0 100644
---
a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
+++
b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
@@ -991,15 +991,13 @@ public class NotebookService {
private boolean isCronUpdated(Map<String, Object> configA, Map<String,
Object> configB) {
- boolean cronUpdated = false;
- if (configA.get("cron") != null && configB.get("cron") != null &&
configA.get("cron")
- .equals(configB.get("cron"))) {
- cronUpdated = true;
- } else if (configA.get("cron") != null || configB.get("cron") != null) {
- cronUpdated = true;
+ Object cronA = configA.get("cron");
+ Object cronB = configB.get("cron");
+ if (cronA == null) {
+ return cronB != null;
}
- return cronUpdated;
+ return !cronA.equals(cronB);
}
public void saveNoteForms(String noteId,
diff --git
a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
index 2eeb0f650c..1c4f554d50 100644
---
a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
+++
b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
@@ -69,6 +69,7 @@ import
org.apache.zeppelin.notebook.exception.NotePathAlreadyExistsException;
import org.apache.zeppelin.notebook.repo.NotebookRepo;
import org.apache.zeppelin.notebook.repo.VFSNotebookRepo;
import org.apache.zeppelin.notebook.scheduler.QuartzSchedulerService;
+import org.apache.zeppelin.notebook.scheduler.SchedulerService;
import org.apache.zeppelin.rest.exception.ForbiddenException;
import org.apache.zeppelin.rest.exception.NoteNotFoundException;
import org.apache.zeppelin.scheduler.Job.Status;
@@ -95,6 +96,7 @@ class NotebookServiceTest {
private SearchService searchService;
private Notebook notebook;
private AuthorizationService authorizationService;
+ private ZeppelinConfiguration zConf;
private ServiceContext context =
new ServiceContext(AuthenticationInfo.ANONYMOUS, new HashSet<>());
@@ -106,11 +108,12 @@ class NotebookServiceTest {
@BeforeEach
void setUp(TestInfo testInfo) throws Exception {
notebookDir =
Files.createTempDirectory("notebookDir").toAbsolutePath().toFile();
- ZeppelinConfiguration zConf = ZeppelinConfiguration.load();
+ zConf = ZeppelinConfiguration.load();
zConf.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_DIR.getVarName(),
notebookDir.getAbsolutePath());
- // enable cron for testNoteUpdate method
- if ("testNoteUpdate()".equals(testInfo.getDisplayName())){
+ // enable cron for the tests that update a note's cron settings
+ if ("testNoteUpdate()".equals(testInfo.getDisplayName())
+ ||
"testCronRefreshedOnlyWhenTheExpressionChanges()".equals(testInfo.getDisplayName()))
{
confDir = Files.createTempDirectory("confDir").toAbsolutePath().toFile();
zConf.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_CONF_DIR.getVarName(),
confDir.getAbsolutePath());
@@ -474,6 +477,48 @@ class NotebookServiceTest {
});
}
+ @Test
+ void testCronRefreshedOnlyWhenTheExpressionChanges() throws IOException {
+ SchedulerService schedulerService = mock(SchedulerService.class);
+ NotebookService service =
+ new NotebookService(notebook, authorizationService, zConf,
schedulerService);
+ String noteId = service.createNote("/folder_cron/note_test_cron", "test",
true, context,
+ callback);
+
+ Map<String, Object> config = new HashMap<>();
+ config.put("isZeppelinNotebookCronEnable", true);
+ config.put("looknfeel", "looknfeel");
+ config.put("cron", "0 0/5 * * * ?");
+ config.put("cronExecutingRoles", "[\"test\"]");
+ config.put("cronExecutingUser", "test");
+
+ // adding a cron expression schedules the note
+ service.updateNote(noteId, "note_test_cron", new HashMap<>(config),
context, callback);
+ verify(schedulerService).refreshCron(noteId);
+
+ // an unrelated change that keeps the same expression leaves the scheduler
alone.
+ // onSuccess proves the update ran to the end, since updateNote has
earlier exits that
+ // would satisfy never() without ever reaching the cron decision
+ reset(schedulerService);
+ reset(callback);
+ config.put("looknfeel", "simple");
+ service.updateNote(noteId, "note_test_cron", new HashMap<>(config),
context, callback);
+ verify(callback).onSuccess(any(Note.class), any(ServiceContext.class));
+ verify(schedulerService, never()).refreshCron(noteId);
+
+ // changing the expression schedules it again
+ reset(schedulerService);
+ config.put("cron", "0 0 0/1 * * ?");
+ service.updateNote(noteId, "note_test_cron", new HashMap<>(config),
context, callback);
+ verify(schedulerService).refreshCron(noteId);
+
+ // removing the expression unschedules it
+ reset(schedulerService);
+ config.remove("cron");
+ service.updateNote(noteId, "note_test_cron", new HashMap<>(config),
context, callback);
+ verify(schedulerService).refreshCron(noteId);
+ }
+
@Test
void testRenameNoteRejectsDuplicate() throws IOException {
String note1Id = notebookService.createNote("/folder/note1", "test", true,
context, callback);
