Skip to content
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

Plugin event notification subscription wildcard support #6347

Conversation

rustyrussell
Copy link
Contributor

Subscribing to "" will now give you every event: I've added support to pyln-client and libplugin for this.

Note: you can tell which notification it was in Python by having an argument called "request" and looking at "request.method"

Suggested-by: @ShahanaFarooqui

@rustyrussell rustyrussell added this to the v23.08 milestone Jun 19, 2023
@ShahanaFarooqui
Copy link
Collaborator

Related Issue: #6340
Related PR: #6345

@ShahanaFarooqui
Copy link
Collaborator

@rustyrussell It throws error ValueError: No subscription for warning found., when I subscribed for all events with "".
Eg.

@plugin.subscribe("")
def subscription_handler(plugin, response, **kwargs):
    plugin.log("Hello")

@rustyrussell
Copy link
Contributor Author

Hmm, that will happen if you're not running the modified pyln-client? Make sure you're running under poetry shell, and maybe poetry install?

Copy link
Collaborator

@ShahanaFarooqui ShahanaFarooqui left a comment

Choose a reason for hiding this comment

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

It is now working as expected. Good to go.

Copy link
Collaborator

@ShahanaFarooqui ShahanaFarooqui left a comment

Choose a reason for hiding this comment

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

I would recommend switching the wildcard from an empty string to [""] format. This format aligns with common practice of using "" as a wildcard symbol and array will be helpful in future with selective subscriptions.

@rustyrussell
Copy link
Contributor Author

I have switched to * everywhere, testing now.

Requested-by: Shahana Farooqui @shahana
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: plugins can subscribe to all notifications using "*".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We fixed the others.  There are no fields, but this keeps it consistent.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `shutdown` notification contains `shutdown` object (notification consistency)
@rustyrussell rustyrussell force-pushed the guilt/plugin-notifier-generalization branch from 3bfbbf2 to c76b9d9 Compare July 10, 2023 22:25
@rustyrussell
Copy link
Contributor Author

Fixed up now notifications are object-wrapped in master.

@ShahanaFarooqui
Copy link
Collaborator

Ack c76b9d9

@rustyrussell rustyrussell merged commit ad592d8 into ElementsProject:master Jul 13, 2023
38 checks passed
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.

None yet

2 participants