chore: use poetry 2.0 #209
Conversation
|
Warning Rate limit exceeded@aaronsteers has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 44 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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request focuses on updating the Poetry version from 1.8.4 to 2.0.0 across multiple GitHub Actions workflow files and the project's Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
What do you think about these suggested reviewers? Do they seem like a good fit for this PR? wdyt? Finishing Touches
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
🧹 Nitpick comments (1)
.github/workflows/autofix-command.yml (1)
Line range hint
2-2: Consider documenting Poetry 2.0 migration stepsSince Poetry 2.0 might introduce breaking changes, would you like me to help create a migration guide in the repository's documentation? This could help other contributors understand any behavioral changes in dependency resolution, lock file format, or plugin compatibility. WDYT?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.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(2 hunks).github/workflows/pytest_matrix.yml(1 hunks).github/workflows/python_lint.yml(3 hunks).github/workflows/test-command.yml(1 hunks)pyproject.toml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (9)
.github/workflows/pdoc_preview.yml (1)
19-19: Hey there! Quick heads-up about Poetry 2.0 and --all-extras 🎵Poetry 2.0 has some changes in how
--all-extrasworks. Would it make sense to verify that all required extras for doc generation are still being installed correctly? We could add a quick test to ensure the docs build successfully with the new version, wdyt?.github/workflows/pdoc_publish.yml (1)
36-36: Same consideration as in pdoc_preview.yml.github/workflows/python_lint.yml (1)
20-20: Quick thought about Poetry 2.0 and dev dependencies 🤔Poetry 2.0 has some changes in dependency resolution. Should we verify that all linting tools (ruff, mypy) are still being installed correctly with their exact versions? Here's a quick script to check:
Also applies to: 43-43, 66-66
✅ Verification successful
All good with Poetry 2.0 and linting tools! ✅
The dev dependencies are properly specified in pyproject.toml and compatible with Poetry 2.0. Ruff is pinned to ^0.7.2 and mypy's version is flexible, which is perfectly fine for development tools.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if linting tools are installed with correct versions echo "Checking development dependencies..." poetry show | grep -E "ruff|mypy"Length of output: 215
Script:
#!/bin/bash # Find and show pyproject.toml contents fd pyproject.toml --type f --exec cat {}Length of output: 8799
.github/workflows/pytest_fast.yml (1)
20-20: Heads up about Poetry 2.0 build system requirements! 🏗️Poetry 2.0 might need an update to
poetry-coreinpyproject.toml. Should we verify that the build system requirements are compatible? Here's a quick check:Also applies to: 47-47
✅ Verification successful
All good with Poetry 2.0 build requirements! ✨
The
pyproject.tomlis already set up withpoetry-core>=2.0.0, which is perfect for Poetry 2.0. You're all set! 🚀🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check build system requirements in pyproject.toml echo "Checking build system requirements..." grep -A 2 "\[build-system\]" pyproject.tomlLength of output: 274
.github/workflows/pytest_matrix.yml (1)
64-64: Quick check on Poetry 2.0 Python compatibility 🐍Poetry 2.0 might have different Python version support. Should we verify that all Python versions in the matrix (3.10, 3.11) are fully supported by Poetry 2.0? Here's a quick check:
✅ Verification successful
Poetry 2.0 looks good with your Python matrix! 👍
The Python versions in your matrix (3.10, 3.11) are fully supported by Poetry 2.0. I see you've already handled the Python 3.12 limitation due to Pendulum dependency - nice catch!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check Poetry's Python compatibility poetry debug info | grep "Python"Length of output: 130
Script:
#!/bin/bash # Check Python versions in matrix configuration cat .github/workflows/pytest_matrix.ymlLength of output: 3258
pyproject.toml (1)
2-2: Verify poetry-dynamic-versioning compatibility with Poetry 2.0The poetry-core requirement has been updated correctly, but let's verify the compatibility of poetry-dynamic-versioning with Poetry 2.0. Would you like me to check the compatibility matrix?
✅ Verification successful
poetry-dynamic-versioning is compatible with Poetry 2.0
The release notes confirm explicit support for Poetry 2.0, with specific features and adjustments made for compatibility. The current constraint
>=1.0.0,<2.0.0is valid for use with Poetry 2.0.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check poetry-dynamic-versioning compatibility with Poetry 2.0 # Expected: Find any documented version compatibility information echo "Checking poetry-dynamic-versioning releases for Poetry 2.0 compatibility..." gh api repos/mtkennerly/poetry-dynamic-versioning/releases | jq -r '.[].body' | grep -i "poetry.*2\.0"Length of output: 766
.github/workflows/poetry-lock-command.yml (1)
75-75: Verify setup-poetry action compatibilityThe Poetry version has been updated, but let's verify if Gr1N/setup-poetry@v9 fully supports Poetry 2.0.0. Shall we check the action's compatibility?
✅ Verification successful
Poetry 2.0.0 should work fine with setup-poetry@v9 🎯
The action is version-agnostic and simply passes through whatever version you specify. Since it's actively maintained (recently upgraded to node20) and doesn't impose any version constraints, it should work perfectly fine with Poetry 2.0.0. Would you like me to verify anything else about the Poetry setup?
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check setup-poetry action compatibility with Poetry 2.0 # Expected: Find any version compatibility information echo "Checking setup-poetry action releases for Poetry 2.0 compatibility..." gh api repos/Gr1N/setup-poetry/releases | jq -r '.[].body' | grep -i "poetry.*2\.0"Length of output: 234
Script:
#!/bin/bash # Check setup-poetry action's source code for version handling echo "Checking action.yml for version handling..." curl -s https://raw.githubusercontent.com/Gr1N/setup-poetry/master/action.yml echo -e "\nChecking latest commits for Poetry 2.0 updates..." curl -s https://api.github.com/repos/Gr1N/setup-poetry/commits?per_page=5 | jq -r '.[].commit.message'Length of output: 1071
.github/workflows/test-command.yml (1)
88-88: LGTM! Poetry version update is consistentThe Poetry version update is consistent with other workflow files.
.github/workflows/autofix-command.yml (1)
78-78: LGTM! Poetry version update is consistentThe Poetry version update is consistent with other workflow files.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
pyproject.toml (3)
61-87: Optional dependencies are well-categorized! Quick thought on versions?Love the organization into logical groups! 🎯 Just wondering about some exact pins in the file-based group:
unstructured[docx,pptx] == 0.10.27 pytesseract == 0.3.10 pdf2image == 1.16.3Would allowing patch updates (
~=instead of==) work here? This could help with bug fixes while maintaining compatibility.
100-119: Dev dependencies look solid, but shall we pin some wildcards?I noticed a few dependencies using
"*":freezegun = "*" mypy = "*" pympler = "*" pytest-cov = "*" pytest-httpserver = "*" pytest-mock = "*" requests-mock = "*"For better reproducibility in development environments, would you consider adding version constraints for these? Happy to help suggest specific versions if you'd like! 🤝
97-98: Consider moving requires-poetry to a more standard locationThe
requires-poetrysetting under[tool.poetry]seems a bit lonely. Since we're migrating to Poetry 2.0, would it make more sense to move this to the[build-system]section or remove it if not needed? wdyt? 🤔
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
pyproject.toml(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Linters
pyproject.toml
[error] 23-23: Failed to parse version specifier in requires-python field. Invalid version format: '^3.10,<3.13'
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
🔇 Additional comments (3)
pyproject.toml (3)
1-4: LGTM! Build system configuration is properly updated for Poetry 2.0 🎯The update to
poetry-core>=2.0.0aligns perfectly with the Poetry 2.0 migration objective.
Line range hint
5-22: Project metadata looks great with Poetry 2.0 structure! 🎉Nice work moving from
[tool.poetry]to[project]and updating the authors format to the new dictionary style.
25-59: Dependencies look well-structured, but shall we discuss some version constraints?The dependencies are nicely organized, but I noticed some exact version pins that might be worth reviewing:
pandas ==2.2.2psutil ==6.1.0wcmatch ==10.0pytz ==2024.2Would it make sense to allow patch updates for these? For example:
pandas ~=2.2.2, wdyt? 🤔
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pyproject.toml (2)
25-59: Dependencies look well-organized, but some constraints might be too strict 🤔I notice several exact version pins (e.g.,
psutil ==6.1.0,pandas ==2.2.2). While this ensures reproducibility, it might make future updates more challenging. Would you consider using compatible release operators (~=) for some of these to allow patch updates while maintaining stability, wdyt?For example:
- "psutil ==6.1.0", + "psutil ~=6.1.0", - "pandas ==2.2.2", + "pandas ~=2.2.0",🧰 Tools
🪛 GitHub Actions: PyTest Matrix
[error] Lock file is out of sync with pyproject.toml. Run 'poetry lock' to update poetry.lock file.
🪛 GitHub Actions: Pytest (Fast)
[error] pyproject.toml changed significantly since poetry.lock was last generated. Run
poetry lockto fix the lock file.
101-119: Dev dependencies could use more consistent version patterns 🎨I notice we're mixing wildcards (
*) with specific constraints. For better reproducibility, should we consider:
- Using specific constraints for all deps instead of wildcards?
- Adding upper bounds to prevent breaking changes?
Example:
-freezegun = "*" +freezegun = ">=1.1.0, <2.0" -mypy = "*" +mypy = ">=1.8.0, <2.0"🧰 Tools
🪛 GitHub Actions: PyTest Matrix
[error] Lock file is out of sync with pyproject.toml. Run 'poetry lock' to update poetry.lock file.
🪛 GitHub Actions: Pytest (Fast)
[error] pyproject.toml changed significantly since poetry.lock was last generated. Run
poetry lockto fix the lock file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: PyTest Matrix
pyproject.toml
[error] Lock file is out of sync with pyproject.toml. Run 'poetry lock' to update poetry.lock file.
🪛 GitHub Actions: Pytest (Fast)
pyproject.toml
[error] pyproject.toml changed significantly since poetry.lock was last generated. Run poetry lock to fix the lock file.
🔇 Additional comments (3)
pyproject.toml (3)
Line range hint
5-23: Project metadata looks great! 👍The migration to the new Poetry 2.0
[project]format is well-structured. I particularly like how you've organized the metadata fields and fixed the Python version specifier format.🧰 Tools
🪛 GitHub Actions: PyTest Matrix
[error] Lock file is out of sync with pyproject.toml. Run 'poetry lock' to update poetry.lock file.
🪛 GitHub Actions: Pytest (Fast)
[error] pyproject.toml changed significantly since poetry.lock was last generated. Run
poetry lockto fix the lock file.
Line range hint
133-219: Tool configurations are comprehensive and well-documented! 🌟Great job on organizing the poe tasks and providing clear documentation for each task. The pytest and CI configurations are also well-structured.
🧰 Tools
🪛 GitHub Actions: PyTest Matrix
[error] Lock file is out of sync with pyproject.toml. Run 'poetry lock' to update poetry.lock file.
🪛 GitHub Actions: Pytest (Fast)
[error] pyproject.toml changed significantly since poetry.lock was last generated. Run
poetry lockto fix the lock file.
2-2: LGTM on poetry-core upgrade! 🎉The upgrade to
poetry-core>=2.0.0aligns with Poetry 2.0 requirements. However, shall we verify if there's a recommended upper bound for poetry-core 2.x, wdyt?✅ Verification successful
The poetry-core version constraint looks good! ✨
The
>=2.0.0constraint is appropriate here since poetry-core is a stable build dependency, and no security vulnerabilities or compatibility issues were found. The absence of an upper bound allows for seamless updates within the 2.x series, which is managed by Poetry itself.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any known compatibility issues with poetry-core versions gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: PIP, package: "poetry-core") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 344
Script:
#!/bin/bash # Search for poetry-core version patterns in documentation and configuration files rg -i "poetry.?core.*[0-9]" --type md --type toml -A 2 -B 2 # Also check if there are any other pyproject.toml files that might provide insights fd pyproject.toml --type f --exec head -n 20 {}Length of output: 1003
🧰 Tools
🪛 GitHub Actions: PyTest Matrix
[error] Lock file is out of sync with pyproject.toml. Run 'poetry lock' to update poetry.lock file.
🪛 GitHub Actions: Pytest (Fast)
[error] pyproject.toml changed significantly since poetry.lock was last generated. Run
poetry lockto fix the lock file.
|
/poetry-lock
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/connector-tests.yml (2)
119-123: The Python setup looks good! Would you consider a small enhancement? 🤔The Python setup with Poetry caching is well configured. Since this is part of the Poetry 2.0 migration, would you consider adding Poetry installation caching as well? This could further speed up the workflow, wdyt?
Here's a suggestion to enhance caching:
with: python-version: "3.10" - cache: "poetry" + cache: "poetry" + cache-dependency-path: | + poetry.lock + **/poetry.lock
119-123: Quick thought about Python version management 🤔The Python version is currently hardcoded to "3.10". Would it make sense to use a matrix strategy or move this to a shared variable that can be reused across workflows? This could make version updates easier to manage in the future, wdyt?
Example approach:
env: PYTHON_VERSION: "3.10" # Then in the step: with: python-version: ${{ env.PYTHON_VERSION }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
.github/workflows/connector-tests.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (Fast)
Context from Slack (internal):
Warning: Sharp Edges
After going through the migration steps here in this PR, the only sharp edges I'm seeing so far after post-migration is that developers will now need 2 versions of Poetry installed on their machines.
As noted below, we can use
pipx install "poetry>=2.0" --suffix=2to then havepoetry2as a side-by-side alternative topoetry, but the user will need to remember which to use.Mitigation:
poetasks calledinstallandlock, in which casepoe installandpoe lockcould smartly usepoetryorpoetry2, on a per project basis.Poetry 2.0 Migration Steps
I. Update dependencies to use poetry 2.0
Update CI workflows to use the new Poetry version.
II. Install Poetry 2 locally
Installing this way allows you to have
poetryandpoetry2installed side-by-side without conflicts:pipx install "poetry>=2.0" --suffix=2III. Update pyproject.toml.
III.I: Start with
poetry2 checkIII.II. Resolve check failures until all is clear and until both
poetry2 lockandpoetry2 installwork correctly.III.III. Dependency Re-Declaration
There's apparently a new PEP (PEP 508) for how project dependencies should be declared, and carrot ("^") is not suppported. Poetry has a very nice and concise summary of this change in their docs:
Note that there's no native PEP standard for declaring dev dependencies. So, for now, I think they need to keep with the old declaration syntax when defining dev dependencies.