feat(cloud): allow config API root override#1023
feat(cloud): allow config API root override#1023Aaron ("AJ") Steers (aaronsteers) merged 6 commits into
Conversation
🤖 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. 💡 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/1779122008-config-api-root-override' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1779122008-config-api-root-override'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. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds optional config_api_root routing across the SDK: inference and env resolution in api_util, threading a config_api_root through config-API helpers, exposing it in CloudClientConfig/CloudWorkspace/CloudOrganization, integrating into cloud operations and MCP wiring, and adding unit tests for resolution logic. ChangesConfig API root support across Airbyte
🎯 3 (Moderate) | ⏱️ ~25 minutes Would you like me to highlight any specific changed functions or potential follow-up tests to review next, wdyt? 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
airbyte/_util/api_util.py (1)
1991-2008:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCould we align this docstring with the actual function signature?
At Line 2007, the docstring documents
config_api_root, butlist_organizations_for_user()does not accept that parameter. Could we remove that arg entry (or add the parameter if intended) so generated docs don’t mislead callers, wdyt?Suggested doc-only fix
def list_organizations_for_user( @@ Args: api_root: The API root URL client_id: OAuth client ID client_secret: OAuth client secret bearer_token: Bearer token for authentication (alternative to client credentials). - config_api_root: Optional explicit Config API root URL.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@airbyte/_util/api_util.py` around lines 1991 - 2008, The docstring for list_organizations_for_user is out of sync with its signature: remove the stray "config_api_root" entry from the Args section (or if the parameter was intended, add a matching parameter to the function signature) so the documented parameters match the actual function signature; update only the docstring for list_organizations_for_user to drop the config_api_root bullet (or add the parameter and its typing/defaults to the function) and ensure the Args list matches the parameters client_id, client_secret, bearer_token, and api_root.airbyte/mcp/cloud.py (1)
1345-1350:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCould we forward
config_api_rootin organization lookup to preserve override behavior?At Line 1345,
_resolve_organization()callsapi_util.list_organizations_for_user(...)withoutconfig_api_root, so explicit MCP override can be ignored during org resolution. On custom deployments, this can fail before later calls that do pass the override. Could we thread it through here as well, wdyt?Proposed fix
orgs = api_util.list_organizations_for_user( api_root=api_root, client_id=client_id, client_secret=client_secret, bearer_token=bearer_token, + config_api_root=config_api_root, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@airbyte/mcp/cloud.py` around lines 1345 - 1350, The org lookup in _resolve_organization omits the config_api_root override when calling api_util.list_organizations_for_user; modify the call in _resolve_organization to pass the config_api_root argument (the same override used elsewhere) so list_organizations_for_user receives the override and respects custom MCP deployments. Update the call site to include config_api_root=config_api_root while keeping existing client_id/client_secret/bearer_token parameters and adjust any callers of _resolve_organization if its signature must be threaded to provide config_api_root.
🧹 Nitpick comments (1)
tests/unit_tests/test_cloud_api_roots.py (1)
66-75: ⚡ Quick winCould we add one assertion that env override wins over an explicit
config_api_rootargument, wdyt?This would lock down precedence and prevent regressions in the highest-priority override path.
Possible addition
def test_get_config_api_root_env_override(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv( api_util.CLOUD_CONFIG_API_ROOT_ENV_VAR, "https://example.airbyte.com/env/config/", ) assert ( api_util.get_config_api_root("https://example.airbyte.com/custom/public") == "https://example.airbyte.com/env/config" ) + assert ( + api_util.get_config_api_root( + "https://example.airbyte.com/custom/public", + config_api_root="https://example.airbyte.com/explicit/config", + ) + == "https://example.airbyte.com/env/config" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit_tests/test_cloud_api_roots.py` around lines 66 - 75, Add an assertion to the test_get_config_api_root_env_override test to verify that the CLOUD_CONFIG_API_ROOT_ENV_VAR environment override takes precedence over an explicit config_api_root argument passed to api_util.get_config_api_root; specifically, call api_util.get_config_api_root with a non-matching "https://example.airbyte.com/custom/public" (or similar) and assert the return equals the env value ("https://example.airbyte.com/env/config"), ensuring the env override wins over the config_api_root parameter.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@airbyte/_util/api_util.py`:
- Around line 1991-2008: The docstring for list_organizations_for_user is out of
sync with its signature: remove the stray "config_api_root" entry from the Args
section (or if the parameter was intended, add a matching parameter to the
function signature) so the documented parameters match the actual function
signature; update only the docstring for list_organizations_for_user to drop the
config_api_root bullet (or add the parameter and its typing/defaults to the
function) and ensure the Args list matches the parameters client_id,
client_secret, bearer_token, and api_root.
In `@airbyte/mcp/cloud.py`:
- Around line 1345-1350: The org lookup in _resolve_organization omits the
config_api_root override when calling api_util.list_organizations_for_user;
modify the call in _resolve_organization to pass the config_api_root argument
(the same override used elsewhere) so list_organizations_for_user receives the
override and respects custom MCP deployments. Update the call site to include
config_api_root=config_api_root while keeping existing
client_id/client_secret/bearer_token parameters and adjust any callers of
_resolve_organization if its signature must be threaded to provide
config_api_root.
---
Nitpick comments:
In `@tests/unit_tests/test_cloud_api_roots.py`:
- Around line 66-75: Add an assertion to the
test_get_config_api_root_env_override test to verify that the
CLOUD_CONFIG_API_ROOT_ENV_VAR environment override takes precedence over an
explicit config_api_root argument passed to api_util.get_config_api_root;
specifically, call api_util.get_config_api_root with a non-matching
"https://example.airbyte.com/custom/public" (or similar) and assert the return
equals the env value ("https://example.airbyte.com/env/config"), ensuring the
env override wins over the config_api_root parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 26bfc8f4-1d8b-42ba-ac42-8bc095cdc4ff
📒 Files selected for processing (11)
airbyte/_util/api_util.pyairbyte/cloud/client_config.pyairbyte/cloud/connections.pyairbyte/cloud/connectors.pyairbyte/cloud/sync_results.pyairbyte/cloud/workspaces.pyairbyte/constants.pyairbyte/mcp/_tool_utils.pyairbyte/mcp/cloud.pyairbyte/mcp/server.pytests/unit_tests/test_cloud_api_roots.py
There was a problem hiding this comment.
Pull request overview
This PR adds support for overriding and inferring the Airbyte Config API root when using Cloud APIs, especially for self-managed deployments whose public API root differs from the Config API root.
Changes:
- Adds
config_api_rootplumbing through Cloud workspace/config objects and Config API helper calls. - Adds MCP config/header support for
config_api_url. - Adds unit tests for Config API root resolution and self-managed inference.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
airbyte/_util/api_util.py |
Adds Config API root inference/override support and threads it into Config API helpers. |
airbyte/cloud/client_config.py |
Adds config_api_root to cloud client configuration and env resolution. |
airbyte/cloud/workspaces.py |
Adds config_api_root to workspace/organization objects and downstream calls. |
airbyte/cloud/connections.py |
Passes workspace Config API root to catalog/state Config API operations. |
airbyte/cloud/connectors.py |
Passes workspace Config API root to connector check and builder APIs. |
airbyte/cloud/sync_results.py |
Passes workspace Config API root to job Config API requests. |
airbyte/constants.py |
Adds MCP config/header constants for Config API URL. |
airbyte/mcp/_tool_utils.py |
Adds MCP server config argument for Config API URL. |
airbyte/mcp/cloud.py |
Reads and propagates MCP Config API URL settings. |
tests/unit_tests/test_cloud_api_roots.py |
Adds unit coverage for Config API root resolution behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
airbyte/_util/api_util.py (1)
143-144: 💤 Low valueMinor inconsistency in trailing slash normalization?
The other return paths (lines 134, 139) strip trailing slashes, and
_infer_config_api_rootalso strips them internally. However, line 144 returnsCLOUD_CONFIG_API_ROOTdirectly without.rstrip("/").Given that URL construction at line 1491 does
config_api_root + path(where paths start with/), a trailing slash on the constant would produce double slashes likehttps://cloud.airbyte.com/api/v1//state/get. Most servers handle this fine, but for consistency, would you consider normalizing this return path as well, wdyt?♻️ Suggested fix
if api_root.rstrip("/") == CLOUD_API_ROOT.rstrip("/"): - return CLOUD_CONFIG_API_ROOT + return CLOUD_CONFIG_API_ROOT.rstrip("/")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@airbyte/_util/api_util.py` around lines 143 - 144, The branch that returns CLOUD_CONFIG_API_ROOT when api_root matches CLOUD_API_ROOT should normalize trailing slashes to match other paths: update the return in _infer_config_api_root so it returns CLOUD_CONFIG_API_ROOT.rstrip("/") (or otherwise apply .rstrip("/") consistently) instead of the raw constant, ensuring subsequent URL construction (config_api_root + path) won't produce a double slash; reference symbols: api_root, CLOUD_API_ROOT, CLOUD_CONFIG_API_ROOT, and function _infer_config_api_root.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@airbyte/_util/api_util.py`:
- Around line 143-144: The branch that returns CLOUD_CONFIG_API_ROOT when
api_root matches CLOUD_API_ROOT should normalize trailing slashes to match other
paths: update the return in _infer_config_api_root so it returns
CLOUD_CONFIG_API_ROOT.rstrip("/") (or otherwise apply .rstrip("/") consistently)
instead of the raw constant, ensuring subsequent URL construction
(config_api_root + path) won't produce a double slash; reference symbols:
api_root, CLOUD_API_ROOT, CLOUD_CONFIG_API_ROOT, and function
_infer_config_api_root.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b2e6fc89-9721-4d2b-9b51-456ba712e0ad
📒 Files selected for processing (2)
airbyte/_util/api_util.pyairbyte/cloud/workspaces.py
🚧 Files skipped from review as they are similar to previous changes (1)
- airbyte/cloud/workspaces.py
|
Devin, make sure you reply inline to all bot feedback. |
Aaron ("AJ") Steers (aaronsteers)
deleted the
devin/1779122008-config-api-root-override
branch
Summary
CloudConnection.dump_raw_catalog()did not work reliably for self-managed Airbyte instances with a custom publicapi_root, because Config API calls could only derive the Config API root for Airbyte Cloud or an env-var override.config_api_rootplumbing throughCloudWorkspace,CloudClientConfig, Config API helper functions, connection/catalog/state helpers, connector builder helpers, sync result job helpers, and MCP cloud config./api/public/v1by converting them to/api/v1, while still allowing explicitconfig_api_rootoverrides via argument,AIRBYTE_CLOUD_CONFIG_API_URL, or MCP config/header.CloudWorkspacekeyword-only so future optional constructor fields do not destabilize positional argument ordering.api_rootandconfig_api_root.CloudWorkspaceconstruction.Review & Testing Checklist for Human
/api/public/v1->/api/v1) matches expected OSS/Enterprise deployments behind any path prefixes.config_api_roottoCloudWorkspaceworks against a real self-managed instance fordump_raw_catalog().CloudWorkspacekeyword-only is acceptable for public API consumers.config_api_url/X-Airbyte-Cloud-Config-Api-Url) matches expected consumer conventions.Notes
Evidence that
/api/public/v1->/api/v1is a decent default for documented self-managed deployments:<YOUR_AIRBYTE_URL>/api/public/v1/: https://github.com/airbytehq/airbyte/blob/master/docs/developers/api-documentation.md#use-the-right-base-url/api/public/v1/applications/tokenfor self-managed token requests: https://github.com/airbytehq/airbyte/blob/master/docs/platform/using-airbyte/configuring-api-access.md#step-2-get-an-access-token/api/public/v1: https://github.com/airbytehq/airbyte-platform/blob/main/oss/airbyte-api/server-api/src/main/openapi/public_api.yaml#L47-L61/v1/..., e.g. connection/state endpoints used by this codepath: https://github.com/airbytehq/airbyte-platform/blob/main/oss/airbyte-api/server-api/src/main/openapi/config.yaml#L2913-L2917 and https://github.com/airbytehq/airbyte-platform/blob/main/oss/airbyte-api/server-api/src/main/openapi/config.yaml#L3051-L3055/v1/connections/*and/v1/state/*: https://github.com/airbytehq/airbyte-platform/blob/main/oss/airbyte-server/src/main/kotlin/io/airbyte/server/apis/controllers/ConnectionApiController.kt#L75 and https://github.com/airbytehq/airbyte-platform/blob/main/oss/airbyte-server/src/main/kotlin/io/airbyte/server/apis/controllers/StateApiController.kt#L22/api/v1and Public API at/api/public: https://github.com/airbytehq/airbyte-platform/blob/main/oss/charts/v2/airbyte/templates/config/_common.tpl#L130-L132 and https://github.com/airbytehq/airbyte-platform/blob/main/oss/charts/v2/airbyte/templates/config/_webapp.tpl#L11-L13Local checks run:
uv run --project /home/ubuntu/repos/PyAirbyte ruff format --check /home/ubuntu/repos/PyAirbyte/airbyte/_util/api_util.py /home/ubuntu/repos/PyAirbyte/tests/unit_tests/test_cloud_api_roots.pyuv run --project /home/ubuntu/repos/PyAirbyte ruff check /home/ubuntu/repos/PyAirbyte/airbyte/_util/api_util.py /home/ubuntu/repos/PyAirbyte/tests/unit_tests/test_cloud_api_roots.pyuv run --project /home/ubuntu/repos/PyAirbyte pyrefly checkuv run --project /home/ubuntu/repos/PyAirbyte pytest /home/ubuntu/repos/PyAirbyte/tests/unit_tests/test_cloud_api_roots.pyuv run --project /home/ubuntu/repos/PyAirbyte ruff format --check /home/ubuntu/repos/PyAirbyte/airbyte/cloud/__init__.pyuv run --project /home/ubuntu/repos/PyAirbyte ruff check /home/ubuntu/repos/PyAirbyte/airbyte/cloud/__init__.pyuv run --project /home/ubuntu/repos/PyAirbyte poe -C /home/ubuntu/repos/PyAirbyte docs-generateuv run --project /home/ubuntu/repos/PyAirbyte ruff format --check /home/ubuntu/repos/PyAirbyte/airbyte/cloud/workspaces.py /home/ubuntu/repos/PyAirbyte/tests/unit_tests/test_cloud_api_roots.pyuv run --project /home/ubuntu/repos/PyAirbyte ruff check /home/ubuntu/repos/PyAirbyte/airbyte/cloud/workspaces.py /home/ubuntu/repos/PyAirbyte/tests/unit_tests/test_cloud_api_roots.pyuv run --project /home/ubuntu/repos/PyAirbyte pyrefly check /home/ubuntu/repos/PyAirbyte/airbyte/cloud/workspaces.py /home/ubuntu/repos/PyAirbyte/tests/unit_tests/test_cloud_api_roots.pyuv run --project /home/ubuntu/repos/PyAirbyte pytest /home/ubuntu/repos/PyAirbyte/tests/unit_tests/test_cloud_api_roots.pyLink to Devin session: https://app.devin.ai/sessions/cc23474934e64bd79c12748baf72dfe0
Requested by: Aaron ("AJ") Steers (@aaronsteers)