Upgrade klog to v2 #2470
Upgrade klog to v2 #2470
Conversation
|
@Doris-xm It seems that you didn't sign your commits with |
Doris-xm
force-pushed
the
upgrade-klog-v2
branch
from
c7002a7 to
7bae448
Compare
|
@Electronic-Waste Thanks for your detailed guidance! I've fixed the issue. The PR should now pass the DCO check. |
|
/ok-to-test |
|
@Doris-xm Welcome to the Kubeflow community! 🎉🎉🎉 You can now rerun the failed tests with command |
|
/rerun-all |
There was a problem hiding this comment.
Thanks for your great contribution! Just a few suggestions @Doris-xm .
/assign @kubeflow/wg-automl-leads @helenxie-bit PTAL if you have time:)
cmd/db-manager/v1beta1/main.go
Outdated
| @@ -28,7 +28,7 @@ import ( | |||
| api_pb "github.com/kubeflow/katib/pkg/apis/manager/v1beta1" | |||
| db "github.com/kubeflow/katib/pkg/db/v1beta1" | |||
| "github.com/kubeflow/katib/pkg/db/v1beta1/common" | |||
| "k8s.io/klog" | |||
| klog "k8s.io/klog/v2" | |||
There was a problem hiding this comment.
Maybe we could omit klog since k8s.io/klog/v2 could already be referenced by klog
There was a problem hiding this comment.
Thanks for your suggestion! I've removed the redundant klog alias. The code becomes cleaner and consistent now. Appreciate your review!
Doris-xm
force-pushed
the
upgrade-klog-v2
branch
from
7bae448 to
38429be
Compare
|
/rerun-all |
|
It seems that the CI failed due to This PR basically LGTM. I will leave it for @kubeflow/wg-automl-leads @tariq-hasan @helenxie-bit . PTAL if you have time. Thanks for your great contribution! @Doris-xm /lgtm |
|
@Electronic-Waste: GitHub didn't allow me to assign the following users: tariq-hasan. Note that only kubeflow members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this: 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. |
|
Thanks for the contribution! LGTM |
Signed-off-by: Xinmin Du <[email protected]> Signed-off-by: Xinmin Du <[email protected]>
Doris-xm
force-pushed
the
upgrade-klog-v2
branch
from
38429be to
5efdf10
Compare
Signed-off-by: Du Xinmin <[email protected]>
Signed-off-by: Xinmin Du <[email protected]>
Doris-xm
force-pushed
the
upgrade-klog-v2
branch
from
8489ce2 to
c82c031
Compare
|
/rerun-all |
1 similar comment
|
/rerun-all |
|
Thanks for the reminder! The CI failure is about timeout during exporting cache layers. It seems this happened during a peak testing period on GitHub. I'll rerun the tests later to verify. @tenzen-y |
If you have enough time, could you open a separate PR to ignore the error similar to kubeflow/training-operator#2336? |
Appreciate your suggestion and guidance! I’ll work on it soon. |
|
/rerun-all |
Signed-off-by: Xinmin Du <[email protected]> # Conflicts: # go.mod # go.sum
|
/rerun-all |
1 similar comment
|
/rerun-all |
|
@Doris-xm Could you plz resolve the conflicts? |
Signed-off-by: Xinmin Du <[email protected]> # Conflicts: # go.mod
|
/rerun-all |
|
/hold cancel |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tenzen-y 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 |
What this PR does / why we need it:
This PR upgrades klog to v2 to unify logging dependencies in the project.
Which issue(s) this PR fixes :
Fixes #2469
Checklist: