ci(deps): bump poetry version used by CI to 1.8.4 #128
Conversation
|
CI being incredibly red here is due to #126 not being merged, and also a showcase of why this change is being made in the first place. |
|
Warning Rate limit exceeded@aaronsteers has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request updates multiple GitHub Actions workflow files to change the version of Poetry from Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
.github/workflows/pdoc_preview.yml (1)
Line range hint
22-24: Consider using matrix strategy for Python version?Since this is a documentation workflow, would it make sense to align the Python version with the minimum supported version in pyproject.toml? Or perhaps use a matrix strategy if we need to verify docs generation across multiple Python versions? wdyt? 🤔
.github/workflows/pdoc_publish.yml (1)
36-36: LGTM! Poetry version update is consistent.The update to Poetry 1.8.4 matches other workflows and includes proper environment configuration. One tiny suggestion: would it make sense to add
actions: readto the permissions block for extra security? It's not strictly necessary but follows the principle of least privilege. wdyt? 🔒.github/workflows/python_lint.yml (1)
20-20: LGTM! Consider using a reusable workflow to reduce duplication, wdyt?The Poetry version update to 1.8.4 is consistent across all jobs. However, I notice that the setup steps are identical in all three jobs. To reduce maintenance overhead, we could consider extracting these common steps into a reusable workflow. This would make future version updates easier to manage. What do you think?
Also applies to: 43-43, 66-66
.github/workflows/poetry-lock-command.yml (1)
75-75: LGTM! Enhanced lock file validation with Poetry 1.8.4The update to Poetry 1.8.4 is particularly beneficial here since this version has improved lock file validation. This directly addresses the sync issues mentioned in #125. The workflow looks well-structured with proper error handling and status reporting.
Quick thought: Since this workflow is crucial for maintaining dependency consistency, should we add a comment in the workflow file documenting the minimum Poetry version requirement and why it's important? This could help future maintainers understand the version constraint. wdyt?
.github/workflows/test-command.yml (2)
Line range hint
71-76: Consider using reusable workflows for the setup stepsHey! 👋 I noticed we're duplicating some setup steps from
python_pytest.yml. Have you considered extracting these common steps into a reusable workflow? This could make maintenance easier and reduce duplication. Something like:jobs: setup: uses: ./.github/workflows/setup-python-env.yml with: python-version: ${{ matrix.python-version }} poetry-version: "1.8.4"What do you think about this approach? 🤔
Line range hint
127-143: Consider using GitHub Actions status check APIThe current implementation uses a raw curl command to post CI status. GitHub Actions provides a more idiomatic way to handle this through status checks. Would you be interested in a more maintainable approach using the built-in features? Here's what I'm thinking:
- name: Post CI Status uses: actions/github-script@v7 with: script: | await github.rest.repos.createCommitStatus({ owner: context.repo.owner, repo: context.repo.repo, sha: process.env.PR_SHA, state: 'success', context: 'Pytest (All, Python ${{ matrix.python-version }}, ${{ matrix.os }})', target_url: process.env.GITHUB_SERVER_URL + '/' + context.repo.owner + '/' + context.repo.repo + '/actions/runs/' + context.runId })This would give us better error handling and type safety. What are your thoughts on this? 🛠️
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
.github/workflows/autofix-command.yml(1 hunks).github/workflows/pdoc_preview.yml(1 hunks).github/workflows/pdoc_publish.yml(1 hunks).github/workflows/poetry-lock-command.yml(1 hunks).github/workflows/pytest_fast.yml(1 hunks).github/workflows/pytest_matrix.yml(1 hunks).github/workflows/python_lint.yml(3 hunks).github/workflows/test-command.yml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/autofix-command.yml
🔇 Additional comments (6)
.github/workflows/pdoc_preview.yml (1)
19-19: LGTM! Poetry version update looks good.
This update to Poetry 1.8.4 aligns with the PR objectives and should help prevent lock file sync issues.
.github/workflows/pytest_fast.yml (2)
39-39: LGTM! Poetry version update is consistent.
The update to Poetry 1.8.4 aligns with other workflows.
Line range hint 16-17: Verify dynamic versioning compatibility.
I notice we're using a dummy version for dynamic versioning. Let's verify this works with Poetry 1.8.4:
✅ Verification successful
The previous output showed that poetry-dynamic-versioning is configured, but we need to see the complete configuration and workflow files to understand how versioning is being handled. This will help us verify if the current setup is compatible with Poetry 1.8.4.
Dynamic versioning is properly configured and compatible with Poetry 1.8.4
The configuration is correctly set up:
- poetry-dynamic-versioning is listed in build-system.requires with compatible version constraint:
poetry-dynamic-versioning>=1.0.0,<2.0.0 - The build backend is properly configured:
poetry_dynamic_versioning.backend - Dynamic versioning is enabled in tool.poetry-dynamic-versioning
- The workflow is using Poetry 1.8.4 and successfully builds packages with dynamic versioning
The current setup is working as expected in the CI pipeline, no changes needed! 🎉
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if poetry-dynamic-versioning is listed in dependencies and its compatibility
# Check for poetry-dynamic-versioning in pyproject.toml
echo "Checking for poetry-dynamic-versioning configuration..."
rg -A 5 "dynamic-versioning" pyproject.toml
# Check if the package is listed in dependencies
echo "Checking dependencies..."
poetry show | grep "poetry-dynamic-versioning"
Length of output: 837
Script:
#!/bin/bash
# Let's check the pyproject.toml file to understand the Poetry configuration better
echo "Checking pyproject.toml content..."
cat pyproject.toml
# Also check if there are any GitHub workflow files that might be using Poetry
echo -e "\nChecking GitHub workflow files for Poetry usage..."
fd -e yml -e yaml . .github/workflows/ --exec cat {}
Length of output: 66642
.github/workflows/pytest_matrix.yml (1)
64-64: LGTM! Verify Poetry 1.8.4 compatibility with Python versions
The Poetry version update looks good. I notice we're testing against Python 3.10 and 3.11. Since Poetry 1.8.4 is relatively recent, shall we verify its compatibility with these Python versions? Also, there's a note about Python 3.12 being blocked by Pendulum - would you like me to help track when this limitation is resolved?
✅ Verification successful
Poetry 1.8.4 is compatible, but heads up on Pendulum constraint! 🎯
Poetry 1.8.4 fully supports Python 3.8-3.11 (requires >=3.8,<4.0), so we're good with our Python 3.10 and 3.11 matrix! The Python 3.12 comment is spot on - I see we're using Pendulum <3.0.0 which indeed doesn't support Python 3.12 yet. Would you like me to keep an eye on Pendulum's Python 3.12 support in future updates? 🔍
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Poetry 1.8.4 compatibility with Python versions
# Check Poetry's Python compatibility information
curl -s https://pypi.org/pypi/poetry/1.8.4/json | jq '.info.requires_python'
# Check if there are any known issues with Python 3.12
curl -s https://api.github.com/repos/python-poetry/poetry/issues\?labels=python-3.12
Length of output: 182
Script:
#!/bin/bash
# Let's verify the Python versions used in the workflow and check for any Pendulum-related constraints
# Check Python versions in the workflow file
echo "Python versions in workflow:"
grep -A 5 "python-version:" .github/workflows/pytest_matrix.yml
# Check Pendulum version and its Python requirements
echo -e "\nPendulum requirements:"
rg "pendulum" -A 2 pyproject.toml || echo "Pendulum not found in pyproject.toml"
# Alternative check for Pendulum in requirements
rg "pendulum" -A 2 requirements.txt || echo "Pendulum not found in requirements.txt"
Length of output: 814
.github/workflows/test-command.yml (2)
88-88: LGTM on the Poetry version update! 🎉
The update to Poetry 1.8.4 aligns perfectly with the PR objectives and should help prevent lock file synchronization issues.
Line range hint 44-54: Question about test matrix configuration
I notice we're explicitly excluding Windows from the test matrix. While the comment explains this is temporary, should we maybe track this limitation in an issue for future follow-up? This would help ensure we don't forget about cross-platform testing coverage, wdyt? 🤔
Fixes #125
I don't see a clean way of enforcing minimum poetry version by poetry itself until 2.0 is out: python-poetry/poetry#9547, so dev environments are not covered at this time.
Summary by CodeRabbit
Release Notes
New Features
Updates
Bug Fixes