-
-
Notifications
You must be signed in to change notification settings - Fork 36
Add cli commands, restructure data and update code and tests #281
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
Reference: #247 Signed-off-by: John M. Horan <[email protected]>
Reference: #247 Signed-off-by: John M. Horan <[email protected]>
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.
@johnmhoran I've left some comments on the meta
command and related functions.
Thanks for your helpful comments @JonoYang AND for taking a look at this nascent PR so quickly. |
Reference: #247 Signed-off-by: John M. Horan <[email protected]>
Signed-off-by: John M. Horan <[email protected]>
Reference: #247 Signed-off-by: John M. Horan <[email protected]>
Reference: #247 Signed-off-by: John M. Horan <[email protected]>
], | ||
"errors": [], | ||
"warnings": [ | ||
"input PURL: 'pkg:pypi/[email protected]' normalized to 'pkg:pypi/fetchcode'", |
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.
Please do NOT normalize unless someone requested it... and we surely should never drop a version when request for one.
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 get that. You made the point over the weekend and I acknowleged and explained what I'm doing as a result. Please look at the name of this file and the context -- this is the result when the --unique
flag is included.
Reference: #247 Signed-off-by: John M. Horan <[email protected]>
@JonoYang Puzzling -- all 76 of my
An example of the diff for
But that test passes locally and if I check that PURL with the
|
@JonoYang I think I might have found the cause of these erroneous GitHub test failures: the GitHub tests have an old version of
If I'm correct, could we update the GitHub tests to use the current version of Update: I see that |
Reference: #247 Signed-off-by: John M. Horan <[email protected]>
@JonoYang Close but no cigar. There are other problems, apparently related to an effort to pin 0.11.2. See https://github.com/nexB/purldb/actions/runs/7927038107/job/21642750969?pr=281#step:5:60 et seq:
Not sure how to translate this into language I can understand and use to enable my simple tests to pass using the current version of |
Reference: #247 Signed-off-by: John M. Horan <[email protected]>
Signed-off-by: John M. Horan <[email protected]>
Thank you @TG1999 for helping me identify the source of the 0.11.2 vs 0.13.4 packageurl-python conflict. To address the remaining conflicts referred to in the GH PR page. I've updated When I ran my local PURL CLI tests before pushing, I got an error
|
Is this some sort of conflict re |
@johnmhoran I think what happened is that when I updated the dependencies and the pinned requirements in the requirements.txt and requirements-dev.txt files, that the versions of those packages chosen work on python 3.10+. I remember encountering this issue when the github tests would fail on the test instances running python 3.8. I ended up just only having the github tests run on python 3.10 and 3.11. |
@JonoYang But when I try to run test_purlcli.py locally now, I get
As recently as late last night, all 76 of my tests passed. How am I supposed to run my tests locally? The
|
@johnmhoran I'll try to redo the pinned dependencies using Python 3.8, otherwise, I'd suggest using https://github.com/pyenv/pyenv to install a more recent version of Python. |
@JonoYang I already use Python 3.10.13 in my "sandbox` by installing it in the venv. But the purldb venv is 3.8.10. Couldn't I just update Python in that venv? How can I help you do what you need? If purldb relies now on 3.10 because of your changes, we should just update the venv, no? Maybe you need not pin for 3.8 if there's something I can do instead.... |
@johnmhoran Please use Python 3.10+ in your purldb venv if you can |
@JonoYang How can I update the venv on my branch to use 3.10+ ? I'd love to do that! |
or do I just need to install 3.10.13 or whatever in my WSL2 Ubuntu-20.04 distro and the purldb venv will automatically use that? I assume our venv chose the 3.8.10 that I use in my purldb branch. |
I'm going to try to install 3.10+ in my WSL2 Ubuntu distro and see if the purldb venv uses that instead of 3.8.10. |
@JonoYang I now have Python 3.10.13 but my purldb venv has not changed. How do I use 3.10.13 in that venv? I'll try to select 3.10.13 in my VSCode instance, though I don't see why that would affect what happens in my Windows Terminal instance running WSL2. |
I use https://github.com/pyenv/pyenv to manage python on my systems. pyenv has code that allows you to have different python installs that do not interfere with the system installed python. Ive run into a problem where I updated I followed the instructions for the basic github checkout: then I followed the instructions for the bash shell environment configuration: then I installed the Python build dependencies for Ubuntu/Debian/Mint: then I restart the shell: Then you can install Python 3.10.12 by running: You can set Python 3.10.12 to be the default python/python3 run by running: Afterwards, run |
@JonoYang Will do. To confirm: in that order, right?
(Then what? In any event, starting right now.) |
@JonoYang It all ran without errors. What did I just do, and what does this tell us about the cause of all the errors I've encountered? |
I realize I have an editable installation, but wonder whether I only temporarily updated the venv to Python 3.10, and not sure what the purldb-toolkit install -e did. That is not what I've been doing when I worked on this repo. |
I think I need to update the Makefile/configure script so it really uses the python3 set in |
Is that something I could help with? I hate to see you spend any of your time on these errors, though I'm not sure what actually caused them. Did I possibly make an error of my own when I merged |
I think the conflicts in |
@JonoYang I tried to run some of my tests -- new error:
|
The issue is that the versions of dependencies in the requirements.txt and requirements-dev.txt files do not work on Python 3.8. In your shell, You can help me try out a possible fix:
|
@johnmhoran I've updated the commands in my comment above. Let me know if that works or not |
@JonoYang I got this error when I ran
|
@johnmhoran The |
The error refers to Makefile 43, which now reads |
That line should be |
@JonoYang Yes, moved the |
@johnmhoran Feel free to try anything out |
@JonoYang This is encouraging. ;-) About to try a regular purlcli command, then one of my pytest command groups.
|
@JonoYang My command worked and my first sample test worked. But the test also output many many lines like this. This is new behavior -- maybe the deprecations come from 3.8 >> 3.10?
|
@JonoYang What's next? Commit my Makefile changes, push, and hope all the GH tests pass and the warning |
@johnmhoran I will merge https://github.com/nexB/purldb/tree/update-makefile into main, and then you pull main into your branch |
Will do @JonoYang . I'm running my full set of purlcli tests now. (I don't think these run when I run
|
@JonoYang When you ping me I'll pull Just commit and push and watch the GH tests run? Or before that do I need to do another |
@johnmhoran I've merged in the changes into main. Please merge them into your branch and then push it to github.
I dont think you need to do this again |
Reference: #247 Signed-off-by: John M. Horan [email protected]
@JonoYang Just pushed. |
@JonoYang All GH checks passed and |
@JonoYang I ran
|
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.
LGTM!
I think this is time to merge this.
Add cli commands, restructure data and update code and tests
Reference: #247