-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
chore: Add github workflow and script to check python depencies source #34549
chore: Add github workflow and script to check python depencies source #34549
Conversation
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 think it makes sense for the workflow to live in this repo but the find_python_dependencies.py code should live in the repo_tools
directory.
- name: Set up Python | ||
uses: actions/setup-python@v4 | ||
with: | ||
python-version: '3.8' |
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.
Just start with 3.12 for this script.
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 intentionally did that because i believe we have not added support of 3.12 in edx-platform yet,
if we try to update the python-version to 3.12 it results error
Traceback (most recent call last): File "/home/runner/work/edx-platform/edx-platform/scripts/find_python_dependencies.py", line 8, in <module> import requirements File "/opt/hostedtoolcache/Python/3.[12](https://github.com/openedx/edx-platform/actions/runs/8811123770/job/24184575317?pr=34549#step:10:13).3/x64/lib/python3.12/site-packages/requirements/__init__.py", line 19, in <module> from .parser import parse File "/opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/requirements/parser.py", line 23, in <module> from .requirement import Requirement File "/opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/requirements/requirement.py", line 24, in <module> from pkg_resources import Requirement as Req ModuleNotFoundError: No module named 'pkg_resources'
The reason for the above error is
gh-95299: Do not pre-install setuptools in virtual environments created with venv. This means that distutils, setuptools, pkg_resources, and easy_install will no longer available by default; to access these run pip install setuptools in the activated virtual environment.
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 have updated it.
|
||
- name: Copy Python requirements file | ||
run: | | ||
for req_file in "requirements/edx/base.txt"; do |
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.
Do you think there will be more files in this list than just this one in the future? If so, can you make a list of one item that can be added to.
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 have updated it.
scripts/find_python_dependencies.py
Outdated
$ python find_deps.py $OLIVE_DIRS | ||
""" | ||
|
||
import concurrent.futures |
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.
You can probably pare down these imports now that you've updated the code.
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 have updated it.
scripts/find_python_dependencies.py
Outdated
else: | ||
print(f"Failed to retrieve data for package {package}. Status code:", response.status_code) | ||
|
||
def process_directory(): |
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.
Instead of processing a directory, why not just pass the path to a file in the main
function, then we don't need to do all the directory work, just parse the requirements out of the file.
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.
sure i have updated it.
- name: Clone repo-tools repo | ||
uses: actions/checkout@v4 | ||
with: | ||
repository: salman2013/repo-tools | ||
path: repo_tools | ||
ref: salman/add-script-for-python-dependencies | ||
|
||
- name: Set up Python | ||
uses: actions/setup-python@v5 | ||
with: | ||
python-version: ${{ matrix.python-version }} | ||
|
||
- name: Install setuptool | ||
run: pip install setuptools | ||
|
||
- name: Install requests module | ||
run: pip install requests | ||
|
||
- name: Install rich module | ||
run: pip install rich | ||
|
||
- name: Install requirements parser module | ||
run: pip install requirements-parser |
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.
- name: Clone repo-tools repo | |
uses: actions/checkout@v4 | |
with: | |
repository: salman2013/repo-tools | |
path: repo_tools | |
ref: salman/add-script-for-python-dependencies | |
- name: Set up Python | |
uses: actions/setup-python@v5 | |
with: | |
python-version: ${{ matrix.python-version }} | |
- name: Install setuptool | |
run: pip install setuptools | |
- name: Install requests module | |
run: pip install requests | |
- name: Install rich module | |
run: pip install rich | |
- name: Install requirements parser module | |
run: pip install requirements-parser | |
- name: Set up Python | |
uses: actions/setup-python@v5 | |
with: | |
python-version: ${{ matrix.python-version }} | |
- name: Install repo-tools | |
run: pip install git+https://github.com/salman2013/repo-tools.git@salman/add-script-for-python-dependencies |
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.
@feanil There is one thing in this, i need to install requirements-parser
and setuptools
so there are two options should i add them here in CI or add them in repo-tool so that the script can find these packages. What do you suggest?
- name: Make the script files executable | ||
run: chmod +x repo_tools/edx_repo_tools/find_dependencies/find_python_dependencies.py |
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.
- name: Make the script files executable | |
run: chmod +x repo_tools/edx_repo_tools/find_dependencies/find_python_dependencies.py |
This shouldn't be necessary if we're pip-installing repo-tools.
run: chmod +x repo_tools/edx_repo_tools/find_dependencies/find_python_dependencies.py | ||
|
||
- name: Run Python script | ||
run: python repo_tools/edx_repo_tools/find_dependencies/find_python_dependencies.py requirements/edx/base.txt requirements/edx/testing.txt |
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.
run: python repo_tools/edx_repo_tools/find_dependencies/find_python_dependencies.py requirements/edx/base.txt requirements/edx/testing.txt | |
# Tracking ignored repos here: https://github.com/openedx/public-engineering/issues/174 | |
run: python repo_tools/edx_repo_tools/find_dependencies/find_python_dependencies.py requirements/edx/base.txt requirements/edx/testing.txt --ignore https://github.com/edx/codejail-includes --ignore https://github.com/edx/braze-client --ignore https://github.com/edx/edx-name-affirmation --ignore https://github.com/mitodl/edx-sga --ignore https://github.com/edx/token-utils --ignore https://github.com/edx/codejail-includes --ignore https://github.com/edx/braze-client --ignore https://github.com/edx/edx-name-affirmation --ignore https://github.com/mitodl/edx-sga --ignore https://github.com/edx/token-utils |
I've made sure that all these repos are part of a tickte to further investigate and clean them up here: openedx/public-engineering#174 so we can ignore them for now and prevent future additions, we'll make sure to remove the repos from this list as they get removed from edx-platform.
run: pip install setuptools | ||
|
||
- name: Run Python script | ||
run: find_python_dependencies --dirs requirements/edx/base.txt --dirs requirements/edx/testing.txt --ignore https://github.com/edx/codejail-includes --ignore https://github.com/edx/braze-client --ignore https://github.com/edx/edx-name-affirmation --ignore https://github.com/mitodl/edx-sga --ignore https://github.com/edx/token-utils --ignore https://github.com/edx/codejail-includes --ignore https://github.com/edx/braze-client --ignore https://github.com/edx/edx-name-affirmation --ignore https://github.com/mitodl/edx-sga --ignore https://github.com/edx/token-utils |
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.
Nit, can you split this over multiple lines so that it's easier to read?
@feanil I think as per the documentation references, we should explicitly install the setuptool in environment where needed, like in our case I think in CI its best suited place instead of including it in repo-tool base.txt |
5628968
to
87a7e87
Compare
- name: Install repo-tools | ||
run: pip install git+https://github.com/salman2013/repo-tools.git@salman/add-script-for-python-dependencies | ||
|
||
- name: Install requirements parser module | ||
run: pip install requirements-parser | ||
|
||
- name: Install setuptool | ||
run: pip install setuptools |
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.
- name: Install repo-tools | |
run: pip install git+https://github.com/salman2013/repo-tools.git@salman/add-script-for-python-dependencies | |
- name: Install requirements parser module | |
run: pip install requirements-parser | |
- name: Install setuptool | |
run: pip install setuptools | |
- name: Install repo-tools | |
run: pip install edx-repo-tools[find_dependencies] |
I think this is all you should need now.
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.
@feanil repo-tool is requiring setuptool
to run this github action on 3.12
so we have two options either we should keep it here like following step or install it in repo-tool
but I believe it should be here where we need it as python has dropped its default support in 3.12
- name: Install setuptool
run: pip install setuptools
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 have updated other things and everything is working fine.
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
2U Release Notice: This PR has been deployed to the edX production environment. |
Description: Add github action to run the python script from repo tools, which finds the python dependencies source and displays them if they belong to any third-party sources.
Resolves #33189