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

Cln version manager #380

Merged
merged 10 commits into from
Mar 31, 2024
Merged

Conversation

ErikDeSmedt
Copy link
Collaborator

I've introduced cln_version_manager.

It is currently very hard to run gl-testing in standalone-mode.
One of the dependencies of gl-testing are a wide range of Core Lightning versions.
These files are packaged into the docker-file.

I've adapted the gl-testing python-module to download the Core Lightning binaries for the user.
It will download them to XDG_CACHE_DIR by default but users can configure their folder.

@ErikDeSmedt ErikDeSmedt force-pushed the cln_version_manager branch 10 times, most recently from 13f76fe to 6f1f298 Compare March 9, 2024 08:47
@ErikDeSmedt ErikDeSmedt marked this pull request as ready for review March 9, 2024 08:47
@ErikDeSmedt
Copy link
Collaborator Author

This PR is ready for review. It includes

  • cln_version_manager which can download the required Core Lightning binaries
  • a build pipeline that tests cln_version_manager

It doesn't include

  • modifications to gl-testing to use cln_version_manager. I've created a dependency because I moved the NodeVersion-class
  • modifications to the Dockerfile to use cln_version_manager

These changes will follow later in a separate PR

@ErikDeSmedt
Copy link
Collaborator Author

Pipelines will execute successfully once I've pulished cln-version-manager on PyPI

@cdecker cdecker marked this pull request as draft March 11, 2024 14:09
@ErikDeSmedt ErikDeSmedt force-pushed the cln_version_manager branch 3 times, most recently from b2972f3 to 7f1a44c Compare March 28, 2024 12:37
@ErikDeSmedt ErikDeSmedt marked this pull request as ready for review March 28, 2024 12:54
@ErikDeSmedt
Copy link
Collaborator Author

This branch introduce cln-version-manager.

To test it you can use clnvm get-all which will download all versions of cln which greenlight requires.

I did not modify gl-testing to use these versions yet. This work will follow in another PR.

@ErikDeSmedt ErikDeSmedt force-pushed the cln_version_manager branch from 7f1a44c to 25cfb2b Compare March 28, 2024 16:07
@cdecker
Copy link
Collaborator

cdecker commented Mar 28, 2024

I didn't test this yet (on my phone) but aside from a very minor typo (roup -> group) in d374986 this looks great. Not sure the CI lint would even complain about that typo.

assert not request_mock.get.called

