[ZEPPELIN-6340] Add ZeppelinEventBus and update NotebookServer to handle NoteRemoveEvent - #5441
[ZEPPELIN-6340] Add ZeppelinEventBus and update NotebookServer to handle NoteRemoveEvent#5441seung-00 wants to merge 1 commit into
Conversation
73168af to
4ff5711
Compare
54049f1 to
58118cb
Compare
…dle NoteRemovedEvent
58118cb to
058654a
Compare
tbonelee
left a comment
There was a problem hiding this comment.
A few thoughts on subscription registration, since 6341, 6346 and 6348 will likely follow this PR as their model.
observe(NoteEvent.class) receives every subtype of NoteEvent. Once 6348 adds NoteCreatedEvent, this subscription gets it too and falls through to the else branch in handleNoteEvent, logging "Unknown event type" for what is normal behavior. Each new event also means another ticket editing the same if-else chain.
The try/catch in the lambda is load-bearing in a way that is easy to miss. LambdaObserver disposes the subscription when the consumer throws and forwards the error to RxJavaPlugins.onError, which by default only reaches stderr, so a dead subscription leaves no trace in the logs.
Would something like subscribe(Class<T>, Consumer<T>) on EventBus work here? Subscribing by concrete type lets ofType() do the filtering instead of instanceof, and the try/catch sits in the bus once however many subscriptions there are. One caveat: Consumer would need to be io.reactivex.rxjava3.functions.Consumer, since java.util.function.Consumer cannot throw checked exceptions and handlers would go on catching IOException themselves.
What is this PR for?
This PR is the first step toward migrating Zeppelin’s listener-based event handling to an EventBus model, as outlined in the proposal. It sets up the EventBus infrastructure and migrates
NoteRemoveEventhandling to it.Key changes:
ZeppelinEventBusclasszeppelin-zenginemodulezeppelin.eventbus.enabledfeature flagNotebookServerto handleNoteRemoveEventthroughZeppelinEventBuswhen the feature flag is enabledThis PR supersedes #5085. The original branch had fallen behind the current codebase, and the implementation has since changed, so I opened a new PR.
What type of PR is it?
Improvement
Todos
What is the Jira issue?
How should this be tested?
Screenshots (if appropriate)
Questions: