Skip to content

Replace pip and pip-tools with uv - #3357

Merged
mathjazz merged 2 commits into
mozilla:mainfrom
flodolo:replace_pip
Sep 20, 2024
Merged

Replace pip and pip-tools with uv#3357
mathjazz merged 2 commits into
mozilla:mainfrom
flodolo:replace_pip

Conversation

@flodolo

@flodolo flodolo commented Sep 19, 2024

Copy link
Copy Markdown
Collaborator

As discussed, this needs to be tested on stage to make sure that the post compile step works as expected on Heroku.

@flodolo
flodolo marked this pull request as draft September 19, 2024 18:40
@mathjazz

mathjazz commented Sep 19, 2024

Copy link
Copy Markdown
Collaborator

Deployed to stage. The No change in requirements detected, installing from cache part is not what I expected.

Sideshow:mozilla-pontoon mathjazz$ git push staging main
Enumerating objects: 58, done.
Counting objects: 100% (41/41), done.
Delta compression using up to 10 threads
Compressing objects: 100% (21/21), done.
Writing objects: 100% (22/22), 3.43 KiB | 3.43 MiB/s, done.
Total 22 (delta 18), reused 0 (delta 0), pack-reused 0
remote: Updated 1212 paths from 5a8b5658
remote: Compressing source files... done.
remote: Building source:
remote:
remote: -----> Building on the Heroku-22 stack
remote: -----> Using buildpacks:
remote:        1. https://github.com/mozilla/heroku-buildpack-apt.git#v0.1
remote:        2. https://github.com/Osmose/heroku-buildpack-ssh.git#v0.1
remote:        3. heroku/nodejs
remote:        4. heroku/python
remote:        5. https://github.com/gerywahyunugraha/heroku-google-application-credentials-buildpack.git
remote: -----> Apt app detected
remote: -----> Updating apt caches
remote:        Hit:1 http://archive.ubuntu.com/ubuntu jammy InRelease
remote:        Hit:2 http://archive.ubuntu.com/ubuntu jammy-security InRelease
remote:        Hit:3 http://archive.ubuntu.com/ubuntu jammy-updates InRelease
remote:        Hit:4 http://apt.postgresql.org/pub/repos/apt jammy-pgdg InRelease
remote:        Reading package lists...
remote: W: http://apt.postgresql.org/pub/repos/apt/dists/jammy-pgdg/InRelease: Key is stored in legacy trusted.gpg keyring (/etc/apt/trusted.gpg), see the DEPRECATION section in apt-key(8) for details.
remote: -----> Fetching .debs for libsqlite3-dev
remote:        Reading package lists...
remote:        Building dependency tree...
remote:        Suggested packages:
remote:          sqlite3-doc
remote:        The following NEW packages will be installed:
remote:          libsqlite3-dev
remote:        0 upgraded, 1 newly installed, 0 to remove and 30 not upgraded.
remote:        Need to get 0 B/846 kB of archives.
remote:        After this operation, 3,165 kB of additional disk space will be used.
remote:        Download complete and in download only mode
remote: W: --force-yes is deprecated, use one of the options starting with --allow instead.
remote: -----> Fetching .debs for subversion
remote:        Reading package lists...
remote:        Building dependency tree...
remote:        The following additional packages will be installed:
remote:          libapr1 libaprutil1 libserf-1-1 libsvn1 libutf8proc2
remote:        Suggested packages:
remote:          db5.3-util libapache2-mod-svn subversion-tools
remote:        The following NEW packages will be installed:
remote:          libapr1 libaprutil1 libserf-1-1 libsvn1 libutf8proc2 subversion
remote:        0 upgraded, 6 newly installed, 0 to remove and 30 not upgraded.
remote:        Need to get 0 B/2,672 kB of archives.
remote:        After this operation, 10.5 MB of additional disk space will be used.
remote:        Download complete and in download only mode
remote: W: --force-yes is deprecated, use one of the options starting with --allow instead.
remote: -----> Installing libapr1_1.6.3-2_amd64.deb
remote: -----> Installing libapr1_1.7.0-8build1_amd64.deb
remote: -----> Installing libapr1_1.7.0-8ubuntu0.22.04.1_amd64.deb
remote: -----> Installing libaprutil1_1.6.1-2_amd64.deb
remote: -----> Installing libaprutil1_1.6.1-5ubuntu4.22.04.1_amd64.deb
remote: -----> Installing libaprutil1_1.6.1-5ubuntu4.22.04.2_amd64.deb
remote: -----> Installing libaprutil1_1.6.1-5ubuntu4_amd64.deb
remote: -----> Installing libserf-1-1_1.3.9-10ubuntu2_amd64.deb
remote: -----> Installing libserf-1-1_1.3.9-6_amd64.deb
remote: -----> Installing libsqlite3-0_3.22.0-1ubuntu0.5_amd64.deb
remote: -----> Installing libsqlite3-0_3.22.0-1ubuntu0.6_amd64.deb
remote: -----> Installing libsqlite3-0_3.37.2-2ubuntu0.1_amd64.deb
remote: -----> Installing libsqlite3-0_3.37.2-2ubuntu0.3_amd64.deb
remote: -----> Installing libsqlite3-dev_3.22.0-1ubuntu0.4_amd64.deb
remote: -----> Installing libsqlite3-dev_3.22.0-1ubuntu0.5_amd64.deb
remote: -----> Installing libsqlite3-dev_3.22.0-1ubuntu0.6_amd64.deb
remote: -----> Installing libsqlite3-dev_3.37.2-2_amd64.deb
remote: -----> Installing libsqlite3-dev_3.37.2-2ubuntu0.1_amd64.deb
remote: -----> Installing libsqlite3-dev_3.37.2-2ubuntu0.3_amd64.deb
remote: -----> Installing libsvn1_1.14.1-3ubuntu0.22.04.1_amd64.deb
remote: -----> Installing libsvn1_1.9.7-4ubuntu1.1_amd64.deb
remote: -----> Installing libsvn1_1.9.7-4ubuntu1_amd64.deb
remote: -----> Installing libutf8proc2_2.7.0-3_amd64.deb
remote: -----> Installing subversion_1.14.1-3ubuntu0.22.04.1_amd64.deb
remote: -----> Installing subversion_1.9.7-4ubuntu1.1_amd64.deb
remote: -----> Installing subversion_1.9.7-4ubuntu1_amd64.deb
remote: -----> Writing profile script
remote: -----> SSH app detected
remote: -----> Node.js app detected
remote:
remote: -----> Creating runtime environment
remote:
remote:        NPM_CONFIG_LOGLEVEL=error
remote:        NODE_VERBOSE=false
remote:        NODE_ENV=production
remote:        NODE_MODULES_CACHE=true
remote:
remote: -----> Installing binaries
remote:        engines.node (package.json):   unspecified
remote:        engines.npm (package.json):    >= 7
remote:
remote:        Resolving node version 20.x...
remote:        Downloading and installing node 20.17.0...
remote:        Bootstrapping npm >= 7 (replacing 10.8.2)...
remote:        npm 10.8.3 installed
remote:
remote: -----> Restoring cache
remote:        - npm cache
remote:
remote: -----> Installing dependencies
remote:        Installing node modules
remote:
remote:        added 1058 packages, and audited 1062 packages in 13s
remote:
remote:        165 packages are looking for funding
remote:          run `npm fund` for details
remote:
remote:        5 vulnerabilities (1 moderate, 4 high)
remote:
remote:        To address all issues, run:
remote:          npm audit fix
remote:
remote:        Run `npm audit` for details.
remote:
remote: -----> Build
remote:        Detected both "build" and "heroku-postbuild" scripts
remote:        Running heroku-postbuild
remote:
remote:        > pontoon@0.1.0 heroku-postbuild
remote:        > echo Build is taken care of in ./bin/post_compile
remote:
remote:        Build is taken care of in ./bin/post_compile
remote:
remote: -----> Caching build
remote:        - npm cache
remote:
remote: -----> Pruning devDependencies
remote:
remote:        up to date, audited 166 packages in 2s
remote:
remote:        12 packages are looking for funding
remote:          run `npm fund` for details
remote:
remote:        2 high severity vulnerabilities
remote:
remote:        To address all issues, run:
remote:          npm audit fix
remote:
remote:        Run `npm audit` for details.
remote:
remote: -----> Build succeeded!
remote: -----> Python app detected
remote: -----> Using Python version specified in runtime.txt
remote:  !
remote:  !     A Python security update is available! Upgrade as soon as possible to: python-3.11.10
remote:  !     See: https://devcenter.heroku.com/articles/python-runtimes
remote:  !
remote: -----> No change in requirements detected, installing from cache
remote: -----> Using cached install of python-3.11.9
remote: -----> Installing pip 24.0, setuptools 70.3.0 and wheel 0.44.0
remote: -----> Installing SQLite3
remote: -----> Installing requirements with pip
remote:        Collecting silme@ https://github.com/mozilla/silme/archive/v0.11.2.zip (from -r requirements/default.txt (line 1216))
remote:          Downloading https://github.com/mozilla/silme/archive/v0.11.2.zip
remote:          Installing build dependencies: started
remote:          Installing build dependencies: finished with status 'done'
remote:          Getting requirements to build wheel: started
remote:          Getting requirements to build wheel: finished with status 'done'
remote:          Preparing metadata (pyproject.toml): started
remote:          Preparing metadata (pyproject.toml): finished with status 'done'
remote: -----> Skipping Django collectstatic since the env var DISABLE_COLLECTSTATIC is set.
remote: -----> Running post-compile hook
remote:
remote: removed 53 packages, and changed 101 packages in 3s
remote:
remote: 11 packages are looking for funding
remote:   run `npm fund` for details
remote:
remote: added 1058 packages, and audited 1062 packages in 18s
remote:
remote: 165 packages are looking for funding
remote:   run `npm fund` for details
remote:
remote: 5 vulnerabilities (1 moderate, 4 high)
remote:
remote: To address all issues, run:
remote:   npm audit fix
remote:
remote: Run `npm audit` for details.
remote: Building translate & tag-admin...
remote:
remote: > pontoon@0.1.0 build:prod
remote: > npm run build:prod --workspaces --if-present
remote:
remote:
remote: > build:prod
remote: > rollup -c --environment BUILD:production
remote:
remote:
remote: src/index.tsx → dist/translate.js...
remote: created dist/translate.js in 21.2s
remote:
remote: > build:prod
remote: > rollup -c --environment BUILD:production
remote:
remote:
remote: src/index.js → dist/tag_admin.js...
remote: Browserslist: caniuse-lite is outdated. Please run:
remote:   npx update-browserslist-db@latest
remote:   Why you should do it regularly: https://github.com/browserslist/update-db#readme
remote: created dist/tag_admin.js in 2.9s
remote: Collecting static files...
remote: Operations to perform:
remote:   Apply all migrations: account, actionlog, admin, auth, base, checks, contenttypes, guardian, homepage, insights, messaging, notifications, sessions, sites, socialaccount, sync, tags, terminology, tour, uxactionlog
remote: Running migrations:
remote:   No migrations to apply.
remote:
remote: 701 static files copied to '/tmp/build_bcbeb85b/static', 2233 post-processed.
remote: Current commit being deployed:
remote: 49985793396abef214ab7493dac5c83cc6d68484
remote: Sending deploy notification to New Relic....
remote: -----> Google Cloud app detected
remote: ------> Generating .profile.d file to generate google-credentials.json at startup
remote: -----> Discovering process types
remote:        Procfile declares types -> automl-warmup, web, worker
remote:
remote: -----> Compressing...
remote:        Done: 248M
remote: -----> Launching...
remote:        Released v2833
remote:        https://mozilla-pontoon-staging.herokuapp.com/ deployed to Heroku
remote:
remote: This app is using the Heroku-22 stack, however a newer stack is available.
remote: To upgrade to Heroku-24, see:
remote: https://devcenter.heroku.com/articles/upgrading-to-the-latest-stack
remote:
remote: Verifying deploy... done.
To https://git.heroku.com/mozilla-pontoon-staging.git
   0682003b..49985793  main -> main

