chore: update generated openapi spec #730
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/gen-openapi#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/gen-openapiHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
📝 WalkthroughWalkthroughRenames the public schema StreamRead to StreamReadResponse, adds a new StreamReadPages schema, updates references in StreamReadSlices.pages and the /v1/manifest/test_read 200 response to use these, removes the default for StreamTestReadRequest.state, and expands the Full Resolve endpoint description. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant S as Manifest Server
Note over C,S: Test Read flow (updated response schema)
C->>S: POST /v1/manifest/test_read (StreamTestReadRequest)
S-->>C: 200 OK (StreamReadResponse)
Note over S: StreamReadResponse.slices[*].pages[] now use StreamReadPages
rect rgba(220,240,255,0.5)
Note right of C: StreamReadPages contains<br/>- records[] (required)<br/>- request (HttpRequest|null)<br/>- response (HttpResponse|null)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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: 0
🧹 Nitpick comments (4)
airbyte_cdk/manifest_server/openapi.yaml (4)
64-64: Schema rename to StreamReadResponse: verify SDK/codegen impact; consider a deprecated alias?Renaming the component key/title can be source-breaking for generated clients that relied on
StreamRead. Would you consider keeping a deprecated alias to smooth the transition, wdyt?Example alias (generator-side or post-process) to preserve
StreamReadwhile steering users to the new name:components: schemas: + StreamRead: + allOf: + - $ref: '#/components/schemas/StreamReadResponse' + deprecated: true + title: StreamRead (deprecated)
162-168: Nit: use a folded block for the long description for readabilityThe single-quoted multi-line string is valid YAML, but a folded block reads nicer in diffs and tooling. Want to switch, wdyt?
- description: 'Fully resolve a manifest, including dynamic streams. - - - This is a similar operation to resolve, but has an extra step which generates - streams from dynamic stream templates if the manifest contains any. This is - used when a user clicks the generate streams button on a stream template in - the Builder UI' + description: >- + Fully resolve a manifest, including dynamic streams. + + This is a similar operation to resolve, but has an extra step which + generates streams from dynamic stream templates if the manifest contains + any. This is used when a user clicks the generate streams button on a + stream template in the Builder UI.
469-488: records items are untyped (items: {}); tighten to object for better client generation?Many generators emit
Anyfor{}which weakens typing. Should we declare records as objects to help consumers, wdyt?StreamReadPages: properties: records: - items: {} + items: + type: object type: array title: Records
578-581: Removed default for state: confirm server behavior; keep default in spec to avoid client churn?If the server still treats missing
stateas[], documenting the default helps client SDKs and preserves prior behavior. Do we want to keepdefault: []here, wdyt?StreamTestReadRequest: properties: ... state: items: {} type: array title: State + default: []
📜 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)
airbyte_cdk/manifest_server/openapi.yaml(4 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). (10)
- GitHub Check: Check: source-shopify
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-intercom
- GitHub Check: SDM Docker Image Build
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Build and Inspect Python Package
🔇 Additional comments (2)
airbyte_cdk/manifest_server/openapi.yaml (2)
534-534: LGTM: title matches component nameTitle alignment with
StreamReadResponselooks consistent. Nice.
538-541: All clear—no staleStreamReadschema refs detectedI ran the ripgrep checks you suggested and confirmed:
- No lingering
#/components/schemas/StreamReadreferences anywhere in the repo- The new
StreamReadPagesschema is defined exactly once inopenapi.yamland is referenced only where expected- All existing code and tests that reference pages now correctly use
StreamReadPagesLooks safe to me—resolving this one!
I missed generating the openapi spec after the final changes. This regenerates the file using
manifest-server generate-openapiNote: this makes me think we should have some diff checks in CI to ensure the file was generated after changing the manifest server api, but will make an issue / address separately.
Summary by CodeRabbit
Chores
Documentation
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.