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

Fix CiHelper function checkPythonExec to use 'pip3 install --user' and improve error message #12097

Merged
merged 2 commits into from Sep 15, 2020

Conversation

damonreed
Copy link
Contributor

While running the code validation script on an install that hadn't been verified before, pip3 failed when trying to install pylint.

[librenms@devhost ~]$ ./lnms dev:check

Running PHP lint check...
PHP 7.3.21 | 10 parallel jobs
............................................................   60/3995 (1 %)
[...]
...................................                          3995/3995 (100 %)

[...]
Running pip3 install to install developer dependencies.
...
[Package downloads]
...
Installing collected packages: isort, mccabe, toml, wrapt, typed-ast, six, lazy-object-proxy, astroid, pylint
Exception:
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/pip/basecommand.py", line 215, in main
    status = self.run(options, args)
 [...]
  File "/usr/lib64/python3.6/os.py", line 220, in makedirs
    mkdir(name, mode)
PermissionError: [Errno 13] Permission denied: '/usr/local/lib/python3.6/site-packages/isort'

Running installing deps with pip3 failed.
 You should try running 'pip3 install -r requirements.txt' by hand
You can find more info at http://docs.librenms.org/Developing/Validating-Code/

CiHelper was trying to install the dependency (pylint in this case) for the entire system, which is a privileged operation. Also, the error message was inaccurate for what was happening.

I compared this to how the main dependencies are checked/installed:
https://github.com/librenms/librenms/blob/f526ba326b21727ac3025e062437812ce5e6d20e/composer.json

        "python-requirements": [
            "scripts/check_requirements.py || pip3 install --user -r requirements.txt || :"

This makes sense to me, so I added a --user flag to the CIHelper check, and the install now runs cleanly.

Also fixed the error message to reflect the actual target dependency module in question.

DO NOT DELETE THE UNDERLYING TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

  • Have you followed our code guidelines?
  • If my Pull Request does some changes/fixes/enhancements in the WebUI, I have inserted a screenshot of it.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

@damonreed damonreed changed the title Fix pip3 Fix CiHelper function checkPythonExec to use 'pip3 install --user' and improve error message Sep 15, 2020
@Jellyfrog Jellyfrog merged commit e0a0764 into librenms:master Sep 15, 2020
@murrant
Copy link
Member

murrant commented Sep 30, 2020

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/v1-68-release-changelog-september-2020/13525/1

@damonreed damonreed deleted the fix_pip3 branch November 20, 2020 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants