-
Notifications
You must be signed in to change notification settings - Fork 700
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
Create Pytorch Informer/ClientSet #1501
Create Pytorch Informer/ClientSet #1501
Conversation
Hi @migueltol22. 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: migueltol22 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -19,6 +19,7 @@ package versioned | |||
import ( | |||
"fmt" | |||
|
|||
kubeflowv1 "github.com/kubeflow/training-operator/pkg/client/clientset/versioned/typed/pytorch/v1" |
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.
@terrytangyuan here is an example of the conflicting import mentioned in this comment
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.
Could you please also add XGBoostJob, MXNetJob, and MPIJob?
Added all 3. Although the imports aliases conflict with each other. @gaocegege do you happen to know what's needed to resolve this or is the expectation to manually resolve them? |
kubeflowv1 "github.com/kubeflow/training-operator/pkg/client/clientset/versioned/typed/mpi/v1" | ||
kubeflowv1 "github.com/kubeflow/training-operator/pkg/client/clientset/versioned/typed/mxnet/v1" | ||
kubeflowv1 "github.com/kubeflow/training-operator/pkg/client/clientset/versioned/typed/pytorch/v1" |
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.
@gaocegege This is the issue I mentioned with the imports. I'm not sure if I missed something or I should manually fix them?
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.
Oh, get it.
I'm extremely curious, will this pr continue to update? If possible, I'd like to help : ) |
I also think the problem is this groupName. See mpi group name, mxnet group name, pytorch group name, tensorflow group name and xgb group name: they are all
@gaocegege @zw0610 what do you guys think? 🤔 If so, tons of work will need to adapt it I guess |
The second option looks more authentic to me. |
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.
/ok-to-test
We can close this PR as #1610 is merged |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
/close |
@tenzen-y: Closed this PR. 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. |
What this PR does / why we need it:
This PR runs code generator to create a pytorch client and informer.
Which issue(s) this PR fixes Fixes #1499
Checklist: