-
Notifications
You must be signed in to change notification settings - Fork 54
Adds version, fixes stop all bug, extra feedback when user select "n" for custom network #683
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
Adds version, fixes stop all bug, extra feedback when user select "n" for custom network #683
Conversation
pinheadmz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
concept ACK, see docs/developer-notes.md for instructions on passing lint test
docs/warnet.md
Outdated
| Check the version of warnet using `warnet version`. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API docs are autogenerated by resources/scripts/apidocs.py which you can run locally to make sure the output looks correct. The CI will run this automatically on merged PRs so docs on main should always be up to date ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks!
I had messed up my trunk setup so I was getting no linting being done locally
|
Yeah, its annoying ... --> git diff diff --git a/src/warnet/main.py b/src/warnet/main.py
index 08d46809..b6250732 100644
--- a/src/warnet/main.py
+++ b/src/warnet/main.py
@@ -19,6 +19,7 @@ from .users import auth
def cli():
pass
+
@click.command()
def version() -> None:
"""Display the installed version of warnet"""
@@ -28,6 +29,7 @@ def version() -> None:
except importlib.metadata.PackageNotFoundError:
click.echo("warnet version unknown (package not installed)")
+
cli.add_command(admin)
cli.add_command(auth)
cli.add_command(bitcoin)
diff --git a/src/warnet/project.py b/src/warnet/project.py
index cddbab6a..badc5aa8 100644
--- a/src/warnet/project.py
+++ b/src/warnet/project.py
@@ -432,7 +432,9 @@ def new_internal(directory: Path, from_init=False):
click.secho("\nGenerating custom network...", fg="yellow", bold=True)
custom_network_path = inquirer_create_network(directory)
else:
- click.echo(f"No custom network specified, see example network files in {project_path}/networks/")
+ click.echo(
+ f"No custom network specified, see example network files in {project_path}/networks/"
+ )
click.echo("Deploy any of these networks by running:")
click.echo(f" warnet deploy {project_path}/networks/<example-network-name>")
|
|
Oh man, thanks for your patience. 2 learnings:
|
pinheadmz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 2aab33c
Happy to merge this as-is, but wondering if we can squeeze in one more detail about version ...
Show Signature
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK 2aab33c53d14bf005b7583ef21f75d0c2c47c123
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAme18ysACgkQ5+KYS2KJ
yTqbTw//fG4zKng7x0dMjnfqZibTRqy4q3VqC9uKmhJYRh1CmwqkvNfQwOiND2Wi
kl17B/y+3U4dwNzNaOFnYcHx7bPWmEKNVuy30Jo4Zk3Rzus495Zn5Uj+cmpwJvch
R3phZQ2uWQiOStYsKPGglJFKw9nUXV7rQcO28q6CKOr+3GU/Etcp8/GJnm9ZE/XK
VmpfIjFcozDBS4J0dQ8RyVfQko6anVoiJk6D7c+XoPCzh0rcLkYihtzTZzRoq0nV
xhC+G9D0t2Tnm20cjKWZ3Y7+snP9gkEEnKrL0F0WfsUece2SBc31lz4jJ4ms7wL3
JL+A13HXvLMhlbNRQ7ULDGhrwOoFqSxDKLXLNjrSe3SvwvHcXR3DkSNnDMavWlDy
4OUNHvadpdsMFRo/FCoVPrhUs6PyLHXPAh71OwxEqkcYkE5IkRC4O+HCYU9GS6Kz
8zFDrkfYFZREkXKBImebbAs3OCtCA3FbIokQx/lQyaomc3HluObdEYgYmTNJ5MGX
m+kCxQ7KrCU9a9o7Fg5PpMETnXV9DpNOY2eG94QNJwG+BDC9WdbCnU39UB0thNtv
6QQSL9NYzmr7NSFZUHr+0oc9i1aUciUrSDPe+F8Uu+q8tCbfCmD6LprydSthiH3w
8pYb0uY6ehPO/xq8LShIKuW9STMJzL2yeCQ10JCfoKZkVFVtX6k=
=0Ngg
-----END PGP SIGNATURE-----
pinheadmz's public key is on openpgp.org
src/warnet/main.py
Outdated
| """Display the installed version of warnet""" | ||
| try: | ||
| version = importlib.metadata.version("warnet") | ||
| click.echo(f"warnet version {version}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be possible to indicate if the code is modified from release, like
Bitcoin Core daemon version v28.99.0-78c19ddd7c56-dirty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'll look into how this is done 💪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok there are a few options here so I'll need your guidance.
Using setuptools_scm seems to be the way to go for a Python-based project like this.
We have 2 ways to install Warnet, from source and from pypi.
If the user is installing from source, then the basic (default) setuptools_scm configuration I've added will yield versions like follows:
warnet version 1.1.11.dev8+dirty. I've confirmed this locally. This is using the "guess-next-dev" version-scheme.
If the user is installing from pypi then the reported version should report the tagged version: warnet version 1.1.11. I haven't been able to confirm this but this is basically what setuptools_scm is intended to do.
Now the "dirty" version suffix of the current configuration doesn't have the git commit details like Bitcoin Core, e.g. --short=12 HEAD, instead it has dev<number-of-commits-from-tag>:
warnet version 1.1.11.dev8+dirty
rather than
warnet version 1.1.11.e24df9e566fd-dirty
To instead use the git short commit in some way we would have to use a custom local version which can absolutely be done, my concern is that it might make things a bit more fragile without providing much additional utility.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pinheadmz I fixed the Ruff issues 🫠
|
Ok @pinheadmz I have a potential solution that is now pushed to the branch/PR. The situation is a bit more complex than originally anticipated as the following indicates, but we get the following outcome:
The complexity in implementation is related to the package build sequence, and what is possible with
We need a So we have a hybrid approach:
|
pinheadmz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok sorry this had gotten bike shedded 😬
here's what im thinking:
- try to get git commit
- always get scm_version
- if we got a commit, append it to whatever we got from scm_version
so, installing from release:
warnet 1.0.11
installing from github but checking out the exact 1.0.11 tag:
warnet 1.0.11-296e0ba2
.gitignore
Outdated
| @@ -1,4 +1,5 @@ | |||
| .vscode | |||
| .DS_Store | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is fine, but FYI you can add this to your own ~/.gitignore and it'll apply to everything ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A global .gitignore, who woulda thought 😝
1fb68aa to
a5bf800
Compare
a5bf800 to
27af3a7
Compare
|
Hey @pinheadmz it now does the below. No longer a need for I've been testing with test PyPI as you recommended (e.g.
|
|
Ok we're so close ;-) Issue now is that the commit hash appended to the version string is from whatever repo you are currently working in. For example cd to any git repo on your computer, create venv, install your test-pypi warnet and execute |
|
at this point I am ok with just dropping all the commit hash stuff and just merging this so we at least have something. |
Ahh frick, yeah we can't have that 😓 |
|
Ok take #12123721347 No subprocess & wrong git repo, no complexity with setuptools_scm Just use the metadata setuptools_scm provides, which includes
The main limitation is that it won't automatically update the commit hash when a user makes a local commit without reinstalling, because setuptools_scm only writes the version info to P.S. We use |
|
GREAT thank, sorry for all the hassle. <3 |
Adds version:
Stop all bug -
multiprocessingdoesn't like local functionsFixed:
Output from "n" on
warnet new: