chore: add CI check for openapi spec generation#751
chore: add CI check for openapi spec generation#751Pedro S. Lopez (pedroslopez) wants to merge 3 commits intomainfrom
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/check-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/check-openapiHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…in permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds a CI check to automatically validate that the OpenAPI specification is up-to-date whenever API models are modified. The goal is to prevent developers from forgetting to regenerate the OpenAPI spec after making changes to request/response types.
- Adds a new GitHub Actions job that generates the OpenAPI spec and fails if it differs from the committed version
- Updates the README with documentation about the automated validation process
- Includes updates to the OpenAPI spec itself (likely from running the generation command)
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/python_lint.yml |
Adds new openapi-check job to validate OpenAPI spec is current |
airbyte_cdk/manifest_server/README.md |
Documents the automated OpenAPI validation process and manual regeneration command |
airbyte_cdk/manifest_server/openapi.yaml |
Updates OpenAPI spec with new RequestContext schema and context fields |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
📝 WalkthroughWalkthroughAdds a CI job to verify OpenAPI spec generation in python_lint workflow. Updates README with instructions about automated OpenAPI validation. Extends OpenAPI spec by introducing RequestContext and adding optional context fields to multiple request schemas, and broadens slice_descriptor type to allow objects. Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Developer
participant GH as GitHub Actions
participant Job as openapi-check
Dev->>GH: Open PR / push
GH->>Job: Run workflow
Job->>Job: Checkout repo
Job->>Job: Setup Poetry 2.0.1 & Python 3.10
Job->>Job: Install deps (poetry install)
Job->>Job: Generate OpenAPI (poetry run manifest-server generate-openapi)
Job->>Job: git diff for changes
alt Spec changed
Job-->>GH: Mark job failed (spec outdated)
else No changes
Job-->>GH: Job succeeds
end
sequenceDiagram
participant Client
participant API as Manifest Server API
participant Handler as Request Handler
Client->>API: Request (optional context)
API->>Handler: Parse payload incl. context
Handler-->>API: Proceed with operation using context (if provided)
API-->>Client: Response
note over API,Handler: Context fields are optional (workspace_id, project_id)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
✨ 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
♻️ Duplicate comments (3)
.github/workflows/python_lint.yml (1)
2-3: Good security posture: minimal GITHUB_TOKEN perms now declared.This addresses the prior security bot suggestion to limit default permissions. 👍
airbyte_cdk/manifest_server/openapi.yaml (2)
337-341: Repeat of thecontextaddition—same doc suggestion applies.Mirroring the note above: add a brief description in the source model so clients see why/when to use
context, wdyt?
491-495: Samecontextfield here—carry over the docstring suggestion.Adding a short description in the model will surface better help in generated clients, wdyt?
🧹 Nitpick comments (5)
airbyte_cdk/manifest_server/README.md (1)
128-139: Nice addition: call out the CI guardrail; consider linking the job name and mentioning the existing poe task.Could we add the actual job name (“OpenAPI Spec Check”) and a brief note that there’s also a poe task to regenerate the spec, so contributors can choose either command, wdyt?
.github/workflows/python_lint.yml (1)
82-124: Make the diff detection robust to untracked files and scope it to openapi.yaml.If the spec file ever becomes untracked,
git diff --quietwon’t catch it. Would you switch togit status --porcelainscoped to the spec, and set the output accordingly, wdyt?- name: Check for changes id: git-diff - run: | - git diff --quiet && echo "No changes to commit" || echo "changes=true" >> $GITHUB_OUTPUT - shell: bash + shell: bash + run: | + set -eo pipefail + TARGET="airbyte_cdk/manifest_server/openapi.yaml" + git update-index -q --refresh + if git status --porcelain -- "$TARGET" | grep -q .; then + printf "changes=true\n" >> "$GITHUB_OUTPUT" + else + echo "No changes to OpenAPI spec" + fiOptional follow-ups:
- Would you prefer installing only what’s needed for generation to speed up CI (e.g.,
poetry install --extras manifest-server) instead of--all-extras, wdyt?- Poetry versions differ across jobs (this one uses 2.0.1 while others use 1.8.4). Standardizing across the workflow can reduce surprises; want to align them in a follow-up?
airbyte_cdk/manifest_server/openapi.yaml (3)
299-303: Additive context field looks good; consider documenting its intent.To improve generated docs/SDKs, would you add a short description to the
contextproperty in the source model (so it flows into the spec), e.g., “Optional request context for tracing/observability,” wdyt?
472-486: Tighten typing for IDs and enrich field docs.If
workspace_id/project_idare UUIDs, would you addformat: uuid(via PydanticField(..., json_schema_extra={"format": "uuid"})) and brief descriptions so SDKs validate/help users, wdyt? Example in the source model:from pydantic import BaseModel, Field class RequestContext(BaseModel): workspace_id: str | None = Field( default=None, description="Workspace identifier associated with the request.", json_schema_extra={"format": "uuid"}, ) project_id: str | None = Field( default=None, description="Project identifier associated with the request.", json_schema_extra={"format": "uuid"}, )
575-579: Broadenedslice_descriptor= object|string|null—can we define the object shape?For stronger client generation, would you consider modeling the object form (e.g., with a dedicated schema or
oneOfvariants) instead of baretype: object, or at least documenting expected keys in the source model so it flows into the spec, 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 (3)
.github/workflows/python_lint.yml(2 hunks)airbyte_cdk/manifest_server/README.md(1 hunks)airbyte_cdk/manifest_server/openapi.yaml(6 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). (12)
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-shopify
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-intercom
- 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)
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: SDM Docker Image Build
🔇 Additional comments (2)
airbyte_cdk/manifest_server/openapi.yaml (2)
368-372: Context on full resolve—ensure server plumbs it through.Since this feeds dynamic generation, can we confirm the backend handlers accept and propagate
contextwhere needed (logging/metrics/tracing), wdyt?
636-640:contexton test_read—ensure it’s utilized or ignored predictably.Can we confirm test-read handlers either use this for tracing or explicitly ignore it without error, and maybe document that behavior, wdyt?
Pedro S. Lopez (pedroslopez)
requested a review
from Aaron ("AJ") Steers (aaronsteers)
I forget to update the openapi spec since it's a manual thing.
This adds a CI check to make sure we don't forget. There's already a poe task to regenerate so you should be able to run the command if you forget - i prefer that over autocommitting fixes
Summary by CodeRabbit
New Features
Documentation
Chores