Skip to content

Python dependencies are missing in release binaries #7665

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

Open
ShahanaFarooqui opened this issue Sep 13, 2024 · 10 comments
Open

Python dependencies are missing in release binaries #7665

ShahanaFarooqui opened this issue Sep 13, 2024 · 10 comments

Comments

@ShahanaFarooqui
Copy link
Collaborator

ShahanaFarooqui commented Sep 13, 2024

Currently, we are not packaging Python plugins (clnrest & wssproxy) dependencies with the release binaries. To resolve this, we can consider the following options:

Option 1: Use PyInstaller

Convert the plugin into a standalone executable using PyInstaller. This approach bundles the Python interpreter and all required libraries into a single folder or executable file, making distribution easier.

Option 2: Bundle Python Libraries with Binaries

Bundle dist-packages for CLN with the binaries as they are already being installed with && pip install -r requirements.txt \ command in Dockerfile. files.**

Option 3: Bundle requirements.txt with Binaries

Include the requirements.txt with the binaries and install the dependencies after extraction. The installation process would look something like this:

sudo tar -xvf clightning-v24.08-Ubuntu-24.04.tar.xz -C /usr/local --strip-components=2 \
&& sudo pip3 install -r /usr/local/python/requirements.txt \
&& sudo rm -rf /usr/local/python

For more complex future installation steps, we could convert it into a shell script (e.g., install.sh).

#!/bin/bash

# Extract the tar.xz file and strip components
sudo tar -xvf clightning-v24.08-Ubuntu-24.04.tar.xz -C /usr/local --strip-components=2

# Install Python dependencies
sudo pip3 install -r /usr/local/python/requirements.txt

# Delete the python directory after installation
sudo rm -rf /usr/local/python

Note to self: If we choose this option, remember to remove && pip install -r requirements.txt \ from Dockerfile. files.

Open to Other Suggestions:

Feel free to share your thoughts if there is a better solution.

@vincenzopalazzo
Copy link
Collaborator

Due that we already have rust as a build dependencies, why not use the alternative rust plugin that @daywalker90 made for cln rest?

IMHO this should be a good alternative, and we stop required python dependencies to just run vanilla cln

@dfgjknhgcrj
Copy link

pip install pyinstaller

@dfgjknhgcrj
Copy link

&& pip install -r requirements.txt \henry

@king-11
Copy link
Contributor

king-11 commented Oct 13, 2024

I also agree with @vincenzopalazzo packaging and dependency management in python seems like a mess a rust binary plugin just might be a great solution.

From the provided options option 1 seems fair but bundling python interpreter won't that bind the interpreter selection instead of allowing user to choose which one he wants in his system a might just be an extra bloat for lean docker containers.

option 2 & 3 utilize requirements.txt and it seems to be the case that they are discouraged nowadays and was the reason for removing them from cln plugins as well.

@cdecker
Copy link
Member

cdecker commented Oct 17, 2024

Quick idea to get the python-based plugins to behave a bit better again: uv inline dependency declarations.

I have been using uv for a while, and it's much simpler to use than poetry and all the rest, since it a) ignores the system python installation if any, b) it manages its own python binaries, and c) it is incredibly fast at building virtualenvs on the fly, obviating the need to even keep them around.

A plugin with the shebang #!/usr/bin/env -S uv run only requires that the uv executable is somewhere on the path, and then we can specify dependencies in the script itself like this:

# /// script
# requires-python = ">=3.12"
# dependencies = [
#     "flask==3.*",
# ]
# ///

Being invoked with uv run from the env this will be parsed, and uv will spin up a virtualenv with the exact dependency versions specified in the script (and transitive dependencies), and then invoke the script from that virtualenv.

So the management of dependencies is done at startup every time, and there is no drift. Only open question is whether we can make uv silent enough so it doesn't interfere with the stdout / stdin based protocol between lightningd and the plugins

@rustyrussell
Copy link
Contributor

