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

Reckless: add rust installer and json output #7484

Merged
merged 16 commits into from
Aug 8, 2024

Conversation

endothermicdev
Copy link
Collaborator

This PR does several things:

  1. Adds an installer for rust plugins.
  2. Adds a --version flag.
  3. Refactors logging and output.
  4. Allows option flags in any order. (previously limited to before subcommands only)
  5. Adds a --json output option.

In general, this prepares for a reckless-rpc plugin coming next.

@endothermicdev endothermicdev added this to the v24.08 milestone Jul 22, 2024
@endothermicdev endothermicdev force-pushed the reckless-upgrade branch 5 times, most recently from 7d303f8 to be3f0fe Compare July 30, 2024 19:23
@endothermicdev endothermicdev marked this pull request as ready for review July 30, 2024 20:13
@endothermicdev endothermicdev force-pushed the reckless-upgrade branch 3 times, most recently from 678f522 to 23d2b60 Compare August 6, 2024 01:12
@ShahanaFarooqui ShahanaFarooqui self-requested a review August 6, 2024 18:34
@endothermicdev endothermicdev force-pushed the reckless-upgrade branch 2 times, most recently from 53e7cdf to 905c3cc Compare August 6, 2024 21:46
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.

  • Search without result does not show any message, for example: reckless search nonexistingplugin should show a message stating that plugin not found/plugin nonexistingplugin not found

  • holdinvoice install command throws below error (reckless --regtest --verbose --json install holdinvoice)

Traceback (most recent call last):
  File "/usr/local/bin/reckless", line 1803, in <module>
    log.add_result(args.func(target))
  File "/usr/local/bin/reckless", line 1314, in install
    installed = _install_plugin(src)
  File "/usr/local/bin/reckless", line 1245, in _install_plugin
    staged_src = INSTALLER.dependency_call(staged_src)
  File "/usr/local/bin/reckless", line 936, in cargo_installation
    log.error(cargo.stderr.read())
AttributeError: 'str' object has no attribute 'read'
  • Can reckless install accept plugin options too, like summars-columns, summars-sort-by with reckless install summars?

tools/reckless Outdated Show resolved Hide resolved
tools/reckless Outdated Show resolved Hide resolved
tools/reckless Outdated Show resolved Hide resolved
@endothermicdev
Copy link
Collaborator Author

endothermicdev commented Aug 7, 2024

Search without result does not show any message

Good catch, I changed the default logging level and missed changing this one to logging.info fixed!

holdinvoice install command throws below error

Ah, right, I changed the call from Popen to run, which has the outputs strings. Also raised the right error to catch here.

The python mimetype package wasn't useful enough in indentifying
filetypes anyhow.
rather than just source/.  This is required for cargo install, so let's just
use this paradigm globally.
@endothermicdev
Copy link
Collaborator Author

Can reckless install accept plugin options too, like summars-columns, summars-sort-by with reckless install summars?

I like this idea. We could pass these in a list to an optional keyword, and then maybe track the options in the metadata. Is there a good way to validate the plugins options though? Any errors will cause the node to not start.

This follows the same structure that enables python virtual environments:
  reckless/
    <plugin_name>/
      <symlink to compiled bin>
      source/
        <clone of original source plugin dir>/

Changelog-Added: Reckless: added the ability to install rust plugins.
Changelog-Changed: Reckless option flags are now position independent.
This will allow redirection of json output in the following commits.
Also redirect config creation prompts to stderr in order to not interfere
with json output on stdout.

Changelog-Added: reckless provides json output with option flag -j/--json
This more easily allows list output for commands accepting list
input, i.e., installing 3 plugins produces 3 outputs.
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack 87895bd

@rustyrussell rustyrussell merged commit 48258ec into ElementsProject:master Aug 8, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants