You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "terrymanu (via GitHub)" <gi...@apache.org> on 2026/07/17 06:48:11 UTC

[PR] Improve MCP readwrite-splitting status planning [shardingsphere]

terrymanu opened a new pull request, #39164:
URL: https://github.com/apache/shardingsphere/pull/39164

   - fail status plans early when Proxy is not in Cluster mode
   - return terminal guidance and document the Cluster requirement
   - cover Cluster and Standalone paths with unit and Proxy E2E tests
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Improve MCP readwrite-splitting status planning [shardingsphere]

Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #39164:
URL: https://github.com/apache/shardingsphere/pull/39164


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Improve MCP readwrite-splitting status planning [shardingsphere]

Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39164:
URL: https://github.com/apache/shardingsphere/pull/39164#issuecomment-5000035989

   ### Summary
   
   **Review Result: Mergeable**
   
   **Reason:** Excluding `AGENTS.md` as requested, no merge-blocking issues were found within the MCP code scope. The Cluster-mode precondition check, failure guidance, descriptors, unit tests, and Proxy E2E contract form a complete loop. This conclusion is limited to code scope; CI not reviewed by request.
   
   ### Evidence
   
   - `AlterReadwriteSplittingStorageUnitStatusExecutor` explicitly requires Cluster mode, and the planning-stage check is consistent with the underlying execution constraint.
   - Standalone or unknown modes return `WF-MODE-002`, generate no artifacts, and return a terminal next action.
   - Cluster, Standalone, empty-query-result, and guidance branches are covered by tests.
   - All tests in the `mcp/support` and `readwrite-splitting` modules pass, with no Checkstyle violations.
   - No blocking issues were found in public APIs, dependencies, performance, state cleanup, or documentation contracts.
   
   ### Suggestions
   
   The following are non-blocking suggestions and can be considered later:
   
   - Add a mixed-case `mode_type` test to explicitly protect the `equalsIgnoreCase` behavior.
   - Add a test that reuses a `plan_id` after switching from Cluster to Standalone, verifying that old artifacts are cleared; the existing `clearPlanningState()` implementation already provides this protection.
   - The current `CLUSTER_MODE` constant and `queryProxyMode` placement are simple and clear. No additional abstraction is recommended.
   
   ### Review Details
   
   - **Review Focus:** Code Correctness Review. CI not reviewed by request.
   - **Reviewed Scope:** MCP readwrite-splitting, MCP support, and MCP E2E code.
   - **Not Reviewed Scope:** `AGENTS.md`, GitHub Actions, CI checks, and workflow logs.
   - **Verification:** MCP support and readwrite-splitting tests passed; Checkstyle passed; the corresponding Proxy E2E scenario passed.
   - **Release Note / User Docs:** No release note is required; the MCP descriptor and prompt were updated consistently.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org