Conversation
There was a problem hiding this comment.
Code Review
This pull request updates st_engine/engine/llm_runner.py to distinguish between an engine crash and a request-level failure when the Locust result file is missing. It checks if the run-time limit was reached and if the process exited with a return code of 1 to set the status to FAILED_REQUESTS. Feedback suggests making this check more robust by checking both stdout and stderr for the completion message, guarding against None values to prevent potential TypeErrors, and ensuring process is not None before checking its return code.
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.
| run_time_completed = "--run-time limit reached" in stderr | ||
| locust_request_failure_exit = ( | ||
| process.returncode is not None and process.returncode == 1 | ||
| ) |
There was a problem hiding this comment.
Checking only stderr for the "--run-time limit reached" message can be fragile if Locust logs are redirected to stdout (e.g., via custom log configurations or container environments). Additionally, if stderr or stdout is None (such as in certain testing environments), performing an in check will raise a TypeError.\n\nTo make this check more robust and defensive, we should:\n1. Check both stdout and stderr for the completion message.\n2. Guard against None values for stdout and stderr.\n3. Simplify the process.returncode check while guarding against process being None.
run_time_completed = (\n (stderr and "--run-time limit reached" in stderr) or\n (stdout and "--run-time limit reached" in stdout)\n )\n locust_request_failure_exit = (\n process is not None and process.returncode == 1\n )
fix: Distinguish request failures from engine errors when result.json is missing