-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
api: log unsupported models #251
Conversation
Thanks for your PR,
To skip the vendors CIs use one of:
|
@@ -103,6 +103,7 @@ func IsSupportedModel(vendorId, deviceId string) bool { | |||
return true | |||
} | |||
} | |||
log.Info("IsSupportedModel():", "Unsupported model:", "vendorId:", vendorId, "deviceId:", deviceId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, it's better to use log.Warning here and below to get more attention into these log messages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree but if you look at this file, all "errors" are reported via log.Info
. The imported logger can do Info or Error. I could create another logger with glog and call glog.Warning
, but... Up to you at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@e0ne any thoughts?
Thanks for your PR,
To skip the vendors CIs use one of:
|
1 similar comment
Thanks for your PR,
To skip the vendors CIs use one of:
|
It can be really useful to provide logs when a device (PF or VF) is not supported and give the vendorId + deviceId; so the deployer can check what device is being selected and compare with the ConfigMap which describes the supported nics.
Thanks for your PR,
To skip the vendors CIs use one of:
|
/lgtm |
@adrianchiris could I get this one approved please? |
/test-all |
/lgtm |
/cc @bn222 |
merging as i see @pliurh LGTM'd it |
It can be really useful to provide logs when a device (PF or VF) is not
supported and give the vendorId + deviceId; so the deployer can check
what device is being selected and compare with the ConfigMap which
describes the supported nics.