Repository navigation
📝 Fix stale requirements.txt install command in the completion container guide - #1981
HyperionBurn wants to merge 1 commit into
Conversation
📝 Docs previewLast commit 7963544 at: https://66557a93.typertiangolo.pages.dev Modified Pages |
…ner guide The migration to uv (fastapi#1472) deleted the root requirements.txt, but two references to it were left behind: the install step in docs/contributing.md and the build step in scripts/docker/Dockerfile. Since scripts/docker/compose.yaml builds from that Dockerfile, `docker compose run typer bash` currently fails at image build time with the file not found, so the documented workflow for testing shell completion cannot be followed at all. The deleted requirements.txt started with `-e .` plus the tests/docs requirement files. For this container only the project itself is needed, so `uv pip install -e .` keeps the exact previous behaviour of installing Typer into the system Python, where ENV UV_SYSTEM_PYTHON=1 makes it land on PATH for `typer --install-completion`.
a27caec to
7963544
Compare
svlandeg
left a comment
There was a problem hiding this comment.
Thanks for the PR! For future reference, please follow the contribution guidelines by opening a discussion thread first, and declaring AI usage (incl if none).
Anyway this looks good to me, but pinging @DoctorJohn whether he can sign of on this 🙏
| WORKDIR /code | ||
|
|
||
| RUN uv pip install -r requirements.txt | ||
| RUN uv pip install -e . |
There was a problem hiding this comment.
The original requirements.txt file also installed the tests and docs dependency groups. So technically, we should either go with:
| RUN uv pip install -e . | |
| RUN uv sync |
or, if we want to use the pip interface:
| RUN uv pip install -e . | |
| RUN uv pip install --group tests --group docs --editable . |
I'd go with the first option, which syncs the projects dependencies and the dev dependency group, which also includes the tests and docs dependency groups.
Tho, I must admit I'm not even sure this Dockerfile is even used anymore.
What
The migration to
uvin #1472 removed the rootrequirements.txt, but two references to it were left behind:docs/contributing.md— the "install typer inside the container" stepscripts/docker/Dockerfile— the finalRUNstepBecause
scripts/docker/compose.yamlbuilds directly from that Dockerfile:docker compose run typer bashcurrently fails while building the image, because the file theRUNstep reads no longer exists. That is the documented entry point for testing shell completion, so the whole workflow is blocked at the first command.This replaces both with
uv pip install -e ..Why
uv pip install -e .and notuv syncThe deleted
requirements.txtwas:So the previous behaviour was an editable install of the project itself into the container's Python, plus the test/docs/prek extras. The Dockerfile sets
ENV UV_SYSTEM_PYTHON=1, which is what makesuv pip installtarget the system interpreter — that is howtyperended up onPATH, which the rest of the guide depends on fortyper --install-completionandtyper demo.py <TAB>.uv pip install -e .preserves that exactly. I deliberately did not useuv sync, because it creates a project virtualenv at/code/.venvand would leavetyperoffPATHfor the steps that follow in the guide. The test/docs/prek extras are not needed here — this container is only used to exercise shell completion.How this was verified
requirements.txtis absent frommasterand from the working tree.39353af("⬆️ Migrate to uv", ⬆️ Migrate to uv #1472) showsrequirements.txt [removed]along withrequirements-tests.txt,requirements-docs.txtandrequirements-docs-tests.txt. That same commit already updated the other install section indocs/contributing.mdtouv sync, but did not touch the container guide or the Dockerfile.a6a2a37to confirm it started with-e ..requirements.txtreference; only the historical entry indocs/release-notes.mdremains, which is correct as a changelog record.Only two lines change.