-
Notifications
You must be signed in to change notification settings - Fork 30
feat: add Python 3.12 and 3.13 support #684
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
- Update Python version constraint from <3.13 to <3.14 in pyproject.toml - Add Python 3.12 and 3.13 to pytest matrix workflows - Update airbyte_ci python_versions configuration - Update connector-tests.yml to use Python 3.12 - Remove requests-cache blocker comment (version 1.2.1 supports Python 3.12+) - Update poetry.lock with new dependency resolution Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
Original prompt from AJ Steers
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 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@devin/1753929340-python-3.13-support#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 devin/1753929340-python-3.13-support Helpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
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.
Pull Request Overview
This PR adds Python 3.12 and 3.13 support to the airbyte-python-cdk by updating version constraints and CI test configurations. The changes enable the CDK to work with the latest Python versions while maintaining compatibility with existing supported versions.
- Updated Python version constraint from
>=3.10,<3.13
to>=3.10,<3.14
to include Python 3.12 and 3.13 - Added Python 3.12 and 3.13 to CI test matrices across multiple workflow files
- Updated default Python version in connector tests from 3.11 to 3.12
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
pyproject.toml | Updated Python version constraint and airbyte_ci configuration to support Python 3.12 and 3.13 |
.github/workflows/pytest_matrix.yml | Added Python 3.12 and 3.13 to test matrix, removed requests-cache blocker comment |
.github/workflows/test-command.yml | Extended test matrix to include Python 3.12 and 3.13 |
.github/workflows/connector-tests.yml | Updated default Python version from 3.11 to 3.12 |
airbyte_cdk/manifest_migrations/README.md | Fixed escaped asterisk in documentation example |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughUpdates expand Python support to 3.12 and 3.13 in CI and tooling, widen pyproject constraints, refresh selected dependencies, adjust CI settings, add conditional test skip for Python 3.12+ due to SQLite threading, loosen one test’s assertions, and make minor README and action formatting tweaks. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Friendly nudge: Should the duplicate skip decorator in test_concurrent_declarative_source.py be consolidated into a single occurrence to avoid confusion, 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 (
|
Address PR feedback - airbyte-ci is deprecated and the entire tool.airbyte_ci section is stale Co-Authored-By: AJ Steers <aj@airbyte.io>
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/test-command.yml (1)
53-55
: Keep the matrix list multi-line for diff-friendly readability?In the companion workflow (
pytest_matrix.yml
) the samepython-version
list is split over separate lines, which makes future diffs and comments easier to read. Would you like to apply the same formatting here for consistency, e.g.:- python-version: ["3.10", "3.11", "3.12", "3.13"] + python-version: [ + "3.10", + "3.11", + "3.12", + "3.13", + ]wdyt?
pyproject.toml (1)
15-22
: Add classifiers for the newly supported Python versions?The
classifiers
list still advertises only Python 3.10, which might confuse downstream tooling (e.g. PyPI metadata) now that 3.11-3.13 are supported. Should we append the missing classifiers?"License :: OSI Approved :: MIT License", "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", ]wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/pytest_matrix.yml
(1 hunks).github/workflows/test-command.yml
(1 hunks)pyproject.toml
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ChristoGrab
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the `YamlDeclarativeSource` class in `airbyte_cdk/sources/declarative/yaml_declarative_source.py`, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, ignore all `__init__.py` files when providing a recommended reviewing order.
.github/workflows/pytest_matrix.yml (1)
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, ignore all __init__.py
files when providing a recommended reviewing order.
pyproject.toml (5)
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/
, including _run.py
, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, the declarative_component_schema.py
file is auto-generated from declarative_component_schema.yaml
and should be ignored in the recommended reviewing order.
Learnt from: aaronsteers
PR: #2
File: pyproject.toml:135-140
Timestamp: 2024-11-10T03:28:31.410Z
Learning: In pyproject.toml
, for poethepoet tasks, setting ignore_fail
to return_non_zero
allows all subtasks to run while still returning a non-zero exit code if any subtask fails.
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, ignore all __init__.py
files when providing a recommended reviewing order.
Learnt from: aaronsteers
PR: #2
File: pyproject.toml:113-117
Timestamp: 2024-11-10T03:28:33.676Z
Learning: In Poe the Poet tasks defined in pyproject.toml
, setting ignore_fail = "return_non_zero"
allows all tasks in a sequence to run, while still returning a non-zero exit code if any task fails.
⏰ 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). (9)
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-shopify
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: SDM Docker Image Build
🔇 Additional comments (5)
.github/workflows/test-command.yml (1)
116-122
: Is coverage upload meant to stay pinned to 3 .11 only?Now that 3.12/3.13 are first-class citizens, the step is still gated by
matrix.python-version == '3.11'
. If coverage from the newest runtime is valuable (or if 3.11 is no longer the canonical baseline) you might switch to one of the new versions or drop the filter:- if: always() && !cancelled() && matrix.python-version == '3.11' + # e.g. promote the newest stable: + if: always() && !cancelled() && matrix.python-version == '3.13'Just checking intent—happy either way, wdyt?
.github/workflows/pytest_matrix.yml (2)
30-35
: 👍 3.12 & 3.13 added — looks good.
No functional concerns with the expanded matrix.
97-100
: Same coverage-publishing question as in the on-demand workflow.The upload step is still limited to
'3.11'
. Do we want to move this to 3.13 (or remove the filter) so coverage reflects the latest supported Python, for consistency with the broader version bump? wdyt?pyproject.toml (2)
32-32
: Version range update LGTM.
The wider constraint aligns with the CI matrix.
31-35
: No lingering<3.13
version pins detectedI ran an
rg --no-heading -n '<3\.13|==3\.1[23]'
across the repo and found no matches. It looks like all<3.13
caps have been cleared—wdyt?
PyTest Results (Full)3 765 tests 3 753 ✅ 11m 19s ⏱️ Results for commit 6a44f70. ♻️ This comment has been updated with latest results. |
Co-Authored-By: maxime@airbyte.io <maxime@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
FYI, Ignore these unrelated errors:
|
…wer fastavro Co-Authored-By: AJ Steers <aj@airbyte.io>
…urrent test for Python 3.12+ Co-Authored-By: maxime@airbyte.io <maxime@airbyte.io>
…ompatibility Co-Authored-By: AJ Steers <aj@airbyte.io>
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/pytest_matrix.yml
(1 hunks)airbyte_cdk/manifest_migrations/README.md
(1 hunks)pyproject.toml
(3 hunks)unit_tests/sources/declarative/test_concurrent_declarative_source.py
(2 hunks)unit_tests/test_exception_handler.py
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- airbyte_cdk/manifest_migrations/README.md
- unit_tests/sources/declarative/test_concurrent_declarative_source.py
- .github/workflows/pytest_matrix.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ChristoGrab
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the `YamlDeclarativeSource` class in `airbyte_cdk/sources/declarative/yaml_declarative_source.py`, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, ignore all `__init__.py` files when providing a recommended reviewing order.
🪛 GitHub Actions: Linters
unit_tests/test_exception_handler.py
[error] 47-47: ruff formatting check failed. Extra whitespace detected that requires removal.
⏰ 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). (12)
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-intercom
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-shopify
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Analyze (python)
🔇 Additional comments (1)
unit_tests/test_exception_handler.py (1)
51-64
: Nice approach to handle Python version-specific traceback formatting!The conditional logic to handle the different traceback formats between Python 3.13+ and earlier versions looks solid. Python 3.13 introduced more detailed error messages with caret indicators, and you've captured that nicely. This ensures the test remains robust across the supported Python versions.
The implementation correctly identifies that Python 3.13+ includes the full command string and caret line in the traceback, while earlier versions use the simpler format. This aligns perfectly with the PR's objective of adding Python 3.12 and 3.13 support.
Co-Authored-By: maxime@airbyte.io <maxime@airbyte.io>
…nager.devin.ai/proxy/github.com/airbytehq/airbyte-python-cdk into devin/1753929340-python-3.13-support
Co-Authored-By: maxime@airbyte.io <maxime@airbyte.io>
… only Co-Authored-By: AJ Steers <aj@airbyte.io>
…heck - Replace version-specific if/else logic with string contains checks - Use base exception trace that works for both Python <3.13 and >=3.13 - Addresses PR feedback from @aaronsteers to use 'or' operator approach Co-Authored-By: AJ Steers <aj@airbyte.io>
/auto-fix |
- Split long AirbyteLogMessage constructor across multiple lines - Wrap long assert statements with parentheses for readability - Addresses Ruff Format Check CI failure Co-Authored-By: AJ Steers <aj@airbyte.io>
/autofix
|
unit_tests/sources/declarative/test_concurrent_declarative_source.py
Outdated
Show resolved
Hide resolved
…and revert variable name - Add detailed explanation of SQLite threading compatibility issue in Python 3.12+ - Revert variable name from 'base_exception_trace' to 'exception_trace' - Addresses PR comments from @aaronsteers Co-Authored-By: AJ Steers <aj@airbyte.io>
…ompatibility - Simplify expected traceback format to work across Python versions - Python 3.13 changed traceback format with exception message before traceback - Maintains compatibility with Python 3.10, 3.11, and 3.12 Co-Authored-By: AJ Steers <aj@airbyte.io>
…k format - Use flexible string containment checks for individual traceback components - Python 3.13 changed traceback format with more detailed caret information - Test now checks for 'Traceback (most recent call last):' and file reference separately - Maintains compatibility with Python 3.10, 3.11, and 3.12 Co-Authored-By: AJ Steers <aj@airbyte.io>
Devin, can you resolve conflict between this branch and main? |
…t and manifest-server features - Update [tool.airbyte_ci] python_versions to include 3.12 and 3.13 - Keep updated dependency versions for Python 3.12/3.13 compatibility - Merge import statements in test file - Preserve manifest-server functionality from main branch Co-Authored-By: AJ Steers <aj@airbyte.io>
… conflict resolution - Regenerate poetry.lock to match resolved pyproject.toml dependencies - Fix Prettier formatting issues in GitHub Actions and manifest server files - Resolves CI dependency installation failures Co-Authored-By: AJ Steers <aj@airbyte.io>
@dbgold17 - I think I'd like to merge this after reverting the unrelated formatting updates. On its own, this PR doesn't make the higher Python versions default, so it is expected to be safe. In a follow on iteration we can then start testing specific connectors (base images, really) to use 3.12 or 3.13 as default. Are you ok to approve, pending revert of the unrelated formatting stuff? I'll try to get this merged today if so. UPDATE: Formatting reverts complete. ✅ |
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 (4)
airbyte_cdk/manifest_server/openapi.yaml (4)
28-28
: Minor generated title nit: duplicate words and spacingThe title “Response Health Health Get” has a duplicate “Health” and double space. Would you like to tweak the generator/template to emit a cleaner title, e.g. “Health Response” or “Health – 200”? wdyt?
If you prefer a direct fix in the spec (even if generated), the minimal diff would be:
- title: Response Health Health Get + title: Health – 200 Response
383-400
: Required + nullable for HttpRequest.headers is confusing
headers
is marked required, but its schema allowsnull
. This forces clients to send the field yet permitsnull
, which often trips codegen/validation. Shall we either dropnull
from the schema or removeheaders
fromrequired
to make intent unambiguous, wdyt?Potential generator-side diff (conceptual):
headers: - anyOf: - - type: object - - type: "null" + type: object ... - required: - - url - - headers - - http_method + required: + - url + - http_method
357-361
: Use integer maxima/minima for integer fields to avoid client codegen warningsJSON Schema allows numbers, but some codegens lint
maximum: 100.0
fortype: integer
. Switching to integer literals reduces noise. OK to adjust the generator to emit ints here, wdyt?- maximum: 100.0 - minimum: 1.0 + maximum: 100 + minimum: 1 ... - maximum: 5000.0 - minimum: 1.0 + maximum: 5000 + minimum: 1 ... - maximum: 20.0 - minimum: 1.0 + maximum: 20 + minimum: 1Also applies to: 590-595, 596-601, 602-607
197-202
: Checkov CKV_OPENAPI_21: maxItems for arraysThe catalog’s
streams
array has nomaxItems
. For this API, a hard cap may be artificial. Do you want to suppress CKV_OPENAPI_21 for this auto-generated file (preferred), or add a very highmaxItems
to appease the linter, wdyt?Options:
- CI suppression (recommended): configure Checkov to skip CKV_OPENAPI_21 for
airbyte_cdk/manifest_server/openapi.yaml
with repo-level policy.- Spec change (if absolutely needed):
streams: items: $ref: "#/components/schemas/AirbyteStream" type: array + maxItems: 1000000 title: Streams
📜 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 ignored due to path filters (1)
poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (7)
.github/actions/check-docker-tag/action.yml
(2 hunks).github/workflows/pytest_matrix.yml
(1 hunks).github/workflows/test-command.yml
(1 hunks)airbyte_cdk/manifest_server/README.md
(6 hunks)airbyte_cdk/manifest_server/openapi.yaml
(14 hunks)pyproject.toml
(4 hunks)unit_tests/sources/declarative/test_concurrent_declarative_source.py
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/actions/check-docker-tag/action.yml
- airbyte_cdk/manifest_server/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/pytest_matrix.yml
- pyproject.toml
- unit_tests/sources/declarative/test_concurrent_declarative_source.py
- .github/workflows/test-command.yml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-18T23:40:06.391Z
Learnt from: ChristoGrab
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the `YamlDeclarativeSource` class in `airbyte_cdk/sources/declarative/yaml_declarative_source.py`, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Applied to files:
airbyte_cdk/manifest_server/openapi.yaml
🪛 Checkov (3.2.334)
airbyte_cdk/manifest_server/openapi.yaml
[MEDIUM] 197-202: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
⏰ 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). (8)
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-shopify
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (5)
airbyte_cdk/manifest_server/openapi.yaml (5)
16-16
: Tags indentation/format normalization looks goodConsistent tags arrays across operations; no semantic changes. LGTM, wdyt?
Also applies to: 32-32, 49-49, 77-77, 105-105, 133-133, 161-161
20-21
: Quoting HTTP status codes is correctUsing quoted strings for response codes improves YAML robustness and tooling compatibility. Looks good to keep, wdyt?
Also applies to: 40-41, 60-61, 88-89, 116-117, 144-145, 178-179, 66-67, 94-95, 122-123, 150-151, 184-185
45-46
: $ref and JSON Schema 2020-12 nullability are validNormalization to quoted $ref targets and anyOf with "null" is spec-compliant for OAS 3.1. No issues from me, wdyt?
Also applies to: 57-58, 65-66, 71-72, 85-86
73-74
: Security blocks look consistent
HTTPBearer: []
is applied uniformly across secured operations and matches the scheme incomponents.securitySchemes
. LGTM, wdyt?Also applies to: 101-102, 129-130, 157-158, 191-192
1-3
: Add CI validation for the generated OpenAPI spec across Python 3.10–3.13
Could we add a CI matrix job that installs openapi-spec-validator and yamllint (via pip) and runs them againstairbyte_cdk/manifest_server/openapi.yaml
to guard against spec drift across supported Python versions, wdyt?
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
♻️ Duplicate comments (1)
.github/workflows/pytest_matrix.yml (1)
31-35
: Add 3.12 and 3.13 to matrix — LGTMMatrix expansion aligns with the PR objectives and should surface Py3.12/3.13 issues early. Nice.
🧹 Nitpick comments (3)
.github/workflows/pytest_matrix.yml (3)
36-39
: Ensure runner label resolves to a valid imageUsing "Ubuntu" produces "Ubuntu-latest" at runs-on; GitHub runner labels are typically lowercase (e.g., "ubuntu-latest"). Could we standardize to lowercase to avoid any label mismatch edge cases, wdyt?
- os: [ - Ubuntu, - # Windows, # For now, we don't include Windows in the test matrix. - ] + os: [ + ubuntu, + # windows, # For now, we don't include Windows in the test matrix. + ]If you prefer to keep the capitalized display names in the matrix, an alternative is to map to a runner label:
- runs-on: "${{ matrix.os }}-latest" + runs-on: "${{ (matrix.os == 'Ubuntu' && 'ubuntu') || (matrix.os == 'Windows' && 'windows') || matrix.os }}-latest"Would you like to adopt one of these to be explicit, wdyt?
Also applies to: 42-42
97-105
: Publish test results for a newer Python or make it explicitRight now results publish only when matrix.python-version == '3.11'. Since we added 3.12/3.13, do we want to publish from 3.13 to reflect the newest runtime, or keep 3.11 intentionally? Making this explicit avoids confusion, wdyt?
Option A (publish from 3.13):
- if: always() && !cancelled() && steps.changes.outputs.src == 'true' && matrix.python-version == '3.11' + if: always() && !cancelled() && steps.changes.outputs.src == 'true' && matrix.python-version == '3.13'Option B (single publish from the first matrix job only):
- if: always() && !cancelled() && steps.changes.outputs.src == 'true' && matrix.python-version == '3.11' + if: always() && !cancelled() && steps.changes.outputs.src == 'true' && matrix.python-version == matrix.python-version[0](Option B requires setting an output or using a dedicated job; happy to sketch that if you’d like, wdyt?)
67-72
: Pin latest patch for each minor to reduce flakinessWould you consider adding check-latest: true so setup-python resolves the latest patch (e.g., 3.13.x) which tends to include critical fixes early in the cycle, wdyt?
- name: Set up Python uses: actions/setup-python@v5 if: steps.changes.outputs.src == 'true' with: python-version: ${{ matrix.python-version }} + check-latest: true cache: "poetry"
📜 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 (4)
.github/actions/check-docker-tag/action.yml
(1 hunks).github/workflows/pytest_matrix.yml
(1 hunks).github/workflows/test-command.yml
(1 hunks)pyproject.toml
(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/actions/check-docker-tag/action.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/test-command.yml
- pyproject.toml
⏰ 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). (12)
- GitHub Check: Check: source-intercom
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-shopify
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
feat: add Python 3.12 and 3.13 support
Summary
This PR adds Python 3.12 and 3.13 compatibility to the airbyte-python-cdk by updating version constraints and test matrices. The main changes include:
pyproject.toml
from>=3.10,<3.13
to>=3.10,<3.14
pytest_matrix.yml
,test-command.yml
)The historical blockers (requests-cache compatibility and distutils usage) have been resolved - requests-cache 1.2.1 supports Python 3.12+, and the codebase already has a custom
_strtobool
implementation replacingdistutils.util.strtobool
.Review & Testing Checklist for Human
poetry run poe pytest-fast
with these versions)Recommended test plan: After CI passes, manually test a few core CDK features (like running a simple connector) with Python 3.12 and 3.13 to catch any runtime issues that automated tests might miss.
Diagram
Notes
Link to Devin run: https://app.devin.ai/sessions/89494475ebe04a2fa325839dca0e4a83
Requested by: AJ Steers (@aaronsteers)
Summary by CodeRabbit
New Features
Chores
Tests
Documentation
Style