Repository navigation
fix(cloud): treat an unavailable feature probe as unknown, not disabled - #1258
Aaron ("AJ") Steers (aaronsteers) wants to merge 3 commits into
Conversation
Co-authored-by: Piotr Kozikowski <piotr.kozikowski@gmail.com> Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
|
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/1790381530-unavailable-probe-unknown' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1790381530-unavailable-probe-unknown'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. |
Code Coverage OverviewLanguages: Python Python / code-coverage/pytest-no-credsThe overall line coverage in commit 47fa2fc in the Show a line coverage summary of the most impacted files.
Python / code-coverage/pytestThe overall line coverage in commit 47fa2fc in the Show a line coverage summary of the most impacted files.
Python / code-coverage/pytest-unitThe overall line coverage in commit 47fa2fc in the Show a line coverage summary of the most covered files.
Updated |
…is unavailable Co-authored-by: Piotr Kozikowski <piotr.kozikowski@gmail.com> Co-Authored-By: AJ Steers <aj@airbyte.io>
Johnny Schmidt (johnny-schmidt)
left a comment
Johnny Schmidt (johnny-schmidt)
left a comment
There was a problem hiding this comment.
See #1262, which repoints the MCP at the new cloud enablement services. That moots this work. Please rebase on top of it
|
Agreed — #1262 makes the enablement API the source of truth and already reports |
|
Closing as superseded by #1262 (confirmed with Aaron ("AJ") Steers (@aaronsteers)). |
2 participants
Overview
👉 TL;DR: When Airbyte Cloud cannot tell us whether a connector is enabled for agent access (the docs lookup is denied or missing), we now report that as "unknown" instead of quietly saying the connector has no features. The connector list and connector describe tools now agree with each other on this.
Specifically,
CloudWorkspace._get_connector_featuresreturnsNoneon a 403/404 docs probe,CloudConnectorgains a publicget_enabled_features(*, warnings=...) -> frozenset[ConnectorFeature] | None, and bothlist_cloud_connectorsanddescribe_cloud_connectoremitenabled_features: "unknown"for that case.Supersedes part of #1240 (re-cut as a focused PR; co-authored with Piotr Kozikowski (@pkozikow)). Pairs with #1257 (the other half of that split).
Related, already merged: #1249 covered the 5xx/transport probe failure case with the same
"unknown"sentinel; this PR extends it to the 403/404 "docs unavailable" case.Changes
CloudWorkspace._get_connector_features(connector, *, warnings=None) -> frozenset | None: returnsNonewhen_context_layer_inspectyieldsNone(403/404), forwards thewarningssink; stillfrozenset()for no-Context-layer-API and non-passthrough destinations (known empty).CloudConnector.get_enabled_features(*, warnings=None)replaces private_get_enabled_features; caches only known results (including a known-emptyfrozenset()), never cachesNone, so a later probe can recover.enabled_featuresproperty keeps the conservativeNone -> frozenset()behavior.CloudWorkspace.list_connectors(feature_filter=...)(Python API) now goes throughconnector.enabled_features; unknown is still excluded there (conservative).list_cloud_connectorswithfeature_filter: a 403/404 probe now yields a result withenabled_features="unknown"plus the probe warning, instead of silently dropping the connector.describe_cloud_connector: usesget_enabled_features(warnings=warnings);None -> FEATURES_UNKNOWN. Successful-probe warnings still surface via_context_layer_details.warnings.Nonesemantics, no-cache-on-unknown/recovery,enabled_featuresfallback, and list/describe consistency.Review Spotlight
Reviewers with limited time, please review first:
connectors.py#L125-L147: the newget_enabled_featurescaching rulemcp/cloud.py#L1208-L1254: list tool now includes unknown-probe connectorsmcp/cloud.py#L1357-L1360: describe tool unknown mappingNot carried over from #1240
422 "Write actions (create/update/delete) are not supported."detail is now produced by Sonar (https://github.com/airbytehq/sonar/pull/6685) and forwarded by the platform (https://github.com/airbytehq/airbyte-platform-internal/pull/19629), so no PyAirbyte special-casing is needed.AirbyteExternalAccessNotEnabledErrorrelabel in_execute_direct_action: live testing showed that relabel is accurate for not-enabled connectors, so it stays.Risks
list_cloud_connectors(feature_filter=...): connectors whose docs probe 403/404s now appear (as"unknown") rather than being excluded. Agents should treat"unknown"as "verify", not as enabled._context_layer_inspectwarnings keep the raw{error}text (unchanged fromrelease-candidate/v1); this PR does not sanitize them.Test plan
uv run pytest tests/unit_tests/test_cloud_describe.py tests/unit_tests/test_mcp_cloud.py tests/unit_tests/test_cloud_direct_execute.py -q-> 173 passeduv run ruff check airbyte tests/unit_tests/uv run ruff format --check/uv run pyrefly check-> cleanLink to Devin session: https://app.devin.ai/sessions/3c39d8b470bb44f8929a9e9d6ab5c178
Open in Devin Desktop: https://app.devin.ai/desktop/session/3c39d8b470bb44f8929a9e9d6ab5c178?variant=devin
Requested by: Aaron ("AJ") Steers (@aaronsteers)