Skip to content

Conversation

@aaronsteers
Copy link
Contributor

@aaronsteers aaronsteers commented Dec 19, 2025

feat: Add MCP tool and Python API for connection enable/disable and scheduling

Summary

Adds a consolidated MCP tool and underlying Python methods for enabling/disabling connections and managing connection schedules on Airbyte Cloud. The MCP tool is marked as destructive=True so it will be disabled when the MCP server runs in read-only mode.

New CloudConnection methods:

  • enabled property - Gets/sets connection enabled status (getter always fetches fresh data from API)
  • set_enabled(*, enabled: bool, ignore_noop: bool = True) - Sets connection status with no-op handling
  • set_schedule(cron_expression) - Sets a cron schedule for automatic syncs
  • set_manual_schedule() - Disables automatic syncs

New MCP tool:

  • update_cloud_connection - Consolidated tool accepting optional enabled, cron_expression, and manual_schedule parameters

Updates Since Last Revision

Per reviewer feedback:

  1. Combined separate enable() and disable() methods into a single set_enabled() method with ignore_noop flag (defaults to True)
  2. Added enabled property with fresh-read getter (never uses cached values) and setter
  3. Consolidated 4 separate MCP tools into a single update_cloud_connection tool that accepts multiple optional parameters

Review & Testing Checklist for Human

  • Verify API compatibility: Confirm that api_util.patch_connection actually accepts status and schedule parameters - I followed the existing pattern but didn't test against the live API
  • Test enable/disable: Try enabling and disabling a real connection to verify the status changes correctly
  • Test scheduling: Try setting a cron schedule and manual schedule on a real connection
  • Verify fresh-read behavior: Confirm the enabled property getter fetches fresh data on each access

Recommended Test Plan

# Test enabling a connection
poe mcp-tool-test update_cloud_connection '{"connection_id": "<your-connection-id>", "enabled": true}'

# Test disabling a connection
poe mcp-tool-test update_cloud_connection '{"connection_id": "<your-connection-id>", "enabled": false}'

# Test setting a cron schedule
poe mcp-tool-test update_cloud_connection '{"connection_id": "<your-connection-id>", "cron_expression": "0 0 * * *"}'

# Test setting manual schedule
poe mcp-tool-test update_cloud_connection '{"connection_id": "<your-connection-id>", "manual_schedule": true}'

# Test multiple settings at once
poe mcp-tool-test update_cloud_connection '{"connection_id": "<your-connection-id>", "enabled": true, "cron_expression": "0 */6 * * *"}'

Notes

Summary by CodeRabbit

  • New Features
    • Connection lifecycle management: enable or disable cloud connections via the API.
    • Automated sync scheduling: configure CRON-based schedules for automatic syncs.
    • Manual sync scheduling: switch connections to manual sync mode.
    • Management tool: a new operation to update connection state and scheduling (validates inputs and returns a summary of applied changes).

✏️ Tip: You can customize this high-level summary in your review settings.

@devin-ai-integration
Copy link
Contributor

