-
Notifications
You must be signed in to change notification settings - Fork 6
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
Modernize the RACK cli packaging #993
Conversation
@kityansiu this should address the messages you were getting about using a deprecated install process. The process is now very modern! |
@tuxji would you have time to check that I didn't do something silly to the Github CI process? |
Tried out your branch. Works beautifully for me! |
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.
+1
Looks good! Just have a few questions to better understand what's going on.
.github/workflows/continuous.yml
Outdated
run: pip3 install -q -r cli/dev/requirements.txt | ||
run: | | ||
cd cli | ||
pip3 install ".[dev]" |
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.
I'm not familiar with this ".[dev]". What does it mean?
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 (kind of concerning) pip syntax that means to install the package in the current directory .
using the optional dependency set dev
. Another option was for me to write rack[dev]
.github/workflows/continuous.yml
Outdated
pip3 install cli/. | ||
cd RACK/cli | ||
pip3 install . | ||
cd .. |
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.
Too bad that you need two cd commands. What makes pip3 install ./cli
no longer work?
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.
First I tried what you wrote, and that didn't work. I'm new to using pyproject.toml
and when I found that the code committed above working, I went with it.
Your question prompted me to try harder and I learned that a trailing /
indicates to pip that what I want is for it to install a directory, rather than a package name. I've updated the script to use a single command and saved the extra cd
for another day!
@@ -183,5 +183,5 @@ done | |||
# Setup the RACK dataset using the RACK CLI | |||
|
|||
cd "/home/${USER}/RACK/cli/" | |||
python3 -m pip install ./wheels/*.whl | |||
python3 -m pip install --no-dependencies ./wheels/*.whl |
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.
Why not pip3 install
?
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.
only because I wasn't thinking about changing this line of code at all until it failed due to dependency checks. I was singularly focused on --no-dependencies
when I visited this line. I'll make the change.
In case you wondered and didn't consult the failed build logs, the --no-dependencies
became necessary because pip was able to tell that the version of python3-semtk
that we'd installed was not the GitHub version, as specified in the pyproject.toml
but it was the pre-compiled wheel file which self-descripted as version 0.1
. This flag disables all the checks. I believe that this is OK because all the checks were done at the point at which we generated the wheels.
No description provided.