-
Notifications
You must be signed in to change notification settings - Fork 276
Package polaris client as python package #2049
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
Conversation
@eric-maynard / @HonahX may I have a review here? This will get us a proper packaging for polaris client as well as to help us to remove dependency on the bash script (and use it only for repair or initial setup) |
Thanks @MonkeyCanCode for working on this! This is a great step towards polaris client packaging and publication. One small question to confirm, at this stage users will still need to run I once had an experimental PR to use poetry's custom build file to auto-generate the open api client code when building the package. But I did not have time to fix all the CI for that PR. Do you think that could be the next step of the python packaging? |
So the bash script polaris already does it. Also, I added those to the new Makefile PR for adding python client related operations in. But yes, if that is more preferred, I can update poetry to do so as opposed to using an external bash script for those. But ideally, people should be using pip install Polaris from a python repo instead. From the changes introduced in this PR, those generated files will be included. Let me know which route is more preferred. @HonahX |
I also chatted with Eric offline, and we both think it would be ideal to auto-generate those files as part of the packaging or installation, rather than relying on a separate script. Like you mentioned, the bash script isn’t very helpful for users who “just want to use the CLI” without having to download the entire codebase. Ideally, users should be able to pip install polaris with a plain clone of the repo, without running any one-time setup script first. I think we can merge this first as it already made a huge step to set CLI as the entrypoint |
Currently we are using
<git_root_dir>/polaris
as the entrypoint to interact with polaris CLI. This is a okay for local usage but it is not ideal for others who doesn't want to downlaod the entire code base to just use the CLI as well as making polaris client distributable as part of release. To address this issue, we will need to public polaris as an CLI without depends on the bash script.This PR addressed this problem with following changes:
polaris_cli.py
as that will be the entrypoint for us to useclient/python/polais
is auto generated from OpenAPI andpoetry.core.masonry.api
doe build backend (withpoetry.core.masonry.api
as build backend, it only includes files that are tracked by git. in this case, then auto generated files won't be track without including adding an include clause)polaris
within bash scriptpolaris
Here is a quick demo on the packaging:
setup poetry then perform a poetry build (which we can use later to distribute the client code as well as push to Python Package Index):
now assuming I am an user who doesn't want to download the code base but want to use the cli:
Here is a quick demo on the updated bash script:
By doing so, we really don't need bash script
polaris
(at least the name is confusing with this change...instead, it should be client_setup.sh or something similar) and end-users can do regular pip install to setup Polaris CLI. To avoid too much change, I will keep the bash scriptpolaris
for now (this script is still useful for initial setup as well as refresh dependencies etc. but with proper Python Package Index, we should just do pip upgrade instead).