Remove vLLM x86_64 NVIDIA platform warning from pull compat check#794
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="pkg/distribution/distribution/client.go" line_range="798-800" />
<code_context>
if config.GetFormat() == "" {
log.Warn("Model format field is empty for , unable to verify format compatibility", "model", utils.SanitizeForLog(reference))
- } else if !slices.Contains(GetSupportedFormats(), config.GetFormat()) {
- // Write warning but continue with pull
</code_context>
<issue_to_address>
**nitpick (typo):** The warning message has a grammatical issue and is missing the subject after "for".
The log currently reads `"Model format field is empty for , unable to verify format compatibility"`, which suggests a missing interpolated value after `for`. If no identifier is needed here, consider dropping `for` entirely. Otherwise, explicitly include the model identifier (e.g., the `reference` you’re already logging) to make the message clearer.
```suggestion
if config.GetFormat() == "" {
log.Warn("Model format field is empty; unable to verify format compatibility", "model", utils.SanitizeForLog(reference))
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request removes the logic that checked for and warned about unsupported model formats during the model pulling process. Specifically, it removes the warnUnsupportedFormat constant, the slices import, the platform import, and the associated test cases that verified these warnings. I have no feedback to provide.
ilopezluna
left a comment
There was a problem hiding this comment.
Changes LGTM, but I’m unsure about the rationale for removing this warning. Could you share it? 🙏
|
@ilopezluna it really doesn't make sense, it appears on macOS even though we delivered vllm support there. Also, I want people to report vllm doesn't work on my system and see the ugly vllm-log (just like they would see if they tried vanilla vllm). It gives us more feedback on where people are trying to run this. |
abf2f6a to
75c66f3
Compare
75c66f3 to
1621308
Compare
makes sense, thanks! |
No description provided.