ci: Fix upgrade depenecies workflow #1083
Conversation
WalkthroughThe project has undergone a significant overhaul in its dependency management and upgrade mechanisms. The changes include an update to Python 3.11, the adoption of a new dependency management tool named Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Type: Enhancement
PR Summary: This pull request addresses issues introduced by a previous PR that broke the upgrade dependencies workflow. It updates the workflow to use a newer version of Python (3.11) exclusively, simplifies the dependency upgrade commands by utilizing a new tool, and transitions the package configuration parsing from 'setup.cfg' to 'pyproject.toml'.
Decision: Comment
📝 Type: 'Enhancement' - not supported yet.
- Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
✅ Small diff: the diff is small enough to approve with confidence.
No details provided.
General suggestions:
- Ensure that the exclusive use of Python 3.11 aligns with the project's compatibility and support strategy. Dropping support for older Python versions should be a deliberate decision.
- Consider the impact of changing the dependency upgrade tool on the workflow's maintainability and ease of use for other contributors.
- Verify that the transition to 'pyproject.toml' for package configuration parsing does not affect other parts of the project that may rely on 'setup.cfg'.
- Review the new dependency upgrade commands for any potential edge cases or errors that could arise from the changes.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .github/workflows/upgrade-dependencies.yml (2 hunks)
- build_utils/check_updated_packages.py (2 hunks)
Additional comments: 1
build_utils/check_updated_packages.py (1)
- 7-7: The import of
tomlicorrectly replacesConfigParser, aligning with the PR's objective to modernize the parsing of project configuration files.
| config = loads((src_dir / "pyproject.toml").read_text()) | ||
|
|
||
| metadata = config["project"] | ||
|
|
||
| packages = ( | ||
| config["options"]["install_requires"].split("\n") | ||
| + config["options.extras_require"]["pyinstaller"].split("\n") | ||
| + config["options.extras_require"]["all"].split("\n") | ||
| metadata["dependencies"] | ||
| + metadata["optional-dependencies"]["pyinstaller"] | ||
| + metadata["optional-dependencies"]["all"] |
There was a problem hiding this comment.
The transition to reading and parsing pyproject.toml using tomli is correctly implemented. However, there are a few considerations for improvement:
- Error handling for file reading and TOML parsing should be added to catch and handle potential IO or parsing errors gracefully.
- The extraction of dependencies assumes the presence of certain keys (
dependencies,optional-dependencies) in the TOML file. It's advisable to check for the existence of these keys before accessing them to avoidKeyError.
+ try:
config = loads((src_dir / "pyproject.toml").read_text())
+ except (FileNotFoundError, IOError) as e:
+ print(f"Error reading pyproject.toml: {e}", file=sys.stderr)
+ sys.exit(1)
+ except tomli.TOMLDecodeError as e:
+ print(f"Error parsing pyproject.toml: {e}", file=sys.stderr)
+ sys.exit(1)
+ if "project" not in config:
+ print("Missing 'project' section in pyproject.toml", file=sys.stderr)
+ sys.exit(1)
metadata = config["project"]
+ if "dependencies" not in metadata or "optional-dependencies" not in metadata:
+ print("Missing dependencies information in pyproject.toml", file=sys.stderr)
+ sys.exit(1)
packages = (
metadata["dependencies"]
+ metadata.get("optional-dependencies", {}).get("pyinstaller", [])
+ metadata.get("optional-dependencies", {}).get("all", [])
)
+ ```
<!-- This is an auto-generated comment by CodeRabbit -->
---
<!-- suggestion_start -->
<details>
<summary>Committable suggestion</summary>
> :bangbang: **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
```suggestion
try:
config = loads((src_dir / "pyproject.toml").read_text())
except (FileNotFoundError, IOError) as e:
print(f"Error reading pyproject.toml: {e}", file=sys.stderr)
sys.exit(1)
except tomli.TOMLDecodeError as e:
print(f"Error parsing pyproject.toml: {e}", file=sys.stderr)
sys.exit(1)
if "project" not in config:
print("Missing 'project' section in pyproject.toml", file=sys.stderr)
sys.exit(1)
metadata = config["project"]
if "dependencies" not in metadata or "optional-dependencies" not in metadata:
print("Missing dependencies information in pyproject.toml", file=sys.stderr)
sys.exit(1)
packages = (
metadata["dependencies"]
+ metadata.get("optional-dependencies", {}).get("pyinstaller", [])
+ metadata.get("optional-dependencies", {}).get("all", [])
)| # ADD YOUR CUSTOM DEPENDENCY UPGRADE COMMANDS BELOW | ||
| run: | | ||
| set -x | ||
| flags="--extra pyqt5 --extra pyqt6 --extra pyside2 --extra pyside6" | ||
| flags+=" --extra test --extra pyinstaller --allow-unsafe --strip-extras --resolver=backtracking" | ||
| pip install -U uv | ||
| set -x | ||
| flags=(--extra pyqt5 --extra pyqt6 --extra pyside2 --extra pyside6 --extra test --extra pyinstaller) | ||
|
|
||
| for pyv in 3.8 3.9 3.10 3.11; do | ||
| python${pyv} -m pip install -U pip pip-tools | ||
| python${pyv} -m piptools compile --upgrade -o requirements/constraints_py${pyv}.txt pyproject.toml requirements/version_denylist.txt ${flags} | ||
| uv pip compile --python-version ${pyv} --upgrade --output-file requirements/constraints_py${pyv}.txt pyproject.toml requirements/version_denylist.txt "${flags[@]}" | ||
| done | ||
| python3.11 -m piptools compile --upgrade -o requirements/constraints_py3.11_docs.txt pyproject.toml requirements/version_denylist.txt --allow-unsafe --strip-extras --extra docs --extra pyqt6 | ||
| set +x | ||
| uv pip compile --python-version 3.11 --upgrade --output-file requirements/constraints_py3.11_docs.txt pyproject.toml requirements/version_denylist.txt --extra docs --extra pyqt6 | ||
| # END PYTHON DEPENDENCIES | ||
|
|
||
| - name: Check updated packages |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [25-43]
The update to Python 3.11 and the introduction of the uv tool for dependency management are significant improvements. However, there are a few areas that could be refined:
- The use of
set -xbefore running commands is good for debugging, but consider if this level of verbosity is always desired in the workflow logs. - The loop over Python versions (
for pyv in 3.8 3.9 3.10 3.11; do) to upgrade dependencies for each version is a robust approach, ensuring compatibility across multiple Python versions. However, ensure that all specified Python versions are supported by the project's dependencies. - The
--extraflags used withuv pip compileshould be verified to ensure they match the project's actual optional dependencies as defined inpyproject.toml.
- set -x
+ # Consider removing or conditionally using `set -x` for cleaner logs
pip install -U uv
- set -x
+ # Ensure verbosity is controlled and necessary
flags=(--extra pyqt5 --extra pyqt6 --extra pyside2 --extra pyside6 --extra test --extra pyinstaller)
for pyv in 3.8 3.9 3.10 3.11; do
uv pip compile --python-version ${pyv} --upgrade --output-file requirements/constraints_py${pyv}.txt pyproject.toml requirements/version_denylist.txt "${flags[@]}"
done
set +x
uv pip compile --python-version 3.11 --upgrade --output-file requirements/constraints_py3.11_docs.txt pyproject.toml requirements/version_denylist.txt --extra docs --extra pyqt6Additionally, consider adding comments or documentation within the workflow file to explain the purpose and usage of the uv tool and the chosen flags for future maintainers.
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .github/workflows/upgrade-dependencies.yml (2 hunks)
- build_utils/check_updated_packages.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- build_utils/check_updated_packages.py
Additional comments: 6
.github/workflows/upgrade-dependencies.yml (6)
- 25-25: The transition to Python 3.11 is a significant update. Ensure that all Python dependencies and any Python code within the project are compatible with Python 3.11 to avoid runtime issues.
- 35-35: The installation of the
uvtool is a new addition. Verify thatuvis compatible with the project's infrastructure and that it provides the expected benefits over previous tools used for managing dependencies.- 36-41: The use of
uvfor upgrading Python dependencies introduces a new approach. Ensure that the flags and Python versions specified are aligned with the project's requirements. Additionally, verify that the output files (constraints_py${pyv}.txtandconstraints_py3.11_docs.txt) are correctly utilized in the project's dependency management process.- 32-44: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [25-41]
The caching strategy now relies on
pyproject.toml. Confirm that all relevant dependencies are correctly listed inpyproject.tomland that the cache will be effectively utilized to speed up future runs of this workflow.
- 32-44: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [25-41]
The workflow includes a step to check updated packages using a script from
build_utils/check_updated_packages.py. Given the changes to dependency management, ensure that this script is fully compatible with the new setup, especially the transition totomliand the focus onpyproject.toml.
- 32-44: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [25-41]
The creation of a pull request for updating vendored modules uses
peter-evans/create-pull-request@v6. Confirm that this action version is compatible with the updated workflow and that it correctly handles the creation and deletion of branches, especially with the new dependency management approach.
The PR #1076 has broken upgrade dependence workflow. This PR is fixing it.
Summary by CodeRabbit