-
Notifications
You must be signed in to change notification settings - Fork 186
Migrate to uv #540
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
base: main
Are you sure you want to change the base?
Migrate to uv #540
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v5 | ||
| with: | ||
| python-version: ${{ env.PYTHON_VERSION }} |
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.
Smoke test fails: bare pytest call after venv activation removed
High Severity
The old CI workflows ran source .venv/bin/activate before make docker-test, putting pytest on PATH. This PR removes that activation and uses uv sync instead, but docker-smoke-test.sh (line 83) still calls bare pytest — not uv run pytest. Since uv sync creates a .venv without activating it, and setup-uv only adds uv to PATH, pytest won't be found. Every other pytest invocation in the Makefile was correctly updated to use uv run, but this script was missed.
Additional Locations (1)
1ebbd36 to
a0a07d3
Compare
|
@claude review thoroughly and examine edge cases and security considerations as well as any breaking changes |
|
Claude finished @aballman's task —— View job Thorough Review of uv Migration PR ✅I've completed a comprehensive review of this uv migration PR, examining security considerations, edge cases, and potential breaking changes. Review Checklist:
🔒 Security Assessment: GOOD✅ Dependency Supply Chain: Socket security scan shows clean results with high scores (99-100) for the new
|
Anything I can do to help with this? We have ARM runners available, both Github hosted and self-hosted. |
|
It would be cool to have this project use a |
| - name: Set up Python ${{ env.PYTHON_VERSION }} | ||
| run: uv python install ${{ env.PYTHON_VERSION }} | ||
| - name: Install lint dependencies | ||
| run: uv sync --only-group lint --frozen |
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.
CI lint mypy loses all third-party type information
Medium Severity
The CI lint job uses uv sync --only-group lint --frozen, which only installs ruff, mypy, and types-requests — none of the project's runtime dependencies. When mypy then runs with --ignore-missing-imports, it silently skips all imports from fastapi, unstructured, pypdf, pandas, etc. Since the source code in prepline_general/api/ heavily depends on these packages (many of which ship with inline types / py.typed), mypy effectively checks almost nothing meaningful. The previous CI setup installed all dependencies before linting, giving mypy full access to third-party type information. This trade-off isn't explicitly documented and significantly weakens the type-safety net.
Additional Locations (1)
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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| line-length = 100 | ||
| exclude = [ | ||
| "prepline_*/api", | ||
| ] |
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.
Ruff exclude scope broader than original flake8 exclude
Low Severity
The exclude list at the [tool.ruff] level applies to both ruff check (linting) and ruff format (formatting). The old setup only excluded prepline_*/api from flake8 linting, while black still enforced formatting on that directory. Placing the exclude under [tool.ruff.lint] instead of [tool.ruff] would preserve the old behavior where the API directory is excluded from linting but still format-checked.
4ad6f28 to
53ac67f
Compare


Note
Medium Risk
Touches core build/release infrastructure (dependency resolution, CI, Docker publishing, multi-arch images), so misconfigurations could break installs or releases despite minimal runtime code changes.
Overview
Migrates packaging and dependency management to
uvby introducingpyproject.toml(+ dependency groups forlint/test) anduv.lock, removing therequirements/*.in|*.txtworkflow, and updating Dependabot and the dependabot release workflow to bump viauv lock --upgrade.Modernizes CI/Docker: all GitHub Actions jobs move to self-hosted runners, read Python from the new
.python-version, and install/sync deps viauvwith caching; Docker build/publish re-enables multi-arch (amd64+arm64) builds and manifests, and the mainDockerfileswitches to bundling a pinneduvbinary, installing deps viauv sync, and fetching pandoc directly.Test reliability updates: Docker smoke tests are parallelized (one container per xdist worker/port) with container cleanup and retry logic for curl-based parallel-mode checks; OCR tests loosen brittle assertions to tolerate tesseract output variation. Also bumps version to
0.1.0and removes obsolete config/files (e.g.,setup.cfg, old Rocky Linux Dockerfile, pipeline version YAML).Written by Cursor Bugbot for commit 9d13a9e. This will update automatically on new commits. Configure here.