@flodolo

flodolo commented Sep 20, 2024

Copy link
Copy Markdown
Collaborator Author

OK, the action is working now.

Deployed to stage. The No change in requirements detected, installing from cache part is not what I expected.

I was expecting it, because I didn't change this line
https://github.com/flodolo/pontoon/blob/replace_pip/bin/post_compile#L6

Interestingly, I didn't realize this is only for STAGE. There is no line for production, so what happens there? Does Heroku use pip by default? 🤔 EDIT: I'm guessing so.

@flodolo
flodolo marked this pull request as ready for review September 20, 2024 05:31
Comment thread .eslintignore
coverage/
docs/_build/
docs/venv/
docs/*venv/

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

By default, uv creates a hidden .venv folder. I thought it would be nice to keep covering also venv, since I got bitten by this (same in .gitignore).

@mathjazz

mathjazz commented Sep 20, 2024

Copy link
Copy Markdown
Collaborator

Deployed to stage. The No change in requirements detected, installing from cache part is not what I expected.

I was expecting it, because I didn't change this line https://github.com/flodolo/pontoon/blob/replace_pip/bin/post_compile#L6

The No change in requirements detected, installing from cache refers to the requirements files.

Interestingly, I didn't realize this is only for STAGE. There is no line for production, so what happens there? Does Heroku use pip by default? 🤔 EDIT: I'm guessing so.

That line also didn't run... because we've had DJANGO_STAGE conveniently set to False. 😆

I've changed the setting now and installed again and now they've installed.

@flodolo

flodolo commented Sep 20, 2024

Copy link
Copy Markdown
Collaborator Author

The No change in requirements detected, installing from cache refers to the requirements files.

There are no new packages in the requirements though, only removed, so cache should still be valid.

@mathjazz

Copy link
Copy Markdown
Collaborator

Yeah, I guess it's smart enough in checking changes.

Comment thread docker/Dockerfile Outdated
COPY requirements/* /app/requirements/
RUN pip install -U 'pip>=8' && \
pip install --no-cache-dir --require-hashes -r requirements/default.txt -r requirements/dev.txt -r requirements/test.txt -r requirements/lint.txt
RUN uv pip install -U 'pip>=8' && \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Where is that uv coming from? We need to install it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That's a good question. How was this working locally? 🤔

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't see pip being install via apt-get above. Maybe it's already included in the base image?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah:

Sideshow:pontoon mathjazz$ docker run --rm python:3.11-bookworm pip --version
Unable to find image 'python:3.11-bookworm' locally
3.11-bookworm: Pulling from library/python
56c9b9253ff9: Already exists
364d19f59f69: Already exists
843b1d832182: Already exists
a348c2a8d946: Already exists
353de11681b2: Already exists
29b316cac89e: Already exists
e1860d8ed6fa: Already exists
Digest: sha256:157a371e60389919fe4a72dff71ce86eaa5234f59114c23b0b346d0d02c74d39
Status: Downloaded newer image for python:3.11-bookworm
pip 24.0 from /usr/local/lib/python3.11/site-packages/pip (python 3.11)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

But this is running uv pip, not pip.

Trying to figure this out, but it's less trivial than I expected (I can install uv and it works, but then it's not available when running make requirements, I think because of permissions)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I meant that pip is installed in Docker automatically with the base image we're using, so the old RUN pip install... command just worked.

You probably have uv installed locally, but not in Docker?

Maybe this helps?
https://github.com/astral-sh/uv?tab=readme-ov-file#installation

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

My comment How was this working locally? 🤔 was about running the code in this PR (e.g. uv pip install) within the Docker image.

When I tried to run make build this morning on a clean system (all builds/images removed), it failed because uv was not found, but I'm convinced I tested thoroughly this code before. AFAIK, there's no easy way to install uv in the Docker build (no SSH access, make shell is not root and I didn't find a quick way to run commands as root).

Comment thread docker/Dockerfile
&& apt-get autoremove -y

# Install uv
COPY --from=ghcr.io/astral-sh/uv:0.4.13 /uv /bin/uv

@flodolo flodolo Sep 20, 2024

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think I tried all possible ways to extract 0.4.13 into a variable (with both ENV and ARG), and failed.

ENV shouldn't be supported, ARG should work, but something this fails (warnings about UV_VERSION being empty, and then an error).

ARG UV_VERSION=0.4.13
FROM ghcr.io/astral-sh/uv:${UV_VERSION} AS uv_image
COPY --from=uv_image /uv /bin/uv

@flodolo flodolo left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

One thing I noticed is that make requirements is really fast now, compare to pip.

@mathjazz

mathjazz commented Sep 20, 2024

Copy link
Copy Markdown
Collaborator

This branch:

=> [server server 6/13] RUN uv pip install --system -r requirements/default.txt -r requirements/dev.txt -r requirements/test.txt 11.7s

Main:

=> [server 5/12] RUN pip install -U 'pip>=8' && pip install --no-cache-dir --require-hashes -r requirements/default.txt -r r 55.3s

@mathjazz

Copy link
Copy Markdown
Collaborator

Great work, this is a huge performance win for automation and local development. One day we'll benefit from it also on production.

@mathjazz
mathjazz merged commit e3e5935 into mozilla:main Sep 20, 2024
@flodolo
flodolo deleted the replace_pip branch September 27, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants