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

Install cli with pip #38

Merged
merged 4 commits into from
Oct 10, 2024
Merged

Install cli with pip #38

merged 4 commits into from
Oct 10, 2024

Conversation

gadomski
Copy link
Collaborator

What I am changing

  • Add the ability to install the CLI from PyPI (not just cargo)

How I did it

  • Refactored the CLI code so we had a lib to use from Python
  • Broke the main lib (again) to remove Error::BoonCompile, which wasn't Send and therefore couldn't be used with anyhow. Not a big deal because the only place that error variant was used is on validator instantiation, and the cql2 json-schema should never ship broken.
  • Had to do a quick backflip to stop Python from grabbing an interrupt during (e.g.) typing on standard input, 🙇🏼 Cannot stop rust program while running python code PyO3/pyo3#3218
  • Added --version to CLI, forgot to do that before

How you can test it

$ maturin develop -m python/Cargo.toml --uv 1>/dev/null 2>/dev/null && which cql2 && cql2 --version
/Users/gadomski/Code/developmentseed/cql2-rs/.venv/bin/cql2
cql2-cli 0.1.0

Related Issues

@gadomski gadomski requested a review from bitner October 10, 2024 12:54
@gadomski gadomski self-assigned this Oct 10, 2024
@gadomski gadomski force-pushed the issues/33-install-cli-with-pip branch from fb4ec61 to 257f647 Compare October 10, 2024 12:57
@gadomski gadomski mentioned this pull request Oct 10, 2024
@gadomski gadomski force-pushed the issues/33-install-cli-with-pip branch from 257f647 to a1a19bd Compare October 10, 2024 13:31
@kylebarron
Copy link
Member

In the case when you have other PRs that are built on an initial PR, I find it easiest to review if you set the other branch as the target of this PR. Then we can start to review, and the target branch changes automatically to main when you merge the underlying base PR.

@gadomski gadomski marked this pull request as ready for review October 10, 2024 16:35
@gadomski
Copy link
Collaborator Author

In the case when you have other PRs that are built on an initial PR, I find it easiest to review if you set the other branch as the target of this PR. Then we can start to review, and the target branch changes automatically to main when you merge the underlying base PR.

Cool, thanks! Yeah wasn't sure what The Practice was around these parts 🤠

@kylebarron
Copy link
Member

I don't think it's "the practice" I think it's just me, and you don't have to follow me 😄

@gadomski
Copy link
Collaborator Author

Oh no worries, I chronically under-use github features so often break workflows without even knowing.

@gadomski gadomski merged commit 1d67fe2 into main Oct 10, 2024
18 checks passed
@gadomski gadomski deleted the issues/33-install-cli-with-pip branch October 10, 2024 17:54
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.

Enable CLI installation via pip
3 participants