patch: adapt attribution get error #274
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves error handling and logging for the vLLM async server by adding defensive error status code handling and error logging capabilities.
- Added error logging when vLLM chat completion errors occur
- Implemented defensive status code extraction with a fallback to 500
- Removed unused import
ChatCompletionResponsePatched
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if isinstance(generator, ErrorResponse): | ||
| return JSONResponse(content=generator.model_dump(), status_code=generator.code) | ||
| status_code = getattr(generator, "code", None) or 500 |
There was a problem hiding this comment.
The fallback to 500 may mask situations where generator.code is 0 or another falsy value. Consider explicitly checking for None: status_code = getattr(generator, 'code', None); status_code = status_code if status_code is not None else 500 or using status_code = getattr(generator, 'code', 500).
| status_code = getattr(generator, "code", None) or 500 | |
| status_code = getattr(generator, "code", None) | |
| status_code = status_code if status_code is not None else 500 |
There was a problem hiding this comment.
Do you think this comment makes sense?
There was a problem hiding this comment.
Not sure. It may be necessary to check the status code to see if it is used in AgentLightning and which code may be returned by verl.
| return cls | ||
|
|
||
|
|
||
| logger = configure_logger() |
There was a problem hiding this comment.
it should be logger = logging.getLogger(__name__)
2 participants
No description provided.