feat(mcp): Add definition_id to source results and org-level search tool#907
feat(mcp): Add definition_id to source results and org-level search tool#907devin-ai-integration[bot] wants to merge 1 commit intomainfrom
Conversation
- Add definition_id property to CloudSource class - Add definition_id field to CloudSourceResult model - Update list_deployed_cloud_source_connectors to include definition_id - Add new list_sources_by_definition_in_organization tool for searching sources by connector type across all workspaces in an organization Co-Authored-By: aldo.gonzalez@airbyte.io <aldo.gonzalez@airbyte.io>
Original prompt from aldo.gonzalez@airbyte.io |
🤖 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. Testing 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/1765422640-add-definition-id-to-mcp-tools' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1765422640-add-definition-id-to-mcp-tools'Helpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
Community SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
| return OrgSourcesByDefinitionResult( | ||
| organization_id=resolved_org_id, | ||
| definition_id=definition_id, | ||
| total_sources_found=total_sources_found, | ||
| workspaces_with_matches=workspaces_with_matches, | ||
| ) |
There was a problem hiding this comment.
Something feels odd the name is list_sources_by_definition_in_organization but we return workspaces_with_matches=workspaces_with_matches, meh, me no gustar. Do you have different ideas for this?
PyTest Results (Full)416 tests ±0 398 ✅ - 1 24m 26s ⏱️ -2s For more details on these failures, see this check. Results for commit ece066f. ± Comparison against base commit 441105e. |
| return self.connector_id | ||
|
|
||
| @property | ||
| def definition_id(self) -> str: |
There was a problem hiding this comment.
Aldo Gonzalez (@aldogonzalez8) (and Devin I guess): Can we call this connector_definition_id? On its own "definition ID" could be misunderstood to mean something about the actor (instance) ID. I'm curious also if we could get something similar working with the canonical ID (source-something rather than require the definition ID to be exposed for users.
One more thing to think about: this might have a better home in the new Internal-Ops. It looks like you and I are maybe thinking similarly?
There was a problem hiding this comment.
(Or source_definition_id/destination_definition_id? I think I have some references already to this concept in the custom connections interfaces.)
There was a problem hiding this comment.
Good suggestions! I can make these changes if Aldo Gonzalez (@aldogonzalez8) agrees:
-
Rename to
connector_definition_id- Makes it clearer this refers to the connector type, not the actor instance -
Support canonical ID - Could add a
connector_typeparameter (e.g.,source-youtube-analytics) as an alternative to the UUID. The tool would resolve it to the definition ID internally.
Waiting for direction on whether to proceed with these changes here or move this to Internal-Ops.
There was a problem hiding this comment.
source_definition_id makes sense since this is on the CloudSource class - it's more specific than the generic connector_definition_id. I can rename it once there's consensus on the preferred naming.
|
Closing this PR as we've decided to move this functionality to Internal-Ops per discussion with Aaron ("AJ") Steers (@aaronsteers) and Aldo Gonzalez (@aldogonzalez8). Will open a new PR there with the updated design (connection-centric, connector_definition_id naming, canonical name support). |
feat(mcp): Add definition_id to source results and org-level search tool
Summary
This PR adds the ability to identify sources by their connector type (definition_id) rather than just by name. It includes:
definition_idproperty onCloudSource- Exposes the connector definition ID (source type) from the underlying API responseCloudSourceResult- Now includesdefinition_idfield in the responselist_deployed_cloud_source_connectors- Returnsdefinition_idfor each sourcelist_sources_by_definition_in_organizationtool - Searches all workspaces in an organization for sources matching a specific connector typeThis enables use cases like "find all YouTube Analytics sources in an organization" regardless of what the sources are named.
Review & Testing Checklist for Human
definition_idis correctly populated - The property accesses_connector_info.definition_idwhich should be available from the Airbyte API'sSourceResponse. Confirm this field exists and is populated correctly.list_sources_by_definition_in_organizationagainst an org with known sources to verify it finds them correctlydefinition_idfield gracefullyRecommended Test Plan
list_deployed_cloud_source_connectorsand verifydefinition_idappears in resultslist_sources_by_definition_in_organizationwith a known connector definition ID (e.g.,afa734e4-3571-11ec-991a-1e0031268139for YouTube Analytics) and verify it finds expected sourcesNotes
CloudSource.definition_idproperty and the org-level toolpoe fix-and-check)Link to Devin run: https://app.devin.ai/sessions/d8212a7bceaf4995a63369460134dc03
Requested by: aldo.gonzalez@airbyte.io