-
Notifications
You must be signed in to change notification settings - Fork 30
fix: Add circular reference detection to schema_helpers._expand_refs() #787
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
base: main
Are you sure you want to change the base?
fix: Add circular reference detection to schema_helpers._expand_refs() #787
Conversation
Fixes a ValueError: Circular reference detected error that occurs when serpyco-rs tries to serialize connector specs with circular patterns. The _expand_refs() function now tracks visited references during recursive expansion to prevent infinite loops when schemas contain circular references. This resolves issue airbytehq/oncall#9688 for the Facebook Marketing connector and any other connectors with similar circular reference patterns in their specs. Changes: - Add visited_refs parameter to _expand_refs() to track reference URLs - Skip expanding already-visited references to prevent infinite recursion - Remove references from visited set after expansion to allow reuse in different branches - Skip 'definitions' key during iteration to avoid re-expanding resolved definitions - Add unit tests for circular reference handling and simple reference expansion Co-Authored-By: unknown <>
Original prompt from API User |
🤖 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/1760390546-fix-circular-ref-in-expand-refs#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/1760390546-fix-circular-ref-in-expand-refsHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
Co-Authored-By: unknown <>
Response to @aaronsteers' QuestionsRequested by: @aaronsteers Question 1: Recursive Schema Use Case - Customer-Specific or General?The recursive/circular schema references are general to Facebook Marketing, not customer-specific. This is a fundamental characteristic of the connector's schema design. Show/Hide Technical DetailsSchema Structure Analysis The Facebook Marketing connector's spec contains several patterns that can create circular references during schema expansion:
credentials: Union[OAuthCredentials, ServiceAccountCredentials]
custom_insights: Optional[List[InsightConfig]]The
Why This Affects All Users These schema structures are core features of the Facebook Marketing connector that ALL users have access to:
Why It Triggered Now While the schema structure is general, the error was triggered by:
Affected User: Currently only one workspace ( Question 2: Engineer Suggestions for ReviewBased on git history analysis, I recommend consulting with: Primary Recommendation: Anatolii Yatsuk (@tolik0)
Alternative Recommendations:
Git History Evidence: Recent Spec.py ContributorsLink to Devin run: https://app.devin.ai/sessions/9cf5e4b0c4424066b985df1b8d44039e |
fix: Add circular reference detection to schema_helpers._expand_refs()
Summary
Fixes airbytehq/oncall#9688 - a
ValueError: Circular reference detectederror that occurs when serpyco-rs tries to serialize connector specs with circular$refpatterns.The
_expand_refs()function now tracks visited references during recursive expansion to prevent infinite loops when schemas contain circular references. When a circular reference is detected, it's removed from the schema without expansion.Changes:
visited_refsparameter to_expand_refs()to track reference URLs during recursiondefinitionskey during iteration to avoid re-expanding already-resolved definitionsReview & Testing Checklist for Human
Risk: 🟡 Medium - Core utility function modification that affects schema resolution across all connectors.
source-facebook-marketing specto verify it no longer produces circular reference errors (this was intermittent in production, so may need multiple runs){"a": {"$ref": "#/definitions/X"}, "b": {"$ref": "#/definitions/X"}})A -> B -> A)Notes
Requested by: @aaronsteers
Session: https://app.devin.ai/sessions/1961a84783934c6d8fe7b0217441f382