-
Notifications
You must be signed in to change notification settings - Fork 18
chore: migrate to uv and simplify build workflow #107
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
Conversation
📝 WalkthroughWalkthroughAdds a reusable GitHub Action to set up Python and uv, migrates project metadata from Poetry to PDM/PEP621, replaces Poetry-based CI/workflow steps with uv-based commands, updates Makefile and demos to use uv workspaces, and expands .gitignore and demo docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Actions Runner
participant SetupAction as ./.github/actions/setup-python-env
participant SetupPy as actions/setup-python@v5
participant SetupUV as astral-sh/setup-uv@v5
participant UV as uv CLI
Runner->>SetupAction: invoke composite action (inputs: python-version, uv-version)
SetupAction->>SetupPy: install Python (python-version)
SetupAction->>SetupUV: install uv (uv-version), enable cache, pass cache-suffix
SetupAction->>UV: run `uv sync --all-groups --frozen`
Note right of Runner: subsequent job steps use installed Python and synced deps (e.g., `uv run pytest`)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/actions/setup-python-env/action.yml:
- Around line 4-12: The inputs block defines python-version and uv-version with
both default values and required: true, which is contradictory; update the
inputs (python-version and uv-version) to remove or set required to false so
they rely on their defaults (i.e., delete the required: true lines or change
them to required: false) so the action uses the default values when inputs are
not provided.
In @pyproject.toml:
- Around line 1-11: The version field in pyproject.toml uses a non-PEP 440
format ("1.0.0.b4"); update the version value in the version key to a PEP
440-compliant pre-release string (e.g., change the version value to "1.0.0b4")
so packaging and version comparisons behave correctly.
🧹 Nitpick comments (2)
Makefile (2)
3-6: Consider using--frozenfor reproducible installs.The CI action uses
uv sync --all-groups --frozento ensure reproducible installs fromuv.lock, but this target omits--frozen. This could lead to different behavior between local development and CI.If the intent is to allow lock file updates locally, this is fine as-is. Otherwise, consider aligning with CI:
🔎 Proposed fix
.PHONY: install install: ## Install the virtual environment @echo ">> Installing dependencies" - @$(UV) sync --all-groups + @$(UV) sync --all-groups --frozen
28-31: Consider usingrm -rffor simpler cleanup.Using
uv run python -cfor cleanup is unusual and adds overhead. A direct shell command would be more idiomatic for a Makefile:🔎 Proposed alternative
.PHONY: clean clean: ## Clean build and docs artifacts @echo ">> Removing build artifacts" - @$(UV) run python -c "import shutil; shutil.rmtree('dist', ignore_errors=True); shutil.rmtree('docs/_build', ignore_errors=True); shutil.rmtree('tests/seekdb.db', ignore_errors=True)" + @rm -rf dist docs/_build tests/seekdb.dbThis avoids requiring a Python environment just to delete directories.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/actions/setup-python-env/action.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/python-publish.yml.gitignoreMakefilepyproject.toml
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 33-33: Missing required phony target "all"
(minphony)
🔇 Additional comments (13)
.gitignore (2)
6-8: New build and documentation artifacts properly ignored.The additions of
dist/,docs/_build/, andtests/seekdb.db/are all appropriate for the new pdm-backend/uv build system and test infrastructure. They're correctly placed in the# projectsection.
18-18: Entry is correctly positioned.The
demo/rag/data/entry (reordered within the demo section) remains valid.Makefile (3)
8-16: LGTM!The test targets are well-structured with clear echo messages and appropriate pytest flags. The
-k embeddedfilter for integration tests correctly matches the CI workflow behavior.
18-21: LGTM!The docs target correctly mirrors the sphinx-build command used in the CI workflow.
33-37: LGTM!The help target with awk-based extraction and
.DEFAULT_GOAL := helpis a clean pattern for self-documenting Makefiles.Regarding the static analysis warning about missing "all" target: this is a stylistic preference, not a requirement. The explicit targets with
helpas default is appropriate for this project's use case.pyproject.toml (3)
12-21: LGTM!The dependency declarations are well-structured. The platform-specific marker for
pylibseekdbis correctly formatted, and usinguv.lockfor reproducibility makes unpinned versions acceptable here.
28-35: LGTM!The
[dependency-groups]section is the correct PEP 735 format for development dependencies, which uv supports natively.
37-46: LGTM!The pdm-backend configuration with source-includes and excludes is correctly set up for packaging.
.github/workflows/python-publish.yml (1)
26-33: LGTM!The workflow correctly uses the new composite action and
uv buildfor package building. The trusted publishing flow with PyPI remains intact..github/workflows/ci.yml (2)
23-30: LGTM!The unit test job correctly uses the composite action and
uv run pytestwith appropriate flags.
53-86: LGTM!The integration test job correctly uses the composite action. The test matrix with embedded/server/oceanbase modes and the pytest
-kfilter work well together..github/workflows/docs.yml (1)
37-44: LGTM!The docs workflow correctly uses the composite action and the sphinx-build command matches the Makefile's
docstarget..github/actions/setup-python-env/action.yml (1)
14-30: LGTM!The composite action is well-structured:
- Python setup followed by uv installation is the correct order
- Cache enablement with Python version suffix ensures proper cache isolation
--frozenflag ensures reproducible installs fromuv.lock
Signed-off-by: Chojan Shang <psiace@apache.org>
Remove seekdb.db from the ignored files list
hnwyllmm
left a comment
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.
LGTM
Summary
Migrate the project from Poetry to uv, align CI workflows with uv, and simplify local developer commands via a minimal Makefile.
Solution Description
Replace Poetry metadata with PEP 621 and
pdm-backend, introduceuv.lock, add a uv setup composite action for CI, update workflows to use uv, and provide a minimal uv-based Makefile plus packaging exclusions/gitignore updates.Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.