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

pyln: Add a dynamic configs and a callback for changes #7289

Merged

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented May 3, 2024

This is an alternative to #7288, that uses a callback per config
option rather than hooking the setconfig function entirely. This
allows us to react to individual config options changing, while still
maintaining the default setconfig behavior.

@cdecker cdecker force-pushed the 202405-pyln-setconfig-condition branch from 3a54f18 to c10e1a9 Compare May 3, 2024 16:47
@cdecker
Copy link
Member Author

cdecker commented May 3, 2024

I'd love feedback from @rustyrussell and @CGuida on this since it is an alternative to #7288

@cdecker cdecker force-pushed the 202405-pyln-setconfig-condition branch from a80ef6c to e23bc63 Compare May 3, 2024 23:16
@rustyrussell rustyrussell force-pushed the 202405-pyln-setconfig-condition branch from e23bc63 to d7ea1c7 Compare May 9, 2024 06:22
@rustyrussell
Copy link
Contributor

rustyrussell commented May 9, 2024

Fixed commit message on "fix..." commit.

@rustyrussell rustyrussell reopened this May 9, 2024
@rustyrussell rustyrussell force-pushed the 202405-pyln-setconfig-condition branch from d7ea1c7 to 0df80c6 Compare May 10, 2024 05:26
@rustyrussell
Copy link
Contributor

Fixed 'default: null' which cln doesn't like on flag values (if you specify a default, it must be false).

@cdecker cdecker force-pushed the 202405-pyln-setconfig-condition branch from 0df80c6 to 6cc8d43 Compare May 23, 2024 10:47
@rustyrussell rustyrussell force-pushed the 202405-pyln-setconfig-condition branch from 6cc8d43 to 2b76ae3 Compare June 20, 2024 01:12
@rustyrussell
Copy link
Contributor

Trivial rebase on master to re-run tests, and move one part of commit to make flake8 happy with each commit.

Ack 2b76ae3

rustyrussell and others added 4 commits June 20, 2024 12:38
We didn't actually *change* the value you'd see, when we got a setconfig call!

Changelog-Added: pyln-client: implement setconfig hook for plugins so you can see changes in `dynamic` options.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I was looking into using the `threading.Condition` but since we're
already rather heavily using callbacks, this allows us to stay
single-threaded, and not having to completely hook the `setconfig`
function.

Changelog-Added: pyln-client: Added a notification mechanism for config changes
And don't set the value unless it passes.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the 202405-pyln-setconfig-condition branch from 2b76ae3 to 7f1226c Compare June 20, 2024 03:55
@rustyrussell
Copy link
Contributor

Oh, we broke our own plugins which use plugin.option['optname']['value'] instead of get_value like I guess they're supposed to. Fix that so we don't break other plugins!

cdecker and others added 2 commits June 20, 2024 13:54
[ Fix not to include 'value' and 'default' (if None) in getmanifest response --RR ]
[ Fix to support [] operator for existing plugins (including our test ones!) --RR ]
Noticed as I was debugging.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the 202405-pyln-setconfig-condition branch from 7f1226c to fa8b6aa Compare June 20, 2024 04:24
@rustyrussell rustyrussell merged commit e21b70c into ElementsProject:master Jun 20, 2024
35 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.

2 participants