Add vLLM-metax vllm metax model resource plan#310
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new Python script, tools/model_resource_plan.py, which generates resource-aware validation plans for MACA containers. The feedback highlights an improvement opportunity in the self-test logic, recommending the replacement of the assert statement with an explicit conditional check and a RuntimeError to ensure the validation is not bypassed when Python is executed with optimization flags.
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 self_test() -> None: | ||
| rows = plan(CASES[:1]) | ||
| assert rows and rows[0]["case"] == CASES[0] |
There was a problem hiding this comment.
Using assert statements for runtime validation or self-tests is discouraged because assertions can be globally disabled in Python when run with optimization flags (e.g., python -O). If assertions are disabled, this validation check will be completely bypassed. It is safer and more robust to use an explicit conditional check and raise an appropriate exception (such as RuntimeError).
| assert rows and rows[0]["case"] == CASES[0] | |
| if not (rows and rows[0]["case"] == CASES[0]): | |
| raise RuntimeError("Self-test validation failed") |
Summary
Validation
Review notes