Add SGLang MACA runtime preload path handling - #2
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request cleans up debug log messages by removing unicode checkmarks and crosses, and introduces support for the MACA architecture (via the MACA_PATH environment variable) in CUDA home detection and runtime library preloading. It also adds unit tests to verify these new MACA-specific paths. The review feedback suggests converting cuda_home to a Path object internally within _candidate_runtime_library_dirs for robustness, and avoiding the use of clear=True in patch.dict during unit tests to prevent clearing essential environment variables like PATH and PYTHONPATH.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _candidate_runtime_library_dirs(cuda_home: Path): | ||
| maca_home = os.environ.get("MACA_PATH") | ||
| candidate_dirs = [ | ||
| cuda_home / "lib", | ||
| cuda_home / "lib64", | ||
| Path("/usr/lib/x86_64-linux-gnu"), | ||
| Path("/usr/lib/aarch64-linux-gnu"), | ||
| Path("/usr/lib64"), | ||
| Path("/usr/lib"), | ||
| ] |
There was a problem hiding this comment.
To make the function more robust and adhere to defensive programming practices, convert cuda_home to a Path object internally. This prevents potential runtime TypeErrors if a string is passed instead of a Path object.
def _candidate_runtime_library_dirs(cuda_home: Path):
maca_home = os.environ.get("MACA_PATH")
cuda_home = Path(cuda_home)
candidate_dirs = [
cuda_home / "lib",
cuda_home / "lib64",
]| def test_find_cuda_home_uses_maca_cu_bridge(self): | ||
| with TemporaryDirectory() as tmp_dir: | ||
| maca_path = Path(tmp_dir) / "maca" | ||
| with patch.dict(os.environ, {"MACA_PATH": str(maca_path)}, clear=True): |
There was a problem hiding this comment.
Using clear=True in patch.dict clears the entire environment, including essential variables like PATH and PYTHONPATH. This can cause unexpected side effects or failures in some test environments. A safer approach is to copy os.environ, explicitly remove CUDA_HOME and CUDA_PATH, set MACA_PATH, and then patch with clear=True using this complete dictionary.
| with patch.dict(os.environ, {"MACA_PATH": str(maca_path)}, clear=True): | |
| test_env = os.environ.copy() | |
| test_env.pop("CUDA_HOME", None) | |
| test_env.pop("CUDA_PATH", None) | |
| test_env["MACA_PATH"] = str(maca_path) | |
| with patch.dict(os.environ, test_env, clear=True): |
| with TemporaryDirectory() as tmp_dir: | ||
| maca_path = Path(tmp_dir) / "maca" | ||
| cuda_home = maca_path / "tools" / "cu-bridge" | ||
| with patch.dict(os.environ, {"MACA_PATH": str(maca_path)}, clear=True): |
There was a problem hiding this comment.
Using clear=True in patch.dict clears the entire environment, including essential variables like PATH and PYTHONPATH. This can cause unexpected side effects or failures in some test environments. A safer approach is to copy os.environ, explicitly remove CUDA_HOME and CUDA_PATH, set MACA_PATH, and then patch with clear=True using this complete dictionary.
| with patch.dict(os.environ, {"MACA_PATH": str(maca_path)}, clear=True): | |
| test_env = os.environ.copy() | |
| test_env.pop("CUDA_HOME", None) | |
| test_env.pop("CUDA_PATH", None) | |
| test_env["MACA_PATH"] = str(maca_path) | |
| with patch.dict(os.environ, test_env, clear=True): |
?? API ?? 2 ??????
ghangz
changed the title
- Support MACA runtime preload paths - harden MACA runtime preload helpers - Preserve environment in MACA load utils tests - Isolate MACA load utils environment tests
1 participant
Summary
Validation
Review notes