Skip to content

Fix PersistentWatcher not working with NamespaceFacade #1262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 12, 2025

Conversation

kezhuw
Copy link
Member

@kezhuw kezhuw commented Apr 7, 2025

NamespaceFacade does not support getCuratorListenable while #520 use it to listen for CuratorEventType.CLOSING to fix CURATOR-729.

This commit exports CuratorFrameworkBase::client to retrieve underlying framework client to listen for for CuratorEventType.CLOSING.

Fixes #1259.

`NamespaceFacade` does not support `getCuratorListenable` while apache#520 use
it to listen for `CuratorEventType.CLOSING` to fix CURATOR-729.

This commit exports `CuratorFrameworkBase::client` to retrieve
underlying framework client to listen for for `CuratorEventType.CLOSING`.

Fixes apache#1259.
@kezhuw kezhuw requested a review from Copilot April 7, 2025 05:29
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

curator-recipes/src/main/java/org/apache/curator/framework/recipes/watch/PersistentWatcher.java:85

  • Directly casting the client to CuratorFrameworkBase could lead to a ClassCastException if the client is not an instance of CuratorFrameworkBase. Consider adding an instance check or clarifying that the client must be of that type.
((CuratorFrameworkBase) client).client().getCuratorListenable().addListener(((ignored, event) -> {

@kezhuw kezhuw requested review from eolivelli and tisonkun April 7, 2025 05:29
@kezhuw
Copy link
Member Author

kezhuw commented Apr 7, 2025

I think we probably could introduce Runnable listenEvent(CuratorListener listener) in long term to deprecate Listenable<CuratorListener> getCuratorListenable(). This way we gain more control over implementation. I see no semantics reason to forbide NamespaceFacade::getCuratorListenable.

Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of this! LGTM.

@kezhuw kezhuw merged commit bac8ba9 into apache:master Apr 12, 2025
10 checks passed
@kezhuw kezhuw deleted the fix-PersistentWatcher-NamespaceFacade branch April 12, 2025 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PersistentWatcher no longer be used with namespaced CuratorFramework instance
2 participants