feat: update jsonschema constraint to >=4.20,<5.0 for fastmcp compatibility (minimal) #740
Conversation
…bility - Update jsonschema version constraint from ~4.17.3 to >=4.20,<5.0 - Regenerate poetry.lock with new constraint - No code changes - testing if version bump alone resolves CI issues 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/1756486554-jsonschema-minimal-bump#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/1756486554-jsonschema-minimal-bumpHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
There was a problem hiding this comment.
Pull Request Overview
This PR updates the jsonschema dependency constraint from ~4.17.3 to >=4.20,<5.0 to enable fastmcp compatibility. This is a minimal test PR that isolates the version constraint change from the referencing library migration code to determine which change was causing CI failures.
Key changes:
- Updated jsonschema version constraint to support fastmcp requirements (>=4.20.0)
- Regenerated poetry.lock with new transitive dependencies
- Removed the comment about 4.18+ breaking changes since they were related to RefResolver deprecation
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| google-cloud-secret-manager = "^2.17.0" | ||
| isodate = "~0.6.1" | ||
| Jinja2 = "~3.1.2" | ||
| jsonref = "~0.2" |
There was a problem hiding this comment.
[nitpick] Consider adding a comment explaining why the minimum version is 4.20.0 (fastmcp compatibility requirement) to help future maintainers understand the constraint rationale.
Copilot uses AI. Check for mistakes.
|
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. 📝 WalkthroughWalkthroughUpdated the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Would you like to apply these reviewers and the 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (13)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
pyproject.toml (3)
46-46: If you rely on "format" validation, enable non-GPL format extras or wire a FormatChecker explicitly, wdyt?By default, "format" is annotation-only. If tests expect strict format validation (e.g., date-time, email, uri), consider enabling extras to avoid silent behavior shifts.
-jsonschema = ">=4.20,<5.0" +jsonschema = { version = ">=4.20,<5.0", extras = ["format-nongpl"] }Or keep dependency as-is and ensure call sites pass
format_checker=jsonschema.draft202012_format_checker.
46-46: Changelog/Upgrade note: can we add a short entry calling out the dependency bump and potential deprecations, wdyt?Helps connector authors anticipate jsonschema/ref behavior changes.
I can draft a CHANGELOG snippet and an upgrade note section if helpful.
46-46: Align or deprecate jsonref in favor of jsonschema’s built-in $ref handling?
I seeimport jsonrefin bothairbyte_cdk/sources/utils/schema_helpers.pyand your tests, so jsonref isn’t just a test-only dependency. Mixing jsonref (~0.2) with jsonschema>=4.20 risks diverging $ref semantics or double-resolution. Should we retire jsonref (dropping it from pyproject.toml) and lean on jsonschema’sRefResolverexclusively, or else guard its usage and migrate gradually, 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 ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
pyproject.toml(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). (13)
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-intercom
- GitHub Check: Check: source-shopify
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
pyproject.toml (2)
46-46: Audit JSONSchema bump for legacy APIs
Bumping jsonschema to “>=4.20,<5.0” may break usages of deprecated/changed APIs. We detected hotspots in:
- airbyte_cdk/utils/spec_schema_transformations.py (RefResolver.from_schema)
- airbyte_cdk/sources/utils/schema_helpers.py (_expand_refs + validate)
- airbyte_cdk/sources/utils/transform.py (Draft7Validator, RefResolver, validators.create)
- airbyte_cdk/sources/declarative/validators/validate_adheres_to_schema.py (jsonschema.validate)
Could you verify these call sites remain compatible with jsonschema 4.20’s updated ref/format semantics? wdyt?
46-46: Confirm CI wheel availability
Could you verify in our [tool.airbyte_ci] matrix for Python 3.10–3.13 that no steps emit “Building wheel for rpds-py”, “Building wheel for referencing” or “Building wheel for jsonschema-specifications”? Please check the CI install logs and surface any source-build occurrences—wdyt?
PyTest Results (Fast)3 461 tests - 302 3 448 ✅ - 303 6m 31s ⏱️ -8s For more details on these failures, see this check. Results for commit af6e7c8. ± Comparison against base commit e4b34b6. This pull request removes 302 tests.♻️ This comment has been updated with latest results. |
- Update constraint from >=4.20,<5.0 to >=4.20,<4.21 for testing - Regenerate poetry.lock with narrower constraint - Testing if specific jsonschema 4.20.x version resolves CI failures Co-Authored-By: AJ Steers <aj@airbyte.io>
PyTest Results (Full)3 766 tests ±0 3 749 ✅ - 5 11m 12s ⏱️ -15s For more details on these failures, see this check. Results for commit af6e7c8. ± Comparison against base commit e4b34b6. |
|
Resolution tests break on 4.20, even with no code changes. What is odd is that there's no complaint about missing classes or deprecation warnings - the resolution just doesn't work. Could be a factor with the test itself (expecting niche or incorrect behavior) but I'm going to for now assume it's just not viable. Continuing to pursue this in: Closing this PR for now. Error was: https://github.com/airbytehq/airbyte-python-cdk/actions/runs/17329936082/job/49203351546 cc @lmossman |
feat: update jsonschema constraint to >=4.20,<5.0 for fastmcp compatibility (minimal)
Summary
This PR updates the jsonschema version constraint from
~4.17.3to>=4.20,<5.0to enable fastmcp compatibility, which requires jsonschema 4.20.0+. This is a minimal test PR that only changes the version constraint without any referencing library migration code.Key changes:
pyproject.tomljsonschema constraint to>=4.20,<5.0poetry.lockwith the new constraint (jsonschema 4.17.3 → 4.25.1)jsonschema-specifications,referencing,rpds-pypyrsistentThis PR serves as a test to determine if the version bump alone resolves CI issues, or if the problems were related to the referencing library migration code from PR #737.
Review & Testing Checklist for Human
unit_tests.sources.utils.test_transform#0which was previously failingpyrsistentor addition of new dependencies likerpds-pyTest Plan Recommendation
Notes
Link to Devin run: https://app.devin.ai/sessions/5a4dd7c708c64eb2bbc2d42fb688c9a7
Requested by: @aaronsteers
Summary by CodeRabbit