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.

Sign up for GitHub

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 missing setuptools dep #4799

Merged
merged 2 commits into from
Aug 9, 2024
Merged

Fix missing setuptools dep #4799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
201ff94
Fix missing `setuptools` dep
joeyorlando Aug 9, 2024
c9d8346
Merge branch 'dev' into jorlando/fix-missing-setuptools-dep
joeyorlando Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/actions/setup-python/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ runs:
if: ${{ inputs.install-dependencies == 'true' }}
shell: bash
run: |
pip install uv
pip install uv setuptools
uv pip sync --system ${{ inputs.python-requirements-paths }}
2 changes: 1 addition & 1 deletion engine/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ RUN if [ "$TARGETPLATFORM" = "linux/arm64" ]; then \
&& rm grpcio-1.64.1-cp312-cp312-linux_aarch64.whl; \
fi

RUN pip install uv
RUN pip install uv setuptools
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if setuptools is being added through requirements, does it need to be here too? (same above)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess technically no, but I think setuptools is only being included in requirements.txt/requirements-dev.txt because of two transitive dependencies. If those dependencies drop their requirement on setuptools we'll need this anyways 🤔

Python 3.12's release notes mention:

python/cpython#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.


# TODO: figure out how to get this to work.. see comment in .github/workflows/e2e-tests.yml
# https://stackoverflow.com/a/71846527
Expand Down
4 changes: 2 additions & 2 deletions engine/requirements-dev.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ pyyaml==6.0.1
# via
# -c requirements.txt
# pre-commit
requests==2.32.0
requests==2.32.3
# via
# -c requirements.txt
# djangorestframework-stubs
setuptools==70.0.0
setuptools==72.1.0
# via
# -c requirements.txt
# nodeenv
Expand Down
11 changes: 1 addition & 10 deletions engine/requirements.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ python-telegram-bot==13.13
recurring-ical-events==2.1.0
redis==5.0.1
regex==2021.11.2
requests==2.32.0
requests==2.32.3
Copy link
Contributor Author

@joeyorlando joeyorlando Aug 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, bump requests because of this message I was seeing when running uv pip compile:

warning: `requests==2.32.0` is yanked (reason: "Yanked due to conflicts with CVE-2024-35195 mitigation").

https://nvd.nist.gov/vuln/detail/CVE-2024-35195

slack-export-viewer==1.1.4
slack_sdk==3.21.3
social-auth-app-django==5.4.1
Expand All @@ -64,12 +64,3 @@ whitenoise==5.3.0
google-api-python-client==2.122.0
google-auth-httplib2==0.2.0
google-auth-oauthlib==1.2.0
# see the following resources as to why we need to install setuptools manually
#
# Python 3.12 release notes https://docs.python.org/3/whatsnew/3.12.html
#
# python/cpython#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.
setuptools==70.0.0
10 changes: 7 additions & 3 deletions engine/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ cachetools==4.2.2
# via
# google-auth
# python-telegram-bot
celery[redis]==5.3.1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand why it's dropping the [redis] portion here 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, astral-sh/uv#1595 (I don't have a strong opinion to preserve the extra, though)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the reference 👍 (seems like including it/excluding it doesn't make a big difference)

celery==5.3.1
# via -r requirements.in
certifi==2024.2.2
# via
Expand Down Expand Up @@ -157,7 +157,7 @@ firebase-admin==5.4.0
# via fcm-django
flask==3.0.2
# via slack-export-viewer
google-api-core[grpc]==2.17.0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to this comment

google-api-core==2.17.0
# via
# firebase-admin
# google-api-python-client
Expand Down Expand Up @@ -392,7 +392,7 @@ referencing==0.33.0
# jsonschema-specifications
regex==2021.11.2
# via -r requirements.in
requests==2.32.0
requests==2.32.3
# via
# -r requirements.in
# cachecontrol
Expand All @@ -415,6 +415,10 @@ rsa==4.9
# via google-auth
s3transfer==0.10.0
# via boto3
setuptools==72.1.0
# via
# apscheduler
# opentelemetry-instrumentation
Comment on lines +418 to +421
Copy link
Contributor Author

@joeyorlando joeyorlando Aug 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there's not much we can do here? I assume this means that apscheduler and opentelemetry-instrumentation both have a dependency on setuptools?

six==1.16.0
# via
# apscheduler
Expand Down
Loading