feat(cloud): add CloudClient, move CloudOrganization to dedicated cloud.organizations module #1030
Merged
Aaron ("AJ") Steers (aaronsteers) merged 4 commits intoMay 20, 2026
Conversation
- Add CloudClient class with from_auth(env_vars=False) factory - Move CloudOrganization from workspaces.py to organizations.py - Add internal _AirbyteCredentials for shared credential resolution - Add CloudWorkspace.list_workspaces() method - Refactor MCP cloud module to use CloudClient internally - Export CloudClient and CloudOrganization from airbyte.cloud - Add CLOUD_ORGANIZATION_ID_ENV_VAR constant - Add WorkspaceResponse to api_imports Does not include login/logout or credentials-file persistence; those will follow in a stacked PR.
Contributor
🤖 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/1779226376-cloud-core-refactor' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1779226376-cloud-core-refactor'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. |
Contributor
📝 WalkthroughWalkthroughCentralizes Airbyte Cloud credential resolution into _AirbyteCredentials, adds CloudClient as the unified entry point for workspace and organization operations, introduces CloudOrganization with lazy caching, refactors CloudWorkspace and MCP tooling to use the new client/credentials, updates package exports, and adds unit tests for auth/listing/caching behaviors. ChangesCloud Client & Credential Unification
Sequence Diagram(s)sequenceDiagram
participant User
participant CloudClient
participant api_util
participant CloudWorkspace
participant CloudOrganization
User->>CloudClient: list_workspaces(name_contains, organization_id?)
alt organization scope present
CloudClient->>api_util: list_workspaces_in_organization(organization_id, ...)
api_util-->>CloudClient: workspace list (unfiltered)
CloudClient->>CloudClient: apply name_filter / limit
CloudClient-->>User: filtered/truncated list
else global scope
CloudClient->>api_util: list_workspaces(workspace_id="")
api_util-->>CloudClient: workspace list
CloudClient->>CloudClient: apply name_contains -> name_filter
CloudClient-->>User: filtered list
end
User->>CloudClient: get_organization(organization_id or organization_name)
CloudClient->>api_util: list_organizations_for_user()
api_util-->>CloudClient: organizations
CloudClient->>CloudOrganization: construct scoped CloudOrganization
CloudOrganization-->>CloudClient: org object
CloudClient-->>User: CloudOrganization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Would you like me to highlight specific lines for the highest-risk validation logic (e.g., 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
airbyte/mcp/cloud.py (1)
1409-1425: 💤 Low valueConsider simplifying by passing the client to
_resolve_organization_id?The current flow creates a
CloudClient, extracts its credentials to pass to_resolve_organization_id, which then constructs anotherCloudClientinternally. This means two clients are instantiated for this operation. Would it be cleaner to have_resolve_organization_idaccept aCloudClientdirectly, or haveclient.list_workspaces()handle the organization resolution internally when given a name? Just a thought for a future iteration, wdyt?🤖 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 1409 - 1425, Change _resolve_organization_id to accept an already-instantiated CloudClient instead of separate credential params and stop creating a new CloudClient inside it; update the caller in this diff to call _resolve_organization_id(client=client, organization_id=organization_id, organization_name=organization_name) (remove api_root/client_id/client_secret/bearer_token/config_api_root args), and inside _resolve_organization_id use the passed CloudClient methods/properties to resolve org id; ensure all other call sites of _resolve_organization_id are updated to pass a CloudClient and adjust type hints/tests accordingly, or alternatively implement org-name resolution inside CloudClient.list_workspaces and call that from here.airbyte/cloud/workspaces.py (1)
642-657: 💤 Low valueConsider adding a
limitparameter for consistency?The other list methods (
list_connections,list_sources,list_destinations) all include alimitparameter, butlist_workspacesdoesn't. For API consistency, would it make sense to add one here too, wdyt?Also, I notice the method passes
workspace_id=""toapi_util.list_workspaces. Just wanted to confirm this empty string is the intended way to signal "list all workspaces" rather thanNone?🤖 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/cloud/workspaces.py` around lines 642 - 657, Add a limit parameter to list_workspaces (e.g., def list_workspaces(self, name: str | None = None, *, name_filter: Callable | None = None, limit: int | None = None) -> list[WorkspaceResponse]) and pass it through to api_util.list_workspaces as limit=limit for API consistency with list_connections/list_sources/list_destinations; keep the workspace_id argument as-is only if api_util.list_workspaces expects an empty string to mean “all” else change workspace_id="" to workspace_id=None so the call signature to api_util.list_workspaces (the symbol to update) matches the utility’s expected sentinel for listing all workspaces.airbyte/cloud/organizations.py (1)
73-76: ⚡ Quick winBare
except Exceptionmay silently hide important errors.The broad exception handler catches and suppresses all errors without logging. While the resilient fallback to cached data is nice, this could make debugging auth failures or network issues difficult. Would you consider logging the exception (at debug or warning level) before returning the fallback, wdyt?
except Exception: + # TODO: Consider logging the exception for debugging if self._organization_info is None: self._organization_info_fetch_failed = True return self._organization_info or {}🤖 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/cloud/organizations.py` around lines 73 - 76, The bare except in the method that returns organization info swallows all errors; update the except block in the function/method that accesses self._organization_info (the block using self._organization_info_fetch_failed) to catch Exception as e and log the exception before returning the fallback. Specifically, change the handler to "except Exception as e:" and call the module/class logger (e.g., self._logger or a module-level logger) to log e at debug or warning level with context like "failed to fetch organization info", then preserve the current behavior of setting self._organization_info_fetch_failed and returning self._organization_info or {}.airbyte/cloud/client.py (1)
30-50: ⚡ Quick winShould
__init__validate credentials likefrom_authdoes?The
__init__method bypasses the credential validation in_AirbyteCredentials.from_auth. Users callingCloudClient()without any credentials will succeed here but fail later at runtime when making API calls. Would it make sense to either:
- Add validation here to fail fast, or
- Mark
__init__as internal (e.g., rename to_initpattern or document thatfrom_authis the preferred entry point)?This could prevent confusing deferred errors, wdyt?
🤖 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/cloud/client.py` around lines 30 - 50, The __init__ of CloudClient currently constructs _AirbyteCredentials directly and skips the validation logic in _AirbyteCredentials.from_auth, allowing callers to instantiate CloudClient without credentials and only fail later; update CloudClient.__init__ to perform the same validation by calling _AirbyteCredentials.from_auth (or delegating credential validation to that method) using the incoming client_id, client_secret, bearer_token, public_api_root, config_api_root, workspace_id, and organization_id, and assign the returned validated credentials to self._credentials (or raise a clear ValueError if validation fails) so construction fails fast.
🤖 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/cloud/client.py`:
- Around line 30-50: The __init__ of CloudClient currently constructs
_AirbyteCredentials directly and skips the validation logic in
_AirbyteCredentials.from_auth, allowing callers to instantiate CloudClient
without credentials and only fail later; update CloudClient.__init__ to perform
the same validation by calling _AirbyteCredentials.from_auth (or delegating
credential validation to that method) using the incoming client_id,
client_secret, bearer_token, public_api_root, config_api_root, workspace_id, and
organization_id, and assign the returned validated credentials to
self._credentials (or raise a clear ValueError if validation fails) so
construction fails fast.
In `@airbyte/cloud/organizations.py`:
- Around line 73-76: The bare except in the method that returns organization
info swallows all errors; update the except block in the function/method that
accesses self._organization_info (the block using
self._organization_info_fetch_failed) to catch Exception as e and log the
exception before returning the fallback. Specifically, change the handler to
"except Exception as e:" and call the module/class logger (e.g., self._logger or
a module-level logger) to log e at debug or warning level with context like
"failed to fetch organization info", then preserve the current behavior of
setting self._organization_info_fetch_failed and returning
self._organization_info or {}.
In `@airbyte/cloud/workspaces.py`:
- Around line 642-657: Add a limit parameter to list_workspaces (e.g., def
list_workspaces(self, name: str | None = None, *, name_filter: Callable | None =
None, limit: int | None = None) -> list[WorkspaceResponse]) and pass it through
to api_util.list_workspaces as limit=limit for API consistency with
list_connections/list_sources/list_destinations; keep the workspace_id argument
as-is only if api_util.list_workspaces expects an empty string to mean “all”
else change workspace_id="" to workspace_id=None so the call signature to
api_util.list_workspaces (the symbol to update) matches the utility’s expected
sentinel for listing all workspaces.
In `@airbyte/mcp/cloud.py`:
- Around line 1409-1425: Change _resolve_organization_id to accept an
already-instantiated CloudClient instead of separate credential params and stop
creating a new CloudClient inside it; update the caller in this diff to call
_resolve_organization_id(client=client, organization_id=organization_id,
organization_name=organization_name) (remove
api_root/client_id/client_secret/bearer_token/config_api_root args), and inside
_resolve_organization_id use the passed CloudClient methods/properties to
resolve org id; ensure all other call sites of _resolve_organization_id are
updated to pass a CloudClient and adjust type hints/tests accordingly, or
alternatively implement org-name resolution inside CloudClient.list_workspaces
and call that from here.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 028f3584-3a40-4bde-983e-86ff29af35f8
📒 Files selected for processing (9)
airbyte/_util/api_imports.pyairbyte/cloud/__init__.pyairbyte/cloud/_credentials.pyairbyte/cloud/client.pyairbyte/cloud/organizations.pyairbyte/cloud/workspaces.pyairbyte/constants.pyairbyte/mcp/cloud.pytests/unit_tests/test_cloud_credentials.py
Copilot started reviewing on behalf of
Aaron ("AJ") Steers (aaronsteers)
May 20, 2026 00:48
View session
Aaron ("AJ") Steers (aaronsteers)
changed the title
CloudClient, move CloudOrganization to dedicated cloud.organizations module
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extracts the “core” Airbyte Cloud client/auth refactor into a standalone change: it introduces a unified CloudClient facade, centralizes credential resolution in an internal _AirbyteCredentials helper, and moves CloudOrganization into its own module, with MCP updated to use the new client.
Changes:
- Added
CloudClient(airbyte/cloud/client.py) withfrom_auth(env_vars=...), workspace listing, workspace construction, and organization resolution. - Introduced internal
_AirbyteCredentials(airbyte/cloud/_credentials.py) for shared auth/env-var resolution and addedCLOUD_ORGANIZATION_ID_ENV_VAR. - Moved
CloudOrganizationintoairbyte/cloud/organizations.py, updated exports/imports, and refactored MCP cloud tools to useCloudClient.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
airbyte/cloud/client.py |
New CloudClient facade: auth factory, workspace/org helpers, workspace listing. |
airbyte/cloud/_credentials.py |
New internal credential/env-var resolution shared by Cloud client/workspace/org. |
airbyte/cloud/organizations.py |
New CloudOrganization module with lazy org info fetching/caching. |
airbyte/cloud/workspaces.py |
Refactors workspace auth initialization to use _AirbyteCredentials; adds list_workspaces(). |
airbyte/cloud/__init__.py |
Exports CloudClient / CloudOrganization and exposes new submodules. |
airbyte/mcp/cloud.py |
Refactors MCP cloud tooling to construct/auth via CloudClient. |
airbyte/constants.py |
Adds CLOUD_ORGANIZATION_ID_ENV_VAR. |
airbyte/_util/api_imports.py |
Exposes WorkspaceResponse for typing usage. |
tests/unit_tests/test_cloud_credentials.py |
Adds unit coverage for credential resolution, list_workspaces forwarding, and org caching 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
airbyte/mcp/cloud.py (2)
1343-1346: 💤 Low valueStale docstring after refactoring?
The docstring says "This is a convenience wrapper around _resolve_organization" but the function now directly calls
client.get_organization()instead. Would you like to update this to reflect the new implementation, wdyt?def _resolve_organization_id( organization_id: str | None, organization_name: str | None, *, client: CloudClient, ) -> str: - """Resolve organization ID from either ID or exact name match. - - This is a convenience wrapper around _resolve_organization that returns just the ID. - """ + """Resolve organization ID from either ID or exact name match.""" org = client.get_organization(🤖 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 1343 - 1346, The docstring for resolve_organization_id is stale — it mentions being a “convenience wrapper around _resolve_organization” while the implementation now calls client.get_organization(); update the docstring to accurately describe the current behavior: state that resolve_organization_id resolves and returns the organization ID by calling client.get_organization() to fetch an organization by ID or exact name (and mention any parameters/return value expectations), and remove the reference to _resolve_organization so the text matches the code.
1290-1335: ⚡ Quick winConsider removing
_resolve_organizationas it's no longer used.The function at line 1290 is dead code after the refactoring. While
_resolve_organization_idmentions it in the docstring as a "convenience wrapper," the actual implementation callsclient.get_organization()directly instead. No callers of_resolve_organizationexist anywhere in the codebase. Removing it would clean up unnecessary code and reduce maintenance burden—should we take care of this?🤖 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 1290 - 1335, The function _resolve_organization is dead code and should be removed; delete the entire _resolve_organization function and any leftover references to it, and update the docstring of _resolve_organization_id to remove the "convenience wrapper" mention (or point it to CloudClient.get_organization) so documentation stays accurate; ensure no callers remain and that behavior continues to use CloudClient.get_organization for organization resolution.airbyte/cloud/client.py (1)
30-50: ⚡ Quick winShould
__init__validate credentials likefrom_authdoes?The
__init__constructor directly creates_AirbyteCredentialswithout callingfrom_auth, which means it bypasses the validation for mixed auth methods (bearer + client creds) and partial client credentials. A user could createCloudClient(bearer_token="x", client_id="y")without an immediate error.Was this intentional to allow more flexibility for direct construction, or would you want to add validation here too, wdyt?
🤖 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/cloud/client.py` around lines 30 - 50, The CloudClient.__init__ currently instantiates _AirbyteCredentials directly and therefore skips the validation logic in _AirbyteCredentials.from_auth that prevents mixed auth (bearer + client creds) and incomplete client credentials; update __init__ to perform the same validation path as from_auth (e.g., call _AirbyteCredentials.from_auth or replicate its checks) so invalid combinations like supplying both bearer_token and client_id or partial client credentials raise immediately, ensuring consistent behavior between CloudClient.__init__ and CloudClient.from_auth.
🤖 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/cloud/client.py`:
- Around line 30-50: The CloudClient.__init__ currently instantiates
_AirbyteCredentials directly and therefore skips the validation logic in
_AirbyteCredentials.from_auth that prevents mixed auth (bearer + client creds)
and incomplete client credentials; update __init__ to perform the same
validation path as from_auth (e.g., call _AirbyteCredentials.from_auth or
replicate its checks) so invalid combinations like supplying both bearer_token
and client_id or partial client credentials raise immediately, ensuring
consistent behavior between CloudClient.__init__ and CloudClient.from_auth.
In `@airbyte/mcp/cloud.py`:
- Around line 1343-1346: The docstring for resolve_organization_id is stale — it
mentions being a “convenience wrapper around _resolve_organization” while the
implementation now calls client.get_organization(); update the docstring to
accurately describe the current behavior: state that resolve_organization_id
resolves and returns the organization ID by calling client.get_organization() to
fetch an organization by ID or exact name (and mention any parameters/return
value expectations), and remove the reference to _resolve_organization so the
text matches the code.
- Around line 1290-1335: The function _resolve_organization is dead code and
should be removed; delete the entire _resolve_organization function and any
leftover references to it, and update the docstring of _resolve_organization_id
to remove the "convenience wrapper" mention (or point it to
CloudClient.get_organization) so documentation stays accurate; ensure no callers
remain and that behavior continues to use CloudClient.get_organization for
organization resolution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c784ebe3-bb34-41ea-ba38-ae2bf848aa24
📒 Files selected for processing (6)
airbyte/cloud/_credentials.pyairbyte/cloud/client.pyairbyte/cloud/organizations.pyairbyte/cloud/workspaces.pyairbyte/mcp/cloud.pytests/unit_tests/test_cloud_credentials.py
Contributor
|
Addressed the three latest CodeRabbit follow-ups in
Validation:
|
Contributor
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 (1)
airbyte/cloud/client.py (1)
224-239:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCould
get_organization()fall back to the client's defaultorganization_idhere?Right now
CloudClient(bearer_token="...", organization_id="org-id").get_organization()still raisesPyAirbyteInputErrorbecause this method only looks at its parameters.get_workspace()andlist_workspaces()already honor defaults stored on the client, so this makes the new public API inconsistent on the happy path. wdyt?Possible fix
def get_organization( self, organization_id: str | None = None, *, organization_name: str | None = None, ) -> CloudOrganization: """Resolve an organization by ID or exact name.""" - if organization_id and organization_name: + resolved_organization_id = organization_id or self.organization_id + + if resolved_organization_id and organization_name: raise exc.PyAirbyteInputError( message="Provide either organization ID or organization name." ) - if not organization_id and not organization_name: + if not resolved_organization_id and not organization_name: raise exc.PyAirbyteInputError( message="Organization ID or organization name is required." ) @@ - if organization_id: + if resolved_organization_id: matching_organizations = [ organization for organization in organizations - if organization.organization_id == organization_id + if organization.organization_id == resolved_organization_id ] else: matching_organizations = [ organization for organization in organizations @@ if not matching_organizations: raise AirbyteMissingResourceError( resource_type="organization", - resource_name_or_id=organization_id or organization_name, + resource_name_or_id=resolved_organization_id or organization_name, )🤖 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/cloud/client.py` around lines 224 - 239, get_organization currently only checks its parameters and raises PyAirbyteInputError if neither organization_id nor organization_name is passed; update it to fall back to the client's default organization_id (e.g. self.organization_id) when organization_id is None so it behaves like get_workspace()/list_workspaces. Concretely: in CloudClient.get_organization, when organization_id is falsy and organization_name is not provided, use self.organization_id before raising an error; keep the mutual-exclusion check (if both provided) and ensure the validation messages remain correct and types handle Optional[str] for organization_id and organization_name.
🧹 Nitpick comments (1)
airbyte/mcp/cloud.py (1)
1295-1298: ⚡ Quick winCould this return the explicit
organization_iddirectly when it's already provided?Even ID-based calls now go through
client.get_organization(...), solist_cloud_workspaces(..., organization_id=...)pays for an extra organization lookup before doing the real work. That adds avoidable latency and another network/permission failure surface to the direct-ID path. wdyt?Possible simplification
def _resolve_organization_id( organization_id: str | None, organization_name: str | None, *, client: CloudClient, ) -> str: """Resolve organization ID from either ID or exact name match.""" + if organization_id is not None: + return organization_id + org = client.get_organization( organization_id=organization_id, organization_name=organization_name, ) return org.organization_id🤖 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 1295 - 1298, When organization_id is supplied to list_cloud_workspaces, avoid calling client.get_organization unnecessarily: if organization_id is not None, use it directly instead of invoking client.get_organization(organization_id=organization_id, organization_name=organization_name); only call client.get_organization when organization_id is None (or when you need to resolve organization_name to an id). Update the logic in list_cloud_workspaces to short-circuit on a provided organization_id, returning or using that id for subsequent work to eliminate the extra lookup and network/permission surface.
🤖 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/cloud/client.py`:
- Around line 224-239: get_organization currently only checks its parameters and
raises PyAirbyteInputError if neither organization_id nor organization_name is
passed; update it to fall back to the client's default organization_id (e.g.
self.organization_id) when organization_id is None so it behaves like
get_workspace()/list_workspaces. Concretely: in CloudClient.get_organization,
when organization_id is falsy and organization_name is not provided, use
self.organization_id before raising an error; keep the mutual-exclusion check
(if both provided) and ensure the validation messages remain correct and types
handle Optional[str] for organization_id and organization_name.
---
Nitpick comments:
In `@airbyte/mcp/cloud.py`:
- Around line 1295-1298: When organization_id is supplied to
list_cloud_workspaces, avoid calling client.get_organization unnecessarily: if
organization_id is not None, use it directly instead of invoking
client.get_organization(organization_id=organization_id,
organization_name=organization_name); only call client.get_organization when
organization_id is None (or when you need to resolve organization_name to an
id). Update the logic in list_cloud_workspaces to short-circuit on a provided
organization_id, returning or using that id for subsequent work to eliminate the
extra lookup and network/permission surface.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 72eee534-3bd9-48b8-9317-aefaf6a561e8
📒 Files selected for processing (3)
airbyte/cloud/client.pyairbyte/mcp/cloud.pytests/unit_tests/test_cloud_credentials.py
Contributor
|
Confirmed this was a real consistency gap. Addressed in Also added focused coverage for Validation:
|
Aaron ("AJ") Steers (aaronsteers)
deleted the
devin/1779226376-cloud-core-refactor
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extracts the core Cloud client/organization refactor from PR #1027, excluding login/logout and credentials-file persistence. This is the first of a two-PR split requested by AJ.
What's included:
CloudClientclass inairbyte/cloud/client.pywithfrom_auth(*, env_vars=False, ...)factory (merges previousfrom_env/from_authinto one method)CloudOrganizationmoved fromworkspaces.pytoairbyte/cloud/organizations.py_AirbyteCredentialsinairbyte/cloud/_credentials.pyfor shared credential resolution (env-var fallback, no file I/O)CloudWorkspace.__init__now acceptsstr | SecretString | Nonefor credentials;workspace_idis optional at signature levelCloudWorkspace.list_workspaces()methodCloudClientandCloudOrganizationexported fromairbyte.cloud.__all__WorkspaceResponseadded toairbyte._util.api_importsCLOUD_ORGANIZATION_ID_ENV_VARadded toairbyte.constantsCloudClientinternallyWhat's NOT included (deferred to stacked PR):
CloudClient.login()/CloudClient.logout()CloudLoginResultdataclassfrom_file,to_file,CREDENTIALS_FILE_PATH)Context: Split requested by AJ from #1027. See also #1010 for the full CLI PR.
Review & Testing Checklist for Human
CloudClient.from_auth(env_vars=True)correctly resolves from env varsCloudClient.from_auth()(defaultenv_vars=False) requires explicit credentialsCloudOrganizationimport path works from bothairbyte.cloudandairbyte.cloud.organizationsuv run pytest tests/unit_tests/ -x— all 302 tests should passNotes
Local validation:
uv run ruff format+uv run ruff check: cleanuv run pyrefly check: 0 errorsuv run pytest tests/unit_tests/: 302 passed, 1 skippedLink to Devin session: https://app.devin.ai/sessions/b97ebb27aa72481d982ebb1b9131b687
Requested by: Aaron ("AJ") Steers (@aaronsteers)
Summary by CodeRabbit
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.