# get them again using force to ensure we do download them
vm.get_all(force=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can probably be moved within the with so assert request_mock.get.called can be called to verify the force flag was respected

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Setting this up is more a bit more annoying.

Mocking requests.get requires to return a valid request object which returns a 200 - status code and a working lightningd-release in the payload.

I'd prefer not to waste a few hours in creating this test.

Copy link
Collaborator

Choose a reason for hiding this comment

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

With no assertions to show vm.get_all(force=True)actually downloads the versions when the directory already exists, I say we remove it from the test to reduce test times

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is probably a better idea.

@ErikDeSmedt ErikDeSmedt force-pushed the cln_version_manager branch 2 times, most recently from 215e6bf to 1392579 Compare March 29, 2024 09:58
@ErikDeSmedt ErikDeSmedt requested review from Randy808 and nepet March 29, 2024 10:21
@cdecker
Copy link
Collaborator

cdecker commented Mar 29, 2024

Testing this out it seems eerily quiet. It looks like we need to configure the logging package to print things to stdout otherwise the CLI is completely silent (and I aborted a number of times thinking it might be stuck).

@ErikDeSmedt
Copy link
Collaborator Author

Testing this out it seems eerily quiet. It looks like we need to configure the logging package to print things to stdout otherwise the CLI is completely silent (and I aborted a number of times thinking it might be stuck).

Question: Are you on very crappy internet and is the download of cln taking very long?

When I do clnvm get-all it takes a few second until the first version is downloaded.
It outputs the path to lightnignd for all downloaded versions. In total this takes 20 seconds.

/opt/clnvm/928f09a46c707f68c8c5e1385f6a51e10f7b1e57c5cef52f5b73c7d661500af5/v0.10.1/usr/local/bin/lightningd
/opt/clnvm/c323f2e41ffde962ac76b2aeaba3f2360b3aa6481028f11f12f114f408507bfe/v0.10.2/usr/local/bin/lightningd
/opt/clnvm/0f1a49bb8696db44a9ab93d8a226e82b4d3de03c9bae2eb38b750d75d4bcaceb/v0.11.0.1/usr/local/bin/lightningd
/opt/clnvm/b15866b7beea239aaf4e38931fe09ee85bf2e58ad61c2ec79b83bb361364bf97/v0.11.2gl2/usr/local/bin/lightningd
/opt/clnvm/95209242d8ddc4879b959fb5e4594b4d2dcf7bac7227ec7c421ab05019de8633/v0.11.2/usr/local/bin/lightningd
/opt/clnvm/40b6c50babdc74d9fd251816efa46de0c12cac88d72e0c7b02457a8949d2690b/v22.11gl1/usr/local/bin/lightningd
/opt/clnvm/e1a57a8ced59fd92703fad8e34926c014b71ee0c13cc7f863cb18b2ca19a58b9/v23.05gl1/usr/local/bin/lightningd
/opt/clnvm/700e2f4765ba2f8d83ea26e0dc5b47c7176b665c7d6945bdfed2207c90ed9ebd/v23.08gl1/usr/local/bin/lightningd

I prefer to not route all logging through stdout in the default setting.
It is quite easy to create simple scripts using the current output because each line has a clear meaning.

Randy808
Randy808 previously approved these changes Mar 29, 2024
@cdecker
Copy link
Collaborator

cdecker commented Mar 29, 2024

Testing this out it seems eerily quiet. It looks like we need to configure the logging package to print things to stdout otherwise the CLI is completely silent (and I aborted a number of times thinking it might be stuck).

Question: Are you on very crappy internet and is the download of cln taking very long?

When I do clnvm get-all it takes a few second until the first version is downloaded.
It outputs the path to lightnignd for all downloaded versions. In total this takes 20 seconds.

/opt/clnvm/928f09a46c707f68c8c5e1385f6a51e10f7b1e57c5cef52f5b73c7d661500af5/v0.10.1/usr/local/bin/lightningd
/opt/clnvm/c323f2e41ffde962ac76b2aeaba3f2360b3aa6481028f11f12f114f408507bfe/v0.10.2/usr/local/bin/lightningd
/opt/clnvm/0f1a49bb8696db44a9ab93d8a226e82b4d3de03c9bae2eb38b750d75d4bcaceb/v0.11.0.1/usr/local/bin/lightningd
/opt/clnvm/b15866b7beea239aaf4e38931fe09ee85bf2e58ad61c2ec79b83bb361364bf97/v0.11.2gl2/usr/local/bin/lightningd
/opt/clnvm/95209242d8ddc4879b959fb5e4594b4d2dcf7bac7227ec7c421ab05019de8633/v0.11.2/usr/local/bin/lightningd
/opt/clnvm/40b6c50babdc74d9fd251816efa46de0c12cac88d72e0c7b02457a8949d2690b/v22.11gl1/usr/local/bin/lightningd
/opt/clnvm/e1a57a8ced59fd92703fad8e34926c014b71ee0c13cc7f863cb18b2ca19a58b9/v23.05gl1/usr/local/bin/lightningd
/opt/clnvm/700e2f4765ba2f8d83ea26e0dc5b47c7176b665c7d6945bdfed2207c90ed9ebd/v23.08gl1/usr/local/bin/lightningd

I prefer to not route all logging through stdout in the default setting.
It is quite easy to create simple scripts using the current output because each line has a clear meaning.

Indeed that'll be the issue, hotel speeds from the analog times I guess 😉

ErikDeSmedt and others added 10 commits March 31, 2024 18:01
We do `poetry install` here.
This ensures that `clnvm` is installed and can be used in the container
Co-authored-by: Randy808 <[email protected]>
`merge_group` instead of `merge_roup`

Co-Authored-By: Christian Decker <@cdecker>
When a user requests a path to lightningd they might want

- to run the executable. The user would need `node_version.lightningd` which contains the full
  path to the executable
- to add the executable to the path. The user would need
  `node_version.bin_path` which contains the path to the folder
  containing `lightningd`.
- to create a symlink to a known place. The user would need
  `node_version.root_path` which is at the root of the release.
  Typically, `root_path/usr/local/bin/lightningd` contains the
  executable.

This commit handles all these cases more explicitly.

I've also edited `gl-testing` to use the `lightningd`-field instead.
This ensures that users who pip install the package get working
type-hints
Adding `gprcio-tools` as a dependency ensures `poetry install` works.
Adding `cln-version-manager` as a dev-dependency ensures `poetry
install` works and that we use an editable install
Dropping the lines make tests faster.
We don't lose much coverage. We aren't even asserting that setting
force to true has the desired effect
@cdecker cdecker force-pushed the cln_version_manager branch from 0deddb0 to 57ae9b9 Compare March 31, 2024 16:02
@cdecker cdecker enabled auto-merge (rebase) March 31, 2024 16:02
@cdecker cdecker merged commit 630b19f into Blockstream:main Mar 31, 2024
19 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.

3 participants