-
Notifications
You must be signed in to change notification settings - Fork 30
fix: replace flaky setup-poetry action with install-poetry #733
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
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@pedro/install-poetry#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch pedro/install-poetry Helpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
📝 WalkthroughWalkthroughReplaces the Poetry setup action across multiple GitHub Actions workflows from Changes
Sequence Diagram(s)(omitted — changes are action replacement/input rename only; no runtime/control-flow modifications to visualize) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Would you like me to also scan README or contributor docs for CI action references to update mentions of the old action, wdyt? ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (9)
.github/workflows/pytest_fast.yml (2)
18-20
: LGTM on action swap; would you like to pin to a commit for security?The change to
snok/install-poetry@v1
withversion: "2.0.1"
is correct. For defense-in-depth, pinning the action to a commit SHA could be worthwhile, wdyt?
45-47
: Second Poetry setup matches the new pattern — LGTM.Consistent with the first job; nice. Same optional suggestion about pinning to a commit SHA applies here, wdyt?
.github/workflows/pdoc_publish.yml (1)
34-36
: Docs workflow updated correctly; consider pinningsnok/install-poetry
to a commit, wdyt?Change looks good; the rest of the job (Python setup with Poetry cache) remains compatible.
.github/workflows/test-command.yml (1)
86-88
: On-demand test flow uses the new Poetry installer — LGTM; pin to commit for hardening?Everything else in the job continues to work with Poetry 2.0.1. Do you want to pin
snok/install-poetry
to a commit SHA to reduce supply-chain risk, wdyt?.github/workflows/poetry-lock-command.yml (1)
84-86
: Lock command workflow updated — LGTM; optional pin-to-SHA suggestion.Swap to
snok/install-poetry@v1
withversion
is correct. Given this job pushes commits, would you like to pin the action to a specific commit for extra safety, wdyt?.github/workflows/pytest_matrix.yml (1)
61-65
: Switch to snok/install-poetry looks good; consider pinning and preferring active Python.
- Nice swap; value preserved. Would you pin the action to a commit SHA instead of @v1 to tighten supply-chain security, wdyt?
- Since Poetry is installed before actions/setup-python in this job, would you add virtualenvs-prefer-active-python: true so Poetry reliably uses the matrix Python when creating the venv, wdyt?
Apply this minimal tweak:
- - name: Set up Poetry - uses: snok/install-poetry@v1 + - name: Set up Poetry + uses: snok/install-poetry@v1 if: steps.changes.outputs.src == 'true' with: - version: "2.0.1" + version: "2.0.1" + virtualenvs-prefer-active-python: true.github/workflows/pdoc_preview.yml (1)
17-19
: LGTM; mirror security pin and prefer active Python?
- Change is consistent with the PR’s goal. Would you pin snok/install-poetry to a commit SHA here too, wdyt?
- Optional: add virtualenvs-prefer-active-python: true to avoid the runner’s default Python influencing Poetry’s venv selection (harmless even though Python is set up right after), wdyt?
- - name: Set up Poetry - uses: snok/install-poetry@v1 + - name: Set up Poetry + uses: snok/install-poetry@v1 with: - version: "2.0.1" + version: "2.0.1" + virtualenvs-prefer-active-python: true.github/workflows/connector-tests.yml (1)
144-147
: Good swap; add pinning and consider caching to speed this long job.
- Action switch + version mapping looks correct. Would you pin snok/install-poetry to a commit SHA, wdyt?
- This job doesn’t use actions/setup-python cache: "poetry". Adding it can shave minutes off installs—OK to include, wdyt?
- Also, to ensure Poetry uses 3.11 for venvs even if other Pythons are present, would you add virtualenvs-prefer-active-python: true?
Minimal tweak within this step:
- - name: Set up Poetry - if: steps.no_changes.outputs.status != 'cancelled' - uses: snok/install-poetry@v1 + - name: Set up Poetry + if: steps.no_changes.outputs.status != 'cancelled' + uses: snok/install-poetry@v1 with: - version: "2.0.1" + version: "2.0.1" + virtualenvs-prefer-active-python: trueIf you’re open to caching, in the “Set up Python” step above add:
- name: Set up Python if: steps.no_changes.outputs.status != 'cancelled' uses: actions/setup-python@v5 with: python-version: "3.11" + cache: "poetry"
.github/workflows/python_lint.yml (1)
18-21
: Consistent action update; confirm mixed Poetry versions are intentional and consider pinning.
- Nice, all three jobs use snok/install-poetry. Would you pin to a commit SHA across the board, wdyt?
- You’re using Poetry 2.0.1 for ruff-lint but 1.8.4 for ruff-format and mypy. Is the split intentional to keep parity with prior runs or toolchain quirks? If not, standardizing on one Poetry version could reduce drift, wdyt?
- Optional: add virtualenvs-prefer-active-python: true to each step to consistently target the job’s Python.
Example tweak for one job (repeat similarly for the others if you agree):
- name: Set up Poetry - uses: snok/install-poetry@v1 + uses: snok/install-poetry@v1 with: version: "2.0.1" + virtualenvs-prefer-active-python: trueAlso applies to: 41-44, 64-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
.github/workflows/connector-tests.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_dependency_analysis.yml
(1 hunks).github/workflows/python_lint.yml
(3 hunks).github/workflows/test-command.yml
(1 hunks)
⏰ 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). (2)
- GitHub Check: Pytest (Fast)
- GitHub Check: Manifest Server Docker Image Build
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.
Awesome 👍😎
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: 0
🧹 Nitpick comments (2)
.github/workflows/autofix-command.yml (2)
75-79
: Consider pinning the action to a commit SHA for supply-chain hardening.Since this PR is labeled security, would you pin
snok/install-poetry
to a specific commit SHA instead of a moving tag to mitigate tampering risks, wdyt?- uses: snok/install-poetry@v1 + uses: snok/install-poetry@<commit-sha> # v1
75-79
: Ensure Poetry uses the matrix Python and that caching remains effective.To avoid Poetry selecting a different interpreter on the runner and to help
actions/setup-python
’scache: "poetry"
work as intended, would you add the following inputs, wdyt?
- Prefer the active Python set by setup-python.
- Optionally keep venvs outside the project so cache keys target Poetry’s default venv location.
with: - version: "2.0.1" + version: "2.0.1" + virtualenvs-prefer-active-python: true + # Optional: uncomment if you want setup-python caching to target Poetry’s default venvs dir + # virtualenvs-in-project: falseAlso, if you’d like, we could add a tiny debug step after setup to print
poetry --version && poetry config --list
to confirm the effective config. Wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/autofix-command.yml
(1 hunks)
⏰ 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)
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (Fast)
🔇 Additional comments (1)
.github/workflows/autofix-command.yml (1)
75-79
: Switch to snok/install-poetry looks good.Using
snok/install-poetry@v1
withversion: "2.0.1"
is correct and aligns with the PR goal. Nice, this should reduce flakiness. Wdyt?
The old one kept randomly timing out like so:
https://github.com/airbytehq/airbyte-python-cdk/actions/runs/17281035642/job/49049280943
This PR replaces
setup-poetry
withinstall-poetry
, which is used in our other repos and has more stars.Summary by CodeRabbit
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.