-
Notifications
You must be signed in to change notification settings - Fork 446
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
feat: use uv as a pip alternative #1088
Open
MoisesGSalas
wants to merge
1
commit into
overhangio:main
Choose a base branch
from
eduNEXT:mgs/uv
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
- [Improvement] Use [`uv`](https://github.com/astral-sh/uv) as a replacement for | ||
pip for installing and resolving packages. uv provides a faster package | ||
resolution and installation steps, reducing the python-requirements layer | ||
build time by about ~2-5x. | ||
|
||
For the most part uv is a drop-in replacement for main pip functionality with | ||
the exception of VCS editable requirements. The main use of VCS editable | ||
requirements is to copy all the files in the VCS repository when installing | ||
the package. This can be avoided by making proper use of a `MANIFEST.in` file. | ||
It's possible to also use the `PIP_COMMAND=pip` build argument to keep using | ||
pip. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,10 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ | |
apt update && \ | ||
apt install -y build-essential curl git language-pack-en | ||
ENV LC_ALL=en_US.UTF-8 | ||
|
||
COPY --from=ghcr.io/astral-sh/uv:0.4.21 /uv /usr/local/bin/uv | ||
ARG PIP_COMMAND="uv pip" | ||
|
||
{{ patch("openedx-dockerfile-minimal") }} | ||
|
||
###### Install python with pyenv in /opt/pyenv and create virtualenv in /openedx/venv | ||
|
@@ -51,8 +55,10 @@ RUN git config --global user.email "[email protected]" \ | |
{{ patch("openedx-dockerfile-git-patches-default") }} | ||
{%- elif EDX_PLATFORM_VERSION == "master" %} | ||
# Patches in nightly node | ||
RUN curl -fsSL https://github.com/openedx/edx-platform/commit/3642cab3ac61ddfd360e7ceb7463b52be8c4deb0.patch | git am | ||
{%- else %} | ||
# Patches in non-nightly mode | ||
|
||
{%- endif %} | ||
|
||
{# Example: RUN curl -fsSL https://github.com/openedx/edx-platform/commit/<GITSHA1>.patch | git am #} | ||
|
@@ -74,6 +80,7 @@ FROM python AS python-requirements | |
ENV PATH=/openedx/venv/bin:${PATH} | ||
ENV VIRTUAL_ENV=/openedx/venv/ | ||
ENV XDG_CACHE_HOME=/openedx/.cache | ||
ENV UV_CACHE_DIR=$XDG_CACHE_HOME/pip | ||
|
||
RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ | ||
--mount=type=cache,target=/var/lib/apt,sharing=locked \ | ||
|
@@ -91,11 +98,11 @@ RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ | |
# Install base requirements | ||
RUN --mount=type=bind,from=edx-platform,source=/requirements/edx/base.txt,target=/openedx/edx-platform/requirements/edx/base.txt \ | ||
--mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ | ||
pip install -r /openedx/edx-platform/requirements/edx/base.txt | ||
$PIP_COMMAND install -r /openedx/edx-platform/requirements/edx/base.txt | ||
|
||
# Install extra requirements | ||
RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ | ||
pip install \ | ||
$PIP_COMMAND install \ | ||
# Use redis as a django cache https://pypi.org/project/django-redis/ | ||
django-redis==5.4.0 \ | ||
# uwsgi server https://pypi.org/project/uWSGI/ | ||
|
@@ -104,20 +111,21 @@ RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ | |
{{ patch("openedx-dockerfile-post-python-requirements") }} | ||
|
||
# Install scorm xblock | ||
RUN pip install "openedx-scorm-xblock>=18.0.0,<19.0.0" | ||
RUN $PIP_COMMAND install "openedx-scorm-xblock>=18.0.0,<19.0.0" | ||
|
||
{% for extra_requirements in OPENEDX_EXTRA_PIP_REQUIREMENTS %} | ||
RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ | ||
pip install '{{ extra_requirements }}' | ||
$PIP_COMMAND install '{{ extra_requirements }}' | ||
{% endfor %} | ||
|
||
###### Install nodejs with nodeenv in /openedx/nodeenv | ||
FROM python AS nodejs-requirements | ||
ENV VIRTUAL_ENV=/openedx/venv/ | ||
ENV PATH=/openedx/nodeenv/bin:/openedx/venv/bin:${PATH} | ||
|
||
# Install nodeenv with the version provided by edx-platform | ||
# https://github.com/openedx/edx-platform/blob/master/requirements/edx/base.txt | ||
RUN pip install nodeenv==1.8.0 | ||
RUN $PIP_COMMAND install nodeenv==1.8.0 | ||
RUN nodeenv /openedx/nodeenv --node=18.20.1 --prebuilt | ||
|
||
# Install nodejs requirements | ||
|
@@ -174,12 +182,12 @@ WORKDIR /openedx/edx-platform | |
{# Install auto-mounted directories as Python packages. #} | ||
{% for name in iter_mounted_directories(MOUNTS, "openedx") %} | ||
COPY --link --chown=$APP_USER_ID:$APP_USER_ID --from=mnt-{{ name }} / /mnt/{{ name }} | ||
RUN pip install -e "/mnt/{{ name }}" | ||
RUN $PIP_COMMAND install -e "/mnt/{{ name }}" | ||
{% endfor %} | ||
|
||
# We install edx-platform here because it creates an egg-info folder in the current | ||
# repo. We need both the source code and the virtualenv to run this command. | ||
RUN pip install -e . | ||
RUN $PIP_COMMAND install -e . | ||
|
||
# Create folder that will store lms/cms.env.yml files, as well as | ||
# the tutor-specific settings files. | ||
|
@@ -260,16 +268,16 @@ USER app | |
|
||
# Install dev python requirements | ||
RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ | ||
pip install -r requirements/edx/development.txt | ||
$PIP_COMMAND install -r requirements/edx/development.txt | ||
# https://pypi.org/project/ipdb/ | ||
# https://pypi.org/project/ipython (>=Python 3.10 started with 8.20) | ||
RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ | ||
pip install ipdb==0.13.13 ipython==8.24.0 | ||
$PIP_COMMAND install ipdb==0.13.13 ipython==8.24.0 | ||
|
||
{# Re-install mounted requirements, otherwise they will be superseded by upstream reqs #} | ||
{% for name in iter_mounted_directories(MOUNTS, "openedx") %} | ||
COPY --link --chown=$APP_USER_ID:$APP_USER_ID --from=mnt-{{ name }} / /mnt/{{ name }} | ||
RUN pip install -e "/mnt/{{ name }}" | ||
RUN $PIP_COMMAND install -e "/mnt/{{ name }}" | ||
{% endfor %} | ||
|
||
# Add ipdb as default PYTHONBREAKPOINT | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
why is this patch needed?
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.
edx-proctoring-proctortrack is installed as an editable VCS dependency but uv doesn't support that. I have this PR openedx/edx-platform#35654 with the same change that is used in that patch.
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.
The fact that uv doesn't support editable dependencies is a blocking issue, I think. I suggest we wait until the upstream issue is resolved.
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 they are not going to implement it in the near future. Note that it refers to editable remote requirements, it works fine with editable requirements in a local path.
AFAIK the only use for an editable remote requirement is that it copies all the files, but you can fix that using the
MANIFEST.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 understand that. The, using
uv
means we will no longer be able to support adding editable dependencies to edx-platform. I'm not sure I'm ready to make that breaking change, as I expect that many people (Axim included) might add editable dependencies there.Can we consider making an upstream PR to support that feature?
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 comment gave me the impression they don't want to maintain code related to this feature:
Maybe we can keep standard
pip
as the default with the possibility to swap touv
if your fork of edx-platform doesn't contain any editable dependency.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.
That's already the case. People can install uv in their own Docker image and alias
pip='uv pip'
, all with a Tutor plugin.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.
But that wouldn't work for the RUN commands in the Dockerfile, right? The main appeal would be to use it in the
python-requirements
layer.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.
It would. Install uv with the "openedx-dockerfile-minimal" patch, then
ln -s /usr/bin/uv /usr/bin/pip
. Maybe this will be overwritten in the python-requirements layer, in which case we would have to introduce a new patch statement just abovepip install -r /openedx/edx-platform/requirements/edx/base.txt
. Would you like to introduce such a patch?