Conversation
…cate session port Prefer MILES_HOST_IP, then MASTER_ADDR when set, then hostname so multi-node Ray workers can resolve the Miles router address. Remove redundant --session-server-port from sglang args (kept with --use-session-server).
There was a problem hiding this comment.
Code Review
This pull request updates the initialization of miles_host_ip in examples/experimental/swe-agent-v2/run.py to prioritize MILES_HOST_IP, then MASTER_ADDR, and finally the system hostname to prevent unresolvable pod hostnames. A review comment suggests improving the robustness of this logic by consistently stripping whitespace from both environment variables and simplifying the expression.
| miles_host_ip: str = ( | ||
| os.environ.get("MILES_HOST_IP") | ||
| or (os.environ.get("MASTER_ADDR") or "").strip() | ||
| or socket.gethostname() | ||
| ) |
There was a problem hiding this comment.
The logic for determining miles_host_ip can be simplified and made more robust. Currently, it doesn't handle potential whitespace in the MILES_HOST_IP environment variable, and the nested or with strip() for MASTER_ADDR is slightly redundant. Using os.environ.get(key, "").strip() is a cleaner and more idiomatic way to handle missing or whitespace-only environment variables in Python.
| miles_host_ip: str = ( | |
| os.environ.get("MILES_HOST_IP") | |
| or (os.environ.get("MASTER_ADDR") or "").strip() | |
| or socket.gethostname() | |
| ) | |
| miles_host_ip: str = ( | |
| os.environ.get("MILES_HOST_IP", "").strip() | |
| or os.environ.get("MASTER_ADDR", "").strip() | |
| or socket.gethostname() | |
| ) |
Make
MILES_HOST_IPdefault toMASTER_ADDRwhen the latter is set. In a multi-node environment, this ensures that the workers can resolve the correct address of the head node.