-
Notifications
You must be signed in to change notification settings - Fork 76
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
Do not fail extraction if inspector cannot find the root project and minor doc update #1396
Conversation
@@ -84,7 +84,7 @@ Pip Native Inspector runs the [pip-inspector.py script](https://github.com/black | |||
python setup.py install | |||
pip install -r requirements.txt | |||
```` | |||
* Pip detector derives your project name using your setup.py file if you have one. If you do not have a setup.py file, you must provide the correct project name using the propety --detect.pip.project.name. | |||
* Pip detector attempts to derive the project name using your setup.py file if you have one. If you do not have a setup.py file, you can provide the correct project name using the propety `--detect.pip.project.name`. |
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 not fully related to the code change, but as I was skimming through the doc I figured that "must" is too strong of a word here. There are many types of projects for which the PIP Inspector will produce complete results even if there is no root project name.
if (trimmedLine.startsWith(UNKNOWN_PROJECT_NAME)) { | ||
logger.error("Pip inspector could not resolve the project"); | ||
unResolvedPackage = true; | ||
} |
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.
Can a warning message be useful if we are not erroring out? In the message, we can ask users to use detect.pip.project.name
property.
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 condition could be encountered even if users pass in --detect.pip.project.name
.
UNKNOWN_PROJECT_NAME
is when the inspector outputs n?
which means that the given project name (wherever it came from) was not found in installed PIP packages.
This is also encountered when there is no project name at all, and that is actually an OK condition for many projects that could be scanned.
So, I am somewhat on the fence about this. Maybe another reviewer can chime in. :)
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 ended up adding an info message after all.
Do not fail extraction if inspector cannot find the root project and minor doc update
Description
Outside of virtual environments, even after running
python setup.py install
PIP inspector cannot find the project package in cache.Even running
pip list
does not show the project package in such cases, so it doesn't look like we have a clear way to modify the inspector to be able to find it.This means that a recent change to fail extractions when the project is not found in PIP cache was too aggressive. This PR reverts parts of it.