From a1848f6aedf2bbe0d430338ca15d184d392a25f3 Mon Sep 17 00:00:00 2001 From: Michael Schmoock Date: Thu, 19 Sep 2024 10:46:10 +0200 Subject: [PATCH 1/4] pyln-client: adds description to methods via docstring The old `long_description` was removed and deprecated a while ago without adding a proper replacement for plugin developers. The getmanifest JSON that was to be used for that only knows `name` and `usage`. This PR adds an optional `description` parameter that will be filled with the methods docstring `__doc__` (if set). Example: @p.method("example") def some_method(...) """some description""" ... Changelog-Add: optional description paramter to Plugin.Method --- contrib/pyln-client/pyln/client/plugin.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/contrib/pyln-client/pyln/client/plugin.py b/contrib/pyln-client/pyln/client/plugin.py index c8e82a614f54..b800eaef96fa 100644 --- a/contrib/pyln-client/pyln/client/plugin.py +++ b/contrib/pyln-client/pyln/client/plugin.py @@ -48,14 +48,17 @@ class Method(object): """ def __init__(self, name: str, func: Callable[..., JSONType], mtype: MethodType = MethodType.RPCMETHOD, - deprecated: Union[bool, List[str]] = None): + deprecated: Union[bool, List[str]] = None, + description: str = None): self.name = name self.func = func self.mtype = mtype self.background = False self.deprecated = deprecated + self.description = description self.before: List[str] = [] self.after: List[str] = [] + self.description = description class RpcException(Exception): @@ -323,7 +326,8 @@ def convert_featurebits( def add_method(self, name: str, func: Callable[..., Any], background: bool = False, - deprecated: Optional[Union[bool, List[str]]] = None) -> None: + deprecated: Optional[Union[bool, List[str]]] = None, + description: str = None) -> None: """Add a plugin method to the dispatch table. The function will be expected at call time (see `_dispatch`) @@ -360,7 +364,7 @@ def add_method(self, name: str, func: Callable[..., Any], ) # Register the function with the name - method = Method(name, func, MethodType.RPCMETHOD, deprecated) + method = Method(name, func, MethodType.RPCMETHOD, deprecated, description) method.background = background self.methods[name] = method @@ -493,7 +497,8 @@ def decorator(f: Callable[..., None]) -> Callable[..., None]: def method(self, method_name: str, category: Optional[str] = None, desc: Optional[str] = None, long_desc: Optional[str] = None, - deprecated: Union[bool, List[str]] = None) -> JsonDecoratorType: + deprecated: Union[bool, List[str]] = None, + description: str = None) -> JsonDecoratorType: """Decorator to add a plugin method to the dispatch table. Internally uses add_method. @@ -502,7 +507,7 @@ def decorator(f: Callable[..., JSONType]) -> Callable[..., JSONType]: for attr, attr_name in [(category, "Category"), (desc, "Description"), (long_desc, "Long description")]: if attr is not None: self.log("{} is deprecated but defined in method {}; it will be ignored by Core Lightning".format(attr_name, method_name), level="warn") - self.add_method(method_name, f, background=False, deprecated=deprecated) + self.add_method(method_name, f, background=False, deprecated=deprecated, description=f.__doc__) return f return decorator @@ -936,6 +941,9 @@ def _getmanifest(self, **kwargs) -> JSONType: else: args.append("[%s]" % arg) + if method.description: + args.append("\n%s" % method.description) + methods.append({ 'name': method.name, 'usage': " ".join(args) From 185c5b570f0ae90bd3730ab567c5698bf1f15b2d Mon Sep 17 00:00:00 2001 From: Michael Schmoock Date: Thu, 19 Sep 2024 11:54:10 +0200 Subject: [PATCH 2/4] pyln-client: refactors usage string generation to reduce code duplication --- contrib/pyln-client/pyln/client/plugin.py | 66 ++++++++++++----------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/contrib/pyln-client/pyln/client/plugin.py b/contrib/pyln-client/pyln/client/plugin.py index b800eaef96fa..36cbaedd8644 100644 --- a/contrib/pyln-client/pyln/client/plugin.py +++ b/contrib/pyln-client/pyln/client/plugin.py @@ -58,7 +58,35 @@ def __init__(self, name: str, func: Callable[..., JSONType], self.description = description self.before: List[str] = [] self.after: List[str] = [] - self.description = description + + def get_usage(self): + # Handles out-of-order use of parameters like: + # + # ```python3 + # + # def hello_obfus(arg1, arg2, plugin, thing3, request=None, + # thing5='at', thing6=21) + # + # ``` + argspec = inspect.getfullargspec(self.func) + defaults = argspec.defaults + num_defaults = len(defaults) if defaults else 0 + start_kwargs_idx = len(argspec.args) - num_defaults + args = [] + for idx, arg in enumerate(argspec.args): + if arg in ('plugin', 'request'): + continue + # Positional arg + if idx < start_kwargs_idx: + args.append("%s" % arg) + # Keyword arg + else: + args.append("[%s]" % arg) + + if self.description is not None: + args.append("\n%s" % self.description) + + return " ".join(args) class RpcException(Exception): @@ -778,7 +806,7 @@ def _multi_dispatch(self, msgs: List[bytes]) -> bytes: return msgs[-1] - def print_usage(self): + def get_usage(self): import textwrap executable = os.path.abspath(sys.argv[0]) @@ -833,7 +861,7 @@ def print_usage(self): methods_header = None parts.append(method_tpl.format( - name=method.name, + name="%s %s" % (method.name, method.get_usage()), )) options_header = textwrap.dedent(""" @@ -868,8 +896,10 @@ def print_usage(self): default=opt.default, typ=opt.opt_type, )) + return "".join(parts) - sys.stdout.write("".join(parts)) + def print_usage(self): + sys.stdout.write(self.get_usage()) sys.stdout.write("\n") def run(self) -> None: @@ -918,35 +948,9 @@ def _getmanifest(self, **kwargs) -> JSONType: doc = "Undocumented RPC method from a plugin." doc = re.sub('\n+', ' ', doc) - # Handles out-of-order use of parameters like: - # - # ```python3 - # - # def hello_obfus(arg1, arg2, plugin, thing3, request=None, - # thing5='at', thing6=21) - # - # ``` - argspec = inspect.getfullargspec(method.func) - defaults = argspec.defaults - num_defaults = len(defaults) if defaults else 0 - start_kwargs_idx = len(argspec.args) - num_defaults - args = [] - for idx, arg in enumerate(argspec.args): - if arg in ('plugin', 'request'): - continue - # Positional arg - if idx < start_kwargs_idx: - args.append("%s" % arg) - # Keyword arg - else: - args.append("[%s]" % arg) - - if method.description: - args.append("\n%s" % method.description) - methods.append({ 'name': method.name, - 'usage': " ".join(args) + 'usage': method.get_usage() }) manifest = { From 405e7477b9161fa3d3e22a832e1ada3f42732dd5 Mon Sep 17 00:00:00 2001 From: Michael Schmoock Date: Fri, 20 Sep 2024 15:37:25 +0200 Subject: [PATCH 3/4] pyln-client: adds testcase for usage in manifest and print_usage --- contrib/pyln-client/tests/test_plugin.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/contrib/pyln-client/tests/test_plugin.py b/contrib/pyln-client/tests/test_plugin.py index 9393269a5efd..9493a20cc78b 100644 --- a/contrib/pyln-client/tests/test_plugin.py +++ b/contrib/pyln-client/tests/test_plugin.py @@ -435,3 +435,22 @@ def test4(request): ba = p._bind_kwargs(test4, {}, req) with pytest.raises(ValueError, match=r'current state is RequestState\.FINISHED(.*\n*.*)*MARKER4'): test4(*ba.args) + + +def test_usage(): + p = Plugin(autopatch=False) + + @p.method("some_method") + def some_method(some_arg: str = None): + """some description""" + pass + + manifest = p._getmanifest() + usage = p.get_usage() + + assert manifest['rpcmethods'][0]['name'] == 'some_method' + assert "some_arg" in manifest['rpcmethods'][0]['usage'] + assert "some description" in manifest['rpcmethods'][0]['usage'] + assert "some_method" in usage + assert "some_arg" in usage + assert "some description" in usage From 8fd7bba65becac786be8ed5a5ce687401ce9f87e Mon Sep 17 00:00:00 2001 From: Michael Schmoock Date: Fri, 20 Sep 2024 17:39:59 +0200 Subject: [PATCH 4/4] pytest: fix a test that broke because of docstring usage --- tests/test_plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 725a2d638eab..fbbb6d59e48c 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -196,7 +196,7 @@ def test_rpc_passthrough(node_factory): assert(len(cmd) == 1) # Make sure usage message is present. - assert only_one(n.rpc.help('hello')['help'])['command'] == 'hello [name]' + assert only_one(n.rpc.help('hello')['help'])['command'].startswith('hello [name]') # While we're at it, let's check that helloworld.py is logging # correctly via the notifications plugin->lightningd assert n.daemon.is_in_log('Plugin helloworld.py initialized')