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
fix get gpu metric error & typo error of 'gpu exporter' #118
fix get gpu metric error & typo error of 'gpu exporter' #118
Conversation
Hi @soolaugust. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/assign @cheyang |
log.Errorf("gpu metric is not exist in prometheus for query %s", query) | ||
return gpuMetric, fmt.Errorf("gpu metric is not exist in prometheusfor query %s", query) | ||
log.Debugf("gpu metric is not exist in prometheus for query %s", query) | ||
return gpuMetric, nil |
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.
May I ask the reason to not raise up the error ?
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.
Actually with local test, when job is not running(pending), gpu metric is not detected. using arena top job
command at this time will raise a error. This is not user friendly, so I think it should not be a error message.
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.
Okay, Thank you very much for your explanation, testing and contribution! Look forward to your next PR.
/ok-to-test |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheyang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This change is