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/06/04 03:22:46 UTC
[PR] Centralize MCP E2E image metadata [shardingsphere]
terrymanu opened a new pull request, #38795:
URL: https://github.com/apache/shardingsphere/pull/38795
Move MCP E2E MySQL image, LLM base image, optional digest, and model checksum into e2e-env.properties. Wire the workflow, local build script, Dockerfile, runtime evidence, and tests to the shared properties.
For #35294.
--
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] Centralize runtime image metadata [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #38795:
URL: https://github.com/apache/shardingsphere/pull/38795
--
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] Centralize runtime image metadata [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #38795:
URL: https://github.com/apache/shardingsphere/pull/38795#issuecomment-4619238061
### Decision
Merge Verdict: Mergeable
I did not find any merge-blocking issues. This PR centralizes MCP E2E LLM runtime, model, and MySQL image metadata in `e2e-env.properties`; the workflow, local Docker build script, and Java test configuration now read from the same source. I also did not find remaining hardcoded image digests in the workflow/Dockerfile or placeholder model names in the reviewed MCP scope.
### Reviewed Scope
- PR: apache/shardingsphere#38795
- Title: Centralize runtime image metadata
- Latest PR head SHA: `e54ecdfa083de5e92bbdbda6773eac815709357c`
- Current base SHA from GitHub API: `440a930fc7687baf9ece33f48243bf85cb3691d9`
- Local merge-base SHA: `ecb7c87c9da4e20986e993a3eb303176b5fd4bcd`
- Local checkout matched PR head: `HEAD == e54ecdfa083de5e92bbdbda6773eac815709357c`
- GitHub `/pulls/38795/files` matched the local triple-dot diff file list exactly.
Files reviewed:
- `.github/workflows/e2e-mcp.yml`
- `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/llm/config/LLME2EConfiguration.java`
- `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/llm/config/LLME2EConfigurationTest.java`
- `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/llm/conversation/artifact/LLME2EArtifactWriter.java`
- `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/llm/conversation/artifact/LLME2EArtifactWriterTest.java`
- `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/llm/conversation/client/LLMChatModelClientTest.java`
- `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/llm/fixture/LLMRuntimeSupport.java`
- `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/llm/fixture/LLMRuntimeSupportTest.java`
- `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/support/runtime/MySQLRuntimeTestSupport.java`
- `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/support/runtime/MySQLRuntimeTestSupportTest.java`
- `test/e2e/mcp/src/test/resources/docker/llm-runtime/Dockerfile`
- `test/e2e/mcp/src/test/resources/docker/llm-runtime/build-local.sh`
- `test/e2e/mcp/src/test/resources/env/e2e-env.properties`
### Findings
No blocking findings.
### Review Notes
- The workflow now loads MCP E2E image and model metadata from `test/e2e/mcp/src/test/resources/env/e2e-env.properties` and passes those values into Docker build args.
- The local `build-local.sh` uses the same property source and override model as the workflow, so local reproduction and GitHub Actions are aligned.
- `LLME2EConfiguration` now owns model metadata through configuration instead of keeping fixed architecture-specific digest constants in Java code.
- `LLMRuntimeSupport` now serves the configured prepackaged model path and records runtime/model metadata in score evidence.
- `MySQLRuntimeTestSupport` now reads the MySQL Testcontainers image from MCP E2E env properties, matching the existing E2E configuration style.
- I checked the previous placeholder-name concern: the default and built image labels still use `ggml-org/Qwen3-1.7B-GGUF:Q4_K_M`, not `foo/model`, `Q4_TEST`, or `test-model`.
### Local Verification
Commands run locally:
- `curl -s https://api.github.com/repos/apache/shardingsphere/pulls/38795`
Exit code: 0. Confirmed latest head SHA `e54ecdfa083de5e92bbdbda6773eac815709357c`.
- `curl -s 'https://api.github.com/repos/apache/shardingsphere/pulls/38795/files?per_page=100'` compared with local triple-dot diff
Exit code: 0. GitHub file list matched local file list.
- `./mvnw -pl test/e2e/mcp -am -DskipITs -Dspotless.skip=true -Dtest=LLME2EConfigurationTest,LLME2EArtifactWriterTest,LLMChatModelClientTest,LLMRuntimeSupportTest,MySQLRuntimeTestSupportTest -Dsurefire.failIfNoSpecifiedTests=false test`
Exit code: 0. MCP touched tests passed: 23 tests, 0 failures, 0 errors.
- `sh test/e2e/mcp/src/test/resources/docker/llm-runtime/build-local.sh --print`
Exit code: 0. Confirmed properties expand to the expected Qwen model metadata.
- `docker buildx build --check ... test/e2e/mcp/src/test/resources/docker/llm-runtime`
Exit code: 0. Dockerfile check completed with no warnings.
- `sh test/e2e/mcp/src/test/resources/docker/llm-runtime/build-local.sh`
Exit code: 0. Local image build succeeded using the cached model layer and wrote image `sha256:7598f889f44afa0da63c782890b7a8875817e7abad79ce80f1abc2febbdad228`.
- `docker image inspect apache/shardingsphere-mcp-llm-runtime:local --format '{{json .Config.Labels}}'`
Exit code: 0. Labels include `ggml-org/Qwen3-1.7B-GGUF:Q4_K_M`, `Q4_K_M`, revision `daeb8e2d528a760970442092f6bf1e55c3b659eb`, and SHA-256 `d2387ca2dbfee2ffabce7120d3770dadca0b293052bc2f0e138fdc940d9bc7b5`.
### Residual Risk
This review did not rely on CI status. Local verification covered the changed configuration path, selected MCP unit tests, Dockerfile syntax checks, and an actual cached local runtime image build. I do not see remaining in-scope risk that should block merge.
--
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