[Feature][Java] Add java event listeners config#641
[Feature][Java] Add java event listeners config#641twosom wants to merge 9 commits intoapache:mainfrom
Conversation
Thanks for the review! @wenjin272 |
| mailboxExecutor.submit(() -> tryProcessActionTaskForKey(key), "process action task"); | ||
| } | ||
|
|
||
| notifyEventProcessed(event); |
There was a problem hiding this comment.
Hi, @twosom. I carefully reviewed the logic of processEvent. In this function, only an ActionTask is created and submitted. It's actual execution occurs in subsequent mailbox iterations. Therefore, merely moving notifyEventProcessed to the end of the function still remains inconsistent with the documentation of EventListener. After considering all factors, I think it's simpler to place it at the beginning and update the EventListener documentation accordingly.
There was a problem hiding this comment.
Thanks for the catch, @wenjin272
I missed the point that the actual execution happens in subsequent mailbox iterations. I agree that calling it at the beginning and updating the Javadoc is the best way to avoid inconsistency. I'll update the code and the documentation accordingly.
There was a problem hiding this comment.
Hi @wenjin272
I've updated the PR as suggested. Could you take another look? Thanks!
- Move notifyEventProcessed to the start of processEvent to ensure consistency with documentation. - Update EventListener Javadoc to clarify that listeners are notified when an event is received.
Linked issue: #640
Purpose of change
Added event listeners config in java
Tests
ok
API
Documentation
doc-neededdoc-not-neededdoc-included