[ZEPPELIN-6340] Add ZeppelinEventBus and update NotebookServer to handle NoteRemoveEvent#5085
[ZEPPELIN-6340] Add ZeppelinEventBus and update NotebookServer to handle NoteRemoveEvent#5085seung-00 wants to merge 2 commits intoapache:masterfrom
Conversation
|
I've just merged the PRs that fixed the CI failures into master. |
|
I've done it. Thanks. |
|
@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 |
|
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/apache/zeppelin/actions/runs/18057550292/job/51390882307?pr=5085#step:10:16003 CI Error detailError: 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 |
fb157db to
605499c
Compare
- Add ZeppelinEventBus - Update NotebookServer to handle NoteRemoveEvent using EventBus
|
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 #4355 (comment) |
|
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. |
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
NoteRemoveEventflow as the initial target.Key Changes:
ZeppelinEventBusclasszeppelin-zenginemodulezeppelin.eventbus.enabledfeature flagNotebookServer(implements NoteEventListener) to handleNoteRemoveEventviaZeppelinEventBuswhenzeppelin.eventbus.enabledis enabledWhat type of PR is it?
Improvement
Todos
What is the Jira issue?
How should this be tested?
Screenshots (if appropriate)
Questions: