(feat)Python CLI: Switch from Poetry to UV for python package management #3410
Conversation
kevinjqliu
left a comment
kevinjqliu
left a comment
There was a problem hiding this comment.
generally lgtm, i would double check the build artifacts to make sure that its the same as packaged previously using poetry. That tripped us up during the pyiceberg migration
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # |
There was a problem hiding this comment.
looks like it just removes a duplicate header
There was a problem hiding this comment.
Intensional. This file has slightly diff license header (extra space after hashtag) compared to the rest of the python files.
Yes, I did that earlier. Most of them are same except wheel one doesn't have the root level init.py (which is fine). One thing caught me earlier was spec as that got excluded from hatch build due to we have spec in gitignore. |
dimas-b
left a comment
dimas-b
left a comment
There was a problem hiding this comment.
LGTM, but please wait for more reviews from people who deal with our python CLI more regularly than me :)
github-project-automation
bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
HonahX
left a comment
HonahX
left a comment
There was a problem hiding this comment.
Overall LGTM! Thanks for migrating this! I believe this could also make the release process easier in the future.
One thing I noticed was that the spark getting-started example now miss the ipynb in jupyter lab:
But this may not be relevant to this PR.
| requires = ["poetry-core==2.2.1", "openapi-generator-cli==7.12.0"] | ||
| build-backend = "poetry.core.masonry.api" | ||
| requires = ["hatchling", "openapi-generator-cli==7.12.0"] | ||
| build-backend = "hatchling.build" |
There was a problem hiding this comment.
This solves the problem with poetry.core.masonry.api that we cannot build universal wheel if using custom build script. I checked that now with make client-build, we will get
Successfully built dist/apache_polaris-1.4.0.tar.gz
Successfully built dist/apache_polaris-1.4.0-py3-none-any.whl
This could significantly simplify the nightly build/release process that we do not need the additional wheel build workflow anymore: #3036
| # This file, if checked in after running for example regtests, contains unmanaged dependencies that eventually | ||
| # cause unnecessary "security alerts" like https://github.com/apache/polaris/pull/718. |
There was a problem hiding this comment.
[Question] Should we keep this comment since it explains why we exclude `*.lock" file from git? To be honest, I also wondered for a while why poetry.lock was excluded intially.
4 participants
Per discussion in https://lists.apache.org/thread/2hlh3rvmgo7ol3qn08xmyf85grck2p35, team preferred
uvoverpoetry. Couple major changes with this PR:For the next PR, we can do more cleanup to remove unnecessary code/scripts around virtualenv management and let uv handles those natively if preferred. Currently, it is backward compatible as existed one in term of how to invoke the CLI and run docker-compose.
Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)