fix(examples): separate Minikube host and agent gateway URLs - #633
Open
Bluu (Bluuok) wants to merge 1 commit into
Open
Bluu (Bluuok) wants to merge 1 commit into
Bluu (Bluuok) wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused fix has regression coverage, and the targeted Python 3.12 compatibility check found no blocking issue.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes Calc-X Minikube networking by separating the host controller’s gateway URL from the URL used by agent pods.
Changes:
- Uses
localhostfor the controller andhost.minikube.internalfor agents. - Adds an isolated launcher regression test covering configuration overrides and generated pod URLs.
| File | Description |
|---|---|
| tests/examples/test_calc_x_minikube.py | Tests launcher arguments, configuration, and Job manifest URLs. |
| examples/calc_x/run_minikube.sh | Sets separate host and agent gateway URLs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This branch has not been deployed
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.
The Calc-X Minikube launcher starts its controller on the host, but passes
host.minikube.internalas the controller's gateway URL and leaves the pod-specificagent_urlunset. This conflates the two network locations.Use
localhostfor the host controller and setagl_server.agent_urltohost.minikube.internalfor agent pods, matching the documented controller configuration. Add an isolated test that executes the actual launcher, captures its overrides, composes the real configuration and builds a real Job manifest to verify both pod URLs.Validation in the frozen Linux CPU environment:
The regression failed on unchanged main and passed with the fix. Scoped Pyright passed with 0 errors; launcher content normalized to repository LF line endings passed
bash -n. The test also verifies the key, runner type, TTL and arguments containing spaces, with synchronization for the background controller stub.All Minikube, Ray, HTTP, service, cleanup and training commands are isolated mocks. No real cluster, network or GPU training was exercised. Ready for review based on the author-reported validation above; this maintenance pass did not rerun tests. The CLA check passed. Fork CI is awaiting maintainer approval (action_required, 0 jobs); no CI jobs have run. AI assistance was used; the two-file diff and generated manifest were reviewed.