Switching to uv seems sensible (as does switching to the rust cln-rest, but that's independent). Using uv for reckless as well would be nice, rather than the current venv wrapper hack.

For this milestone, we should simply include the requirements.txt in the tarfile. It should be in something like $(pkglibexecdir)/python-requirements.txt. As long as the plugins fail gracefully if their dependencies are not installed (and they do, I believe), we're good.

rustyrussell added a commit to rustyrussell/lightning that referenced this issue Nov 24, 2024
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

See-also: ElementsProject#7665
Signed-off-by: Rusty Russell <[email protected]>
vincenzopalazzo pushed a commit to rustyrussell/lightning that referenced this issue Nov 24, 2024
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

See-also: ElementsProject#7665
Changelog-None: Makefile: generate and install requirements.txt files for Python plugins.
Co-Developed-by: Vincenzo Palazzo <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell modified the milestones: v24.11, v25.02 Dec 13, 2024
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Feb 6, 2025
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

See-also: ElementsProject#7665
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Feb 6, 2025
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

Changelog-Added: Install: we now supply requirements.txt file for Python plugin (i.e. wss-proxy).
See-also: ElementsProject#7665
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Feb 6, 2025
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

Changelog-Added: Install: we now supply requirements.txt file for Python plugin (i.e. wss-proxy).
See-also: ElementsProject#7665
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Feb 12, 2025
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

Changelog-Added: Install: we now supply requirements.txt file for Python plugin (i.e. wss-proxy).
See-also: ElementsProject#7665
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Feb 12, 2025
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

Changelog-Added: Install: we now supply requirements.txt file for Python plugin (i.e. wss-proxy).
See-also: ElementsProject#7665
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Feb 12, 2025
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

Changelog-Added: Install: we now supply requirements.txt file for Python plugin (i.e. wss-proxy).
See-also: ElementsProject#7665
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Feb 12, 2025
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

Changelog-Added: Install: we now supply requirements.txt file for Python plugin (i.e. wss-proxy).
See-also: ElementsProject#7665
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Feb 12, 2025
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

Changelog-Added: Install: we now supply requirements.txt file for Python plugin (i.e. wss-proxy).
See-also: ElementsProject#7665
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Feb 13, 2025
…ins.

Doesn't automate the install, but at least provides some tools for users
to install the requirements on their systems

Changelog-Added: Install: we now supply requirements.txt file for Python plugin (i.e. wss-proxy).
See-also: ElementsProject#7665
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell modified the milestones: v25.02, v25.05 Feb 17, 2025
@king-11
Copy link
Contributor

king-11 commented Apr 13, 2025

@rustyrussell @cdecker I can look into migrating to uv are we all in on that, the changes I see would be:

  • changes to pyproject.toml for supporting generic instead of poetry
  • changes in ci to use uv
  • makefile updates to use uv (not entirely sure on that)
  • reckless update to use uv for venv setup

@cdecker
Copy link
Member

cdecker commented Apr 13, 2025

That'd be amazing. I have been planning to go for it for a bit, however things keep coming up 😉
Your plan looks solid, I just wanted to add that if the host machine has uv installed then we can use the script mode in uv to skip the need for manual venv management completely:

uv supports inline dependencies in scripts (PEP-724) and setting the shebang to be uv itself it'll create and initialize the venv with the required dependencies on the fly, no need to manually do anything. Also every plugin gets its own venv, so version conflicts are a thing of the past.

If reckless were to recognize uv plugins it could skip a whole lot of its own logic, deferring to uv.

@king-11
Copy link
Contributor

king-11 commented Apr 14, 2025

Sounds good @cdecker I think the part about changing inlining scripts I can take up in a follow up PR just to keep the scope for first one limited given it would be a large change I will focus on ci and reckless changes first and then inline dependencies.

@cdecker
Copy link
Member

cdecker commented Apr 20, 2025

Sounds great, adding a follow-up PR was what I was hoping for, I don't want to hold up things too much with my bike shedding ideas 😋

But we seem to be very much aligned 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants