Repository navigation
fix(mcp): polish Connector Builder base PR with additional fixes and improvements - #1408
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This PyAirbyte VersionYou can test this version of PyAirbyte using the following: # Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1792369980-connector-builder-readiness-fixes' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1792369980-connector-builder-readiness-fixes'PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful ResourcesCommunity SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
There was a problem hiding this comment.
🟢 Approval recommended
The changes are focused, regression tests cover the principal fixes, and no blocking issues remain.
No open issues 🎉
What changed in this PR
Fixes Connector Builder readiness checks to respect Airbyte Cloud’s test-read limits.
Changes:
- Defaults readiness reads to 5,000 records and validates limits before API calls.
- Separates failed reads from capped reads in the checklist.
- Updates MCP limits and guidance, with regression tests.
| File | Description |
|---|---|
tests/unit_tests/test_cloud_connector_builder.py |
Tests limit validation, defaults, and failed-read reporting. |
airbyte/mcp/cloud.py |
Aligns argument bounds and clarifies readiness guidance. |
airbyte/cloud/connector_builder.py |
Adds shared limits, validation, and corrected checklist reporting. |
Review effort: Balanced
Code coverage overviewLanguages: Python Python / code-coverage/pytest-no-creds
View coverage of the 10 most impacted files.
Python / code-coverage/pytest
View coverage of the 10 most impacted files.
Python / code-coverage/pytest-unit
Line coverage data for the devin/1792350894-con... branch is not yet available. View coverage of the 10 most covered files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
09f8343 to
9541e67
Compare
|
9541e67 to
87c9d62
Compare
|
182c2e4 to
cc9de94
Compare
Aaron ("AJ") Steers (aaronsteers)
changed the title
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
Hosted preview retest: Goose Desktop with Claude Opus 4.7, fresh chat, zero nudges, against PyAirbyte Created a new connector and ran a real sync. Inventories taken before and after the run show the name didn't collide with any prior one and that a new definition was created (
The first readiness check correctly returned
Remaining friction and coverage limits
|
5caa5a8 to
8c31a70
Compare
Co-Authored-By: AJ Steers <[email protected]>
Co-Authored-By: AJ Steers <[email protected]>
d5fd51b to
2d3c4fe
Compare
Co-Authored-By: AJ Steers <[email protected]>
b8c793a
into
devin/1792350894-connector-builder-tools
Overview
👉 TL;DR: Connector Builder readiness checks now work against live Airbyte Cloud, and they report data, failures and warnings without a success verdict. Before this, every readiness call failed because the default read size was larger than Cloud allows. A full-sync-only call also returned
passed=truebefore its sync finished.Specifically, this adds
TEST_READ_MAX_RECORDS = 5_000,TEST_READ_MAX_PAGES = 20,TEST_READ_MAX_SLICES = 20, validates limits client-side, separates errored reads from capped reads, adds informationalwarnings(round record counts, a full last page, 0 records), and replacespassed/passed_streamswith a per-streamfailed.Stacked on #1407:
Changes
run_readiness_checks()now defaults torecord_limit=5_000(was10_000). Cloud'sPOST /v1/connector_builder_projects/read_streamreturns HTTP 422 forrecordLimit > 5000,pageLimit > 20orsliceLimit > 20. The response body is only{"errorId": ...}, so the agent had nothing to act on._check_test_read_limits()raisesAirbyteLibInputErrorwith the accepted ranges before any API call. Readiness fails once instead of once per stream.test_connector_builder_streamMCP args are bounded by the new constants (le=5000for records, was10000).http_errorsnow getread_to_end=Falseand are listed asnot read. They no longer show up ashit a cap: ... Check these with a full sync after publishing.warnings: list[str]onConnectorBuilderStreamReadResultandConnectorBuilderStreamReadiness, returned by both MCP tools. When a read reached end of stream (test_read_limit_reachedfalse) and has records, it warns if the total is a multiple of 10, or if the last non-empty page has the same record count as the first (so more pages may exist). A read that reached end of stream with 0 records gets a warning instead of failing. Warnings don't changefailedorread_to_end.ConnectorBuilderReadinessReport.passed,.passed_streamsandConnectorBuilderStreamReadiness.passedare removed. NewConnectorBuilderStreamReadiness.failed: true when the read raised an error, returned HTTP errors, or logged errors.failed_streams,pending_streamsandnot_attempted_streamsstay;run_full_syncstill starts only when none of them are non-empty.next_stepsstates facts and actions only: failed, pending, not-attempted and capped streams ("hit a test-read cap, so later pages weren't read; check them with a full sync"), plus "Report these streams'warningsto the user: ...". "All named streams passed" and "Not ready yet" are gone. A stream that both hit a cap and logged an error is listed as failed and capped.run_connector_builder_readiness_checkssays the report calls out failures and warnings without judging readiness, and to report every failure and warning to the user.test_connector_builder_streamexplainstest_read_limit_reached, page-boundary overshoot, andwarnings.Review Spotlight
Reviewers with limited time, please review first:
connector_builder/project.py_check_test_read_limits: the limit guard and guidance textconnector_builder/_util.pyreadiness_checks/readiness_next_steps: capped vs. not-read split,failed, and the warnings stepconnector_builder/models.py_record_count_warnings: the warning heuristicsEvidence
Goose Desktop 1.36.0,
claude-opus-4-7, Devin sandbox Cloud workspace, fresh chat per run. Recordings are in the session thread.0ca475e709f83431passed=false(photosat the 5,000 cap)111673054SUCCEEDED09f83431*repositories,releasesOK;pull_requestscapped at 2,000;issuesGitHub 422 at 1,000;stargazers401issuesto cursor pagination, but the saved manifest still usedPageIncrement. Not published87c9d625cloud-mcp-preview, bearer +X-MCP-Insiders: 1passed=false(photoscap), "Not ready yet" step, 2 warnings on each uncapped stream111677627SUCCEEDED with 5,910 records. 42 calls, 5 errors during recovery, 4m07sJSONPlaceholder 1408 20261019T164712Z) atcc9de942cloud-mcp-previewpassed=false(photoscap), 2 warnings on each uncapped stream111796318SUCCEEDED with 5,910 records. 31 calls, 2 errors, 3m05s* The GitHub run's long-lived server likely lazily imported
connector_builder.pymid-edit from the shared checkout, so it isn't a frozen-SHA result. It also surfaced theread_to_end=trueon HTTP-error streams bug fixed here.Hosted readiness with the new step and warnings:
Limit probe against live Cloud
read_stream:Risks
passed/passed_streamsare removed (see Changes); nothing released depends on them.record_limit> 5,000 (or page/slice > 20): they now getAirbyteLibInputErrorinstead of Cloud's HTTP 422. No call that previously succeeded is rejected.Follow-ups
cc9de942, before the no-pass-verdict change; not yet rerun on preview.Test plan
uv run pytest tests/unit_tests/test_cloud_connector_builder.py tests/unit_tests/test_mcp_cloud.py tests/unit_tests/test_mcp_arg_trace_parity.py tests/unit_tests/test_mcp_trusted_execution.py tests/unit_tests/test_mcp_docs_includes.py -q: 336 passed at2d3c4fe0ruff check,ruff format --check,pyrefly checkon touched filesLink to Devin session: https://app.devin.ai/sessions/37c00a83f788401489bfd0e37c0b7440
Open in Devin Desktop: https://app.devin.ai/desktop/session/37c00a83f788401489bfd0e37c0b7440?variant=devin
Requested by: Aaron ("AJ") Steers (@aaronsteers)