[ZEPPELIN-6340] Add ZeppelinEventBus and update NotebookServer to handle NoteRemoveEvent
What is this PR for?
This PR introduces the initial step of refactoring Zeppelin’s listener-based architecture to an EventBus model(proposal).
It establishes the necessary infrastructure and refactors the NoteRemoveEvent flow as the initial target.
Key Changes:
- Added
ZeppelinEventBusclass - Added RxJava3 dependency in the
zeppelin-zenginemodule - Added
zeppelin.eventbus.enabledfeature flag - Updated
NotebookServer(implements NoteEventListener) to handleNoteRemoveEventviaZeppelinEventBuswhenzeppelin.eventbus.enabledis enabled
What type of PR is it?
Improvement
Todos
- [ ] Refactor other NoteEventListener methods and additional listeners to EventBus
What is the Jira issue?
- Open an issue on Jira https://issues.apache.org/jira/browse/ZEPPELIN-6340
How should this be tested?
- Run the existing test suite against the updated code
- Verify the note deletion flow through the UI
Screenshots (if appropriate)
Questions:
- Does the license files need to update? N
- Is there breaking changes for older versions? N
- Does this needs documentation? N
I've just merged the PRs that fixed the CI failures into master. Your PR doesn't have conflicts with master, but to ensure the CI runs cleanly, could you please rebase it onto master once?
I've done it. Thanks. @tbonelee
@seung-00 I found your merge commit(daa45c1) is empty. Could you rebase again? Because CI Failure caused by zeppelin-zengine issue. It was solved(#5081) and merged to master branch.
https://github.com/apache/zeppelin/actions/runs/18057550292/job/51390882307?pr=5085#step:10:16071
@dididy The changes in this PR branch don’t conflict with master, so the empty merge commit seems natural. The CI(core.yaml) failure is probably caused by the changes in this PR. I’ll check it. Thanks.
I didn’t carefully review the PR description, so I didn’t realize it also included changes related to zeppelin-zengine. It seems that some of the current test failures(NullPointer) are likely due to those modifications.
https://github.com/seung-00/zeppelin/blob/WIP/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
https://github.com/apache/zeppelin/actions/runs/18057550292/job/51390882307?pr=5085#step:10:16003
CI Error detail
Error: Errors: Error: NotebookTest.testAbortParagraphStatusOnInterpreterRestart:1461 » NullPointer Error: NotebookTest.testAngularObjectRemovalOnInterpreterRestart:1262 » NullPointer Error: NotebookTest.testAngularObjectRemovalOnNotebookRemove:1179 » NullPointer Error: NotebookTest.testAngularObjectRemovalOnParagraphRemove:1229 » NullPointer Error: NotebookTest.testClearParagraphOutput:474 » NullPointer Error: NotebookTest.testCloneNote:1116 » NullPointer Error: NotebookTest.testCreateDuplicateNote:1737 » NullPointer Error: NotebookTest.testCreateNoteWithSubject:450 » NullPointer Error: NotebookTest.testCronNoteInTrash:1019 » NullPointer Error: NotebookTest.testExportAndImportNote:1068 » NullPointer Error: NotebookTest.testGetAllNotes:1724 » NullPointer Error: NotebookTest.testInvalidInterpreter:578 » NullPointer Error: NotebookTest.testMoveNote:1898 » NullPointer Error: NotebookTest.testMoveNote:1898 » NullPointer Error: NotebookTest.testMoveNote:1898 » NullPointer Error: NotebookTest.testPerNoteSessionInterpreter:1569->lambda$testPerNoteSessionInterpreter$50:1631 » NullPointer Error: NotebookTest.testPerSessionInterpreter:1512->lambda$testPerSessionInterpreter$48:1556 » NullPointer Error: NotebookTest.testPerSessionInterpreterCloseOnNoteRemoval:1491 » NullPointer Error: NotebookTest.testPermissions:1315 » NullPointer Error: NotebookTest.testPersist:438 » NullPointer Error: NotebookTest.testRemoveCorruptedNote:551 » NullPointer Error: NotebookTest.testResourceRemovealOnParagraphNoteRemove:1151 » NullPointer Error: NotebookTest.testRunAll:616 » NullPointer Error: NotebookTest.testRunBlankParagraph:496 » NullPointer Error: NotebookTest.testSchedule:660 » NullPointer Error: NotebookTest.testScheduleAgainstRunningAndPendingParagraph:714 » NullPointer Error: NotebookTest.testScheduleDisabled:776->terminateScheduledNote:834 » NullPointer Error: NotebookTest.testScheduleDisabledWithName:803->terminateScheduledNote:834 » NullPointer Error: NotebookTest.testSchedulePoolUsage:737->terminateScheduledNote:834 » NullPointer Error: NotebookTest.testSelectingReplImplementation:321 » NullPointer [INFO] Error: Tests run: 266, Failures: 0, Errors: 30, Skipped: 6
I am currently against the introduction of an event bus. As Jongyoul Lee wrote in an email, we should use the existing resources to ensure communication. https://lists.apache.org/thread/nk85rcn4qomxgpw56xjd804h3j9w7vz7
It has been on the roadmap for some time to combine the Zeppelin server and zeppelin-zengine code into one module, as the separation has proven to be a mistake. Currently, the providers are only available in zeppelin-server. This also became apparent to me when I tried to improve the plugin integration. See https://github.com/apache/zeppelin/pull/4355#issuecomment-1234466448
To add some clarity to my previous comment:
I strongly support the goal of this PR to remove the circular dependency. However, I agree with @Reamer that adding an event bus to zeppelin-zengine (which is only used by zeppelin-server) isn't the right solution.
This is a symptom of a larger problem: the fact that these two modules are separate to begin with.
My suggestion is to first merge zeppelin-server and zeppelin-zengine. Once that is done, we can re-apply the dependency removal logic from this PR. This approach fixes the root architectural issue and allows this PR's contribution to be implemented cleanly.
I'm open to discussing this plan.