Original prompt from AJ Steers
SYSTEM:
=== BEGIN THREAD HISTORY (in #hydra-pyairbyte-mcp) ===
Aldo Alberto Jesus Gonzalez Campos (U070BMPDUHJ): Something we want a tool for?
```I don't have a tool to enable/disable connections directly, but I can help you figure out how to enable it!```

AJ Steers (U05AKF1BCC9): Indeed! I don't think we have MCP tools for enabling or disabling, nor do we have them for scheduling. I believe there's a Python interface for scheduling, but perhaps not for enabling disabling.

AJ Steers (U05AKF1BCC9): I'm going to ask Devin to do it, or at least start a PR. 

Extra context: 
• While invoking a sync is considered non-destructive and non-modifying, these new interfaces are not in that same bucket. They would be disabled and should be disabled if the MCP server is in read-only mode. 
• Also, I think we already have a generic tool to modify the connection or update the connection, in one or more ways, and We should first see if we can just fit these new types of changes into the existing tool as input args.

AJ Steers (U05AKF1BCC9): @Devin :point_up:
=== END THREAD HISTORY ===

Thread URL: https://airbytehq-team.slack.com/archives/C065V6XFWNQ/p1766152832997159?thread_ts=1766152832.997159&cid=C065V6XFWNQ

The latest message is the one right above that tagged you.

@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions
Copy link

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

Testing This PyAirbyte Version

You 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/1766175960-connection-enable-disable-schedule' pyairbyte --help

# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1766175960-connection-enable-disable-schedule'

Helpful Resources

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /fix-pr - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test-pr - Runs tests with the updated PyAirbyte
  • /prerelease - Builds and publishes a prerelease version to PyPI

Community Support

Questions? Join the #pyairbyte channel in our Slack workspace.

📝 Edit this welcome message.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 19, 2025

📝 Walkthrough

Walkthrough

Adds enable/disable and CRON/manual scheduling methods to CloudConnection (refreshing state, patching via API, updating internal info, returning self) and a single MCP tool function update_cloud_connection that applies one or more of those changes to a connection and returns a summary.

Changes

Cohort / File(s) Summary
CloudConnection lifecycle & scheduling
airbyte/cloud/connections.py
Added enabled property; set_enabled(self, *, enabled: bool, ignore_noop: bool = True), set_schedule(self, cron_expression: str), and set_manual_schedule(self) which refresh connection state, call api_util.patch_connection with lifecycle or schedule payloads (using AirbyteAPIConnectionSchedule for cron), update _connection_info from API response, and return self.
MCP tool: cloud connection management
airbyte/mcp/cloud_ops.py
Added update_cloud_connection(connection_id, workspace_id=None, enabled=None, cron_expression=None, manual_schedule=None) -> str which validates inputs (requires at least one change, disallows cron + manual simultaneously), resolves workspace, fetches the CloudConnection, applies enabled/schedule/manual changes as provided, accumulates a change summary, and returns a formatted success message including the connection URL.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant MCP_Tool as update_cloud_connection
    participant WorkspaceResolver as WorkspaceResolver
    participant CloudWorkspace as CloudWorkspace
    participant CloudConnection as CloudConnection
    participant AirbyteAPI as AirbyteAPI

    User->>MCP_Tool: call update_cloud_connection(params)
    MCP_Tool->>WorkspaceResolver: resolve workspace (workspace_id?)
    WorkspaceResolver-->>MCP_Tool: CloudWorkspace
    MCP_Tool->>CloudWorkspace: get connection by id
    CloudWorkspace-->>MCP_Tool: CloudConnection
    MCP_Tool->>CloudConnection: refresh state / fetch current info
    CloudConnection->>AirbyteAPI: GET /connections/{id}
    AirbyteAPI-->>CloudConnection: connection info
    alt enabled change provided
        MCP_Tool->>CloudConnection: set_enabled(enabled)
        CloudConnection->>AirbyteAPI: PATCH /connections/{id} (status payload)
        AirbyteAPI-->>CloudConnection: updated connection info
    end
    alt cron_expression provided
        MCP_Tool->>CloudConnection: set_schedule(cron_expression)
        CloudConnection->>AirbyteAPI: PATCH /connections/{id} (schedule payload)
        AirbyteAPI-->>CloudConnection: updated connection info
    end
    alt manual_schedule provided
        MCP_Tool->>CloudConnection: set_manual_schedule()
        CloudConnection->>AirbyteAPI: PATCH /connections/{id} (manual schedule payload)
        AirbyteAPI-->>CloudConnection: updated connection info
    end
    CloudConnection-->>MCP_Tool: returns updated instance
    MCP_Tool-->>User: formatted success message with connection URL and summary
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect airbyte/mcp/cloud_ops.py for input validation edge cases and error messaging.
  • Verify set_enabled behavior around ignore_noop and the raised error on redundant transitions.
  • Confirm payload shapes and use of AirbyteAPIConnectionSchedule match the API contract and that _connection_info is consistently refreshed from PATCH responses.
  • Check any logging or telemetry expectations and whether tests are needed for combined operations (enabled + schedule in one call). wdyt?

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: adding MCP tools and Python API methods for managing connection enable/disable and scheduling functionality.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/1766175960-connection-enable-disable-schedule

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
airbyte/cloud/connections.py (1)

438-468: Consider validating the cron expression format, wdyt?

The implementation follows the established pattern—no input validation before making the API call. This is consistent across all CloudConnection setter methods (rename(), set_table_prefix(), set_selected_streams(), etc.), suggesting a deliberate design choice to delegate validation to the API. Invalid formats will be rejected by the API, though users may see less detailed error messages upstream.

If you'd prefer to add client-side validation for better UX, that would be a nice enhancement. Otherwise, leaving it to the API is a reasonable approach given the existing architecture.

Since no unit tests were found for this method, it would be good to verify that:

  1. Invalid cron expressions are rejected gracefully by the API
  2. The schedule is correctly applied and persisted
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0f76d41 and 330cfeb.

📒 Files selected for processing (2)
  • airbyte/cloud/connections.py (2 hunks)
  • airbyte/mcp/cloud_ops.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
airbyte/mcp/cloud_ops.py (4)
airbyte/mcp/_tool_utils.py (1)
  • mcp_tool (189-235)
tests/integration_tests/cloud/conftest.py (1)
  • workspace_id (49-50)
airbyte/cloud/workspaces.py (1)
  • get_connection (315-327)
airbyte/cloud/connections.py (4)
  • enable (397-414)
  • disable (416-434)
  • set_schedule (438-468)
  • set_manual_schedule (470-490)
airbyte/cloud/connections.py (1)
airbyte/_util/api_util.py (1)
  • patch_connection (1337-1399)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Pytest (All, Python 3.10, Windows)
  • GitHub Check: Pytest (All, Python 3.11, Windows)
  • GitHub Check: Pytest (All, Python 3.11, Ubuntu)
  • GitHub Check: Pytest (All, Python 3.10, Ubuntu)
  • GitHub Check: Pytest (Fast)
  • GitHub Check: Pytest (No Creds)
🔇 Additional comments (3)
airbyte/cloud/connections.py (3)

19-19: LGTM - appropriate lint suppression.

The noqa comment is justified given the intentional expansion of the class's public API surface with these new lifecycle and scheduling methods.


397-434: LGTM - enable/disable implementation looks solid.

Both methods follow the established pattern in this class (similar to rename() and set_table_prefix()), and I've verified from the provided code snippets that api_util.patch_connection does accept the status parameter with type models.ConnectionStatusEnum | None.

The docstrings clearly explain the behavior, particularly the note in disable() that manual syncs can still be triggered.


470-490: LGTM - clean implementation.

The method is straightforward and correctly implements manual scheduling by setting schedule_type=MANUAL. The docstring clearly explains that this disables automatic syncs while still allowing manual triggers.

Comment on lines 2296 to 2436
@mcp_tool(
domain="cloud",
open_world=True,
destructive=True,
extra_help_text=CLOUD_AUTH_TIP_TEXT,
)
def enable_cloud_connection(
connection_id: Annotated[
str,
Field(description="The ID of the connection to enable."),
],
*,
workspace_id: Annotated[
str | None,
Field(
description=WORKSPACE_ID_TIP_TEXT,
default=None,
),
],
) -> str:
"""Enable a connection on Airbyte Cloud.
Sets the connection status to 'active', allowing scheduled syncs to run.
"""
workspace: CloudWorkspace = _get_cloud_workspace(workspace_id)
connection = workspace.get_connection(connection_id=connection_id)
connection.enable()
return (
f"Successfully enabled connection '{connection_id}'. " f"URL: {connection.connection_url}"
)


@mcp_tool(
domain="cloud",
open_world=True,
destructive=True,
extra_help_text=CLOUD_AUTH_TIP_TEXT,
)
def disable_cloud_connection(
connection_id: Annotated[
str,
Field(description="The ID of the connection to disable."),
],
*,
workspace_id: Annotated[
str | None,
Field(
description=WORKSPACE_ID_TIP_TEXT,
default=None,
),
],
) -> str:
"""Disable a connection on Airbyte Cloud.
Sets the connection status to 'inactive', preventing scheduled syncs from running.
Manual syncs can still be triggered using the run_cloud_sync tool.
"""
workspace: CloudWorkspace = _get_cloud_workspace(workspace_id)
connection = workspace.get_connection(connection_id=connection_id)
connection.disable()
return (
f"Successfully disabled connection '{connection_id}'. " f"URL: {connection.connection_url}"
)


@mcp_tool(
domain="cloud",
open_world=True,
destructive=True,
extra_help_text=CLOUD_AUTH_TIP_TEXT,
)
def set_cloud_connection_schedule(
connection_id: Annotated[
str,
Field(description="The ID of the connection to update."),
],
cron_expression: Annotated[
str,
Field(
description=(
"A cron expression defining when syncs should run. "
"Examples: '0 0 * * *' (daily at midnight UTC), "
"'0 */6 * * *' (every 6 hours), "
"'0 0 * * 0' (weekly on Sunday at midnight UTC)."
)
),
],
*,
workspace_id: Annotated[
str | None,
Field(
description=WORKSPACE_ID_TIP_TEXT,
default=None,
),
],
) -> str:
"""Set a cron schedule for a connection on Airbyte Cloud.
Configures the connection to automatically sync on the specified schedule.
"""
workspace: CloudWorkspace = _get_cloud_workspace(workspace_id)
connection = workspace.get_connection(connection_id=connection_id)
connection.set_schedule(cron_expression=cron_expression)
return (
f"Successfully set schedule for connection '{connection_id}' "
f"to '{cron_expression}'. URL: {connection.connection_url}"
)


@mcp_tool(
domain="cloud",
open_world=True,
destructive=True,
extra_help_text=CLOUD_AUTH_TIP_TEXT,
)
def set_cloud_connection_manual_schedule(
connection_id: Annotated[
str,
Field(description="The ID of the connection to update."),
],
*,
workspace_id: Annotated[
str | None,
Field(
description=WORKSPACE_ID_TIP_TEXT,
default=None,
),
],
) -> str:
"""Set a connection to manual scheduling on Airbyte Cloud.
Disables automatic syncs. Syncs will only run when manually triggered
using the run_cloud_sync tool.
"""
workspace: CloudWorkspace = _get_cloud_workspace(workspace_id)
connection = workspace.get_connection(connection_id=connection_id)
connection.set_manual_schedule()
return (
f"Successfully set connection '{connection_id}' to manual scheduling. "
f"URL: {connection.connection_url}"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Should these tools include session guards for safety, wdyt?

I notice that other destructive connection operations in this file use check_guid_created_in_session(connection_id) to prevent modifying resources not created in the current session:

  • set_cloud_connection_table_prefix (line 2239)
  • set_cloud_connection_selected_streams (line 2283)

However, these four new tools don't include this guard. For consistency and safety in MCP safe mode, should we add:

check_guid_created_in_session(connection_id)

at the start of each tool (after the workspace/connection retrieval)?

This would align with the pattern used by similar destructive connection operations and provide the session-based safety mechanism mentioned in the register_cloud_ops_tools docstring at line 2495.

That said, I also notice rename_cloud_connection (line 2182) doesn't use this guard either, so there's some inconsistency in the file already. It might be worth standardizing the approach across all connection modification tools.

🔎 Proposed fix to add session guards

For enable_cloud_connection:

 def enable_cloud_connection(
     connection_id: Annotated[
         str,
         Field(description="The ID of the connection to enable."),
     ],
     *,
     workspace_id: Annotated[
         str | None,
         Field(
             description=WORKSPACE_ID_TIP_TEXT,
             default=None,
         ),
     ],
 ) -> str:
     """Enable a connection on Airbyte Cloud.
 
     Sets the connection status to 'active', allowing scheduled syncs to run.
     """
+    check_guid_created_in_session(connection_id)
     workspace: CloudWorkspace = _get_cloud_workspace(workspace_id)
     connection = workspace.get_connection(connection_id=connection_id)
     connection.enable()

Apply the same pattern to disable_cloud_connection, set_cloud_connection_schedule, and set_cloud_connection_manual_schedule.

🤖 Prompt for AI Agents
In airbyte/mcp/cloud_ops.py around lines 2296 to 2436, the four tools
enable_cloud_connection, disable_cloud_connection,
set_cloud_connection_schedule, and set_cloud_connection_manual_schedule are
missing the session guard used by other destructive connection ops; add a call
to check_guid_created_in_session(connection_id) immediately after retrieving the
connection (i.e., after connection = workspace.get_connection(...)) in each of
the four functions so they follow the same safety pattern as
set_cloud_connection_table_prefix and set_cloud_connection_selected_streams.


# Enable/Disable

def enable(self) -> CloudConnection:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's combine these into a single method to set enabled status. Let's also add an input arg to ignore no op, meaning don't raise if the status is already what we are asking for. Default behavior is to Not raise If the request turns out to be a no-op.

Then, on top of that, add an implementation that's a simpler enabled property, calling the method with the default option to not raise if it's tried to set to something that's already existing. When retrieving this value, we will not use any cached values, because we won't know if another process or human has toggled the setting in between us checking it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! I've refactored the implementation based on your feedback:

  1. Combined enable() and disable() into a single set_enabled(*, enabled: bool, ignore_noop: bool = True) method
  2. Added an enabled property with:
    • Getter: Always fetches fresh data from the API (doesn't use cached values)
    • Setter: Calls set_enabled() with the default ignore_noop=True
  3. The ignore_noop flag defaults to True (silently returns if already in requested state)
  4. Updated the MCP tools to use connection.set_enabled(enabled=True/False)

The enabled parameter is keyword-only to satisfy the FBT001 lint rule (no boolean positional arguments).

@github-actions
Copy link

github-actions bot commented Dec 19, 2025

PyTest Results (Fast Tests Only, No Creds)

348 tests  ±0   348 ✅ ±0   5m 57s ⏱️ +6s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit b5f1ba7. ± Comparison against base commit 0f76d41.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
airbyte/mcp/cloud_ops.py (1)

2296-2325: Missing session guard for consistency with other destructive tools.

This concern was already raised in a previous review - set_cloud_connection_table_prefix and set_cloud_connection_selected_streams include check_guid_created_in_session(connection_id) but these new tools don't. Should we add it for consistency, wdyt?

🧹 Nitpick comments (1)
airbyte/cloud/connections.py (1)

472-502: Solid scheduling implementation!

The methods follow the established patterns nicely. One thought: the cron_expression is passed directly to the API without client-side validation. This is fine since the API will reject invalid formats, but would you consider adding a quick format check (e.g., ensuring 5 space-separated fields) to provide earlier feedback to users, wdyt? Not blocking - just a thought for UX.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 330cfeb and 7a87085.

📒 Files selected for processing (2)
  • airbyte/cloud/connections.py (2 hunks)
  • airbyte/mcp/cloud_ops.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
airbyte/mcp/cloud_ops.py (2)
airbyte/cloud/workspaces.py (2)
  • CloudWorkspace (88-886)
  • get_connection (315-327)
airbyte/cloud/connections.py (3)
  • set_enabled (420-468)
  • enabled (398-408)
  • enabled (411-418)
airbyte/cloud/connections.py (1)
airbyte/_util/api_util.py (1)
  • patch_connection (1337-1399)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Pytest (All, Python 3.10, Windows)
  • GitHub Check: Pytest (All, Python 3.11, Windows)
  • GitHub Check: Pytest (All, Python 3.10, Ubuntu)
  • GitHub Check: Pytest (All, Python 3.11, Ubuntu)
  • GitHub Check: Pytest (No Creds)
  • GitHub Check: Pytest (Fast)
🔇 Additional comments (7)
airbyte/cloud/connections.py (4)

19-19: LGTM on the noqa suppression!

The # noqa: PLR0904 is reasonable given the new methods being added. The class is growing to provide a complete API surface for connection management, so suppressing this lint warning makes sense here.


397-408: Nice implementation of the enabled property getter!

The fresh API fetch on every access is the right call here since connection status can change externally. This aligns perfectly with the requirement to avoid stale cached values.


410-468: Well-designed set_enabled implementation!

The combined method with ignore_noop flag is clean, and the property setter that delegates with ignore_noop=True provides a nice ergonomic API. The fresh fetch before patching ensures we're always working with current state - good defensive design.


504-524: Clean set_manual_schedule implementation!

Straightforward and consistent with the set_schedule method. The docstring clearly explains the behavior.

airbyte/mcp/cloud_ops.py (3)

2328-2358: Good docstring addition!

The clarification that "manual syncs can still be triggered using the run_cloud_sync tool" is helpful context for MCP consumers. Implementation looks correct.


2361-2402: Nice inline examples in the field description!

The cron expression examples ('0 0 * * *', '0 */6 * * *', '0 0 * * 0') in the Field description are really helpful for LLM agents that will consume this tool. Good design choice.


2405-2436: Clean implementation!

The docstring's mention of run_cloud_sync for manual triggering creates a nice cross-reference for tool discovery. LGTM.

@devin-ai-integration devin-ai-integration bot changed the title feat: Add MCP tools for connection enable/disable and scheduling feat: Add MCP tool and Python API for connection enable/disable and scheduling Dec 19, 2025
@devin-ai-integration
Copy link
Contributor

I've consolidated the 4 separate tools into a single update_cloud_connection tool as requested. The new tool accepts optional parameters:

  • enabled (bool | None) - Set connection status to active/inactive
  • cron_expression (str | None) - Set a cron schedule for automatic syncs
  • manual_schedule (bool | None) - Switch to manual-only scheduling

The tool validates that at least one setting is provided and that cron_expression and manual_schedule are mutually exclusive. Multiple settings can be updated in a single call.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
airbyte/mcp/cloud_ops.py (1)

2302-2401: Should this tool include a session guard for safety, wdyt?

Similar to the concern raised in previous reviews, this consolidated tool doesn't call check_guid_created_in_session(connection_id). Other destructive connection operations in this file use this guard for consistency and safety in MCP safe mode:

  • set_cloud_connection_table_prefix (line 2239)
  • set_cloud_connection_selected_streams (line 2283)

Adding the session guard would align with the pattern used by similar destructive connection operations and provide the session-based safety mechanism mentioned in the register_cloud_ops_tools docstring.

🔎 Proposed fix to add session guard
 def update_cloud_connection(
     connection_id: Annotated[
         str,
         Field(description="The ID of the connection to update."),
     ],
     *,
     enabled: Annotated[
         bool | None,
         Field(
             description=(
                 "Set the connection's enabled status. "
                 "True enables the connection (status='active'), "
                 "False disables it (status='inactive'). "
                 "Leave unset to keep the current status."
             ),
             default=None,
         ),
     ],
     cron_expression: Annotated[
         str | None,
         Field(
             description=(
                 "A cron expression defining when syncs should run. "
                 "Examples: '0 0 * * *' (daily at midnight UTC), "
                 "'0 */6 * * *' (every 6 hours), "
                 "'0 0 * * 0' (weekly on Sunday at midnight UTC). "
                 "Leave unset to keep the current schedule. "
                 "Cannot be used together with 'manual_schedule'."
             ),
             default=None,
         ),
     ],
     manual_schedule: Annotated[
         bool | None,
         Field(
             description=(
                 "Set to True to disable automatic syncs (manual scheduling only). "
                 "Syncs will only run when manually triggered. "
                 "Cannot be used together with 'cron_expression'."
             ),
             default=None,
         ),
     ],
     workspace_id: Annotated[
         str | None,
         Field(
             description=WORKSPACE_ID_TIP_TEXT,
             default=None,
         ),
     ],
 ) -> str:
     """Update a connection's settings on Airbyte Cloud.
 
     This tool allows updating multiple connection settings in a single call:
     - Enable or disable the connection
     - Set a cron schedule for automatic syncs
     - Switch to manual scheduling (no automatic syncs)
 
     At least one setting must be provided. The 'cron_expression' and 'manual_schedule'
     parameters are mutually exclusive.
     """
+    check_guid_created_in_session(connection_id)
+
     # Validate that at least one setting is provided
     if enabled is None and cron_expression is None and manual_schedule is None:
🧹 Nitpick comments (1)
airbyte/mcp/cloud_ops.py (1)

2365-2368: Consider using PyAirbyteInputError for consistency, wdyt?

The validation errors here raise ValueError, but other similar validation errors in this file use PyAirbyteInputError (see lines 710-713, 1039-1042, 1270-1276, 1703-1712). Using the consistent exception type would align with the established pattern.

🔎 Proposed fix
     # Validate that at least one setting is provided
     if enabled is None and cron_expression is None and manual_schedule is None:
-        raise ValueError(
-            "At least one setting must be provided: 'enabled', 'cron_expression', "
-            "or 'manual_schedule'."
+        raise PyAirbyteInputError(
+            message=(
+                "At least one setting must be provided: 'enabled', 'cron_expression', "
+                "or 'manual_schedule'."
+            ),
         )
 
     # Validate mutually exclusive schedule options
     if cron_expression is not None and manual_schedule is True:
-        raise ValueError(
-            "Cannot specify both 'cron_expression' and 'manual_schedule=True'. "
-            "Use 'cron_expression' for scheduled syncs or 'manual_schedule=True' "
-            "for manual-only syncs."
+        raise PyAirbyteInputError(
+            message=(
+                "Cannot specify both 'cron_expression' and 'manual_schedule=True'. "
+                "Use 'cron_expression' for scheduled syncs or 'manual_schedule=True' "
+                "for manual-only syncs."
+            ),
         )

Also applies to: 2372-2376

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a87085 and b5f1ba7.

📒 Files selected for processing (1)
  • airbyte/mcp/cloud_ops.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airbyte/mcp/cloud_ops.py (3)
airbyte/mcp/_tool_utils.py (1)
  • mcp_tool (189-235)
airbyte/cloud/connections.py (6)
  • enabled (398-408)
  • enabled (411-418)
  • set_enabled (420-468)
  • set_schedule (472-502)
  • set_manual_schedule (504-524)
  • connection_url (160-162)
airbyte/cloud/workspaces.py (1)
  • get_connection (315-327)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Pytest (All, Python 3.10, Ubuntu)
  • GitHub Check: Pytest (All, Python 3.10, Windows)
  • GitHub Check: Pytest (All, Python 3.11, Windows)
  • GitHub Check: Pytest (All, Python 3.11, Ubuntu)
  • GitHub Check: Pytest (Fast)
  • GitHub Check: Pytest (No Creds)

@github-actions
Copy link

PyTest Results (Full)

418 tests  ±0   401 ✅ ±0   23m 23s ⏱️ - 1m 7s
  1 suites ±0    17 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit b5f1ba7. ± Comparison against base commit 0f76d41.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants