Disable some of the kubeflow repos' trigger#19872
Conversation
|
Hi @PatrickXYS. Thanks for your PR. I'm waiting for a kubernetes 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. DetailsInstructions 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. |
|
Would it be helpful to have an |
That would be great if we have, but I don't think trigger support this feature yet, someone can correct me if I'm wrong |
efe96dd to
60a4e12
Compare
60a4e12 to
0b085c4
Compare
| @@ -19,7 +19,26 @@ triggers: | |||
| - containerd/containerd | |||
| join_org_url: "https://github.com/containerd/project/blob/master/MAINTAINERS" | |||
| - repos: | |||
There was a problem hiding this comment.
@PatrickXYS Lets add a comment here explaining why the default is no longer "on" for new repos.
Can we also file an issue to track updating the new repository setup guide
https://github.com/kubeflow/community/blob/master/repository-setup.md
/cc @Bobgy
There was a problem hiding this comment.
I added comments to explain the reason, also, file issue here to keep track kubeflow/community#450
|
@jlewi: GitHub didn't allow me to request PR reviews from the following users: Bobgy. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn 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. |
0b085c4 to
210ed18
Compare
|
/lgtm |
|
@Bobgy: changing LGTM is restricted to collaborators DetailsIn 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. |
|
@PatrickXYS I'm not sure about the change, in fact. Will k8s tide continue to work on those repos? (Tide is responsible for merging PRs) I'd suggest starting with testing one repo and when we get everything working as expected, move to the others. |
What I change here is about Prow plugin - trigger, and Tide is one of the core component in Prow. So I think Tide should still work as before, and k8s-ci-bot will continuously work on those repos. I'm okay to start with one repo if we're not sure |
|
@PatrickXYS: GitHub didn't allow me to request PR reviews from the following users: Bobgy. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn 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. |
|
SGTM @PatrickXYS because there are no people we can ask for quick approval if the change broke some repos. The risk is high, so shall we reduce to just one repo first? |
|
So the plan is to merge #19897 first to test with a single repo and then if that looks good merge this one? |
|
We started experiment on this PR #19897 After it merged, we expect kubeflow/kfctl only exist one bot to respond and I experimented here kubeflow/kfctl#247 (comment), it's working properly. I'll go ahead with other repos in this one single PR |
|
@PatrickXYS: GitHub didn't allow me to request PR reviews from the following users: Bobgy. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn 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. |
210ed18 to
d4eef8f
Compare
|
/lgtm |
|
@Bobgy: changing LGTM is restricted to collaborators DetailsIn 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. |
|
/lgtm |
This is the error message, but that's exactly what we want to do. I assume this is because pytorch-operator still have postsubmit job configured https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubeflow/kubeflow-postsubmits.yaml#L57-L69 Will try to clean up this as well |
|
Can you help on adding / ok-to-test and I'll wait to see if test can pass? @jlewi |
|
/lgtm |
I see, maybe I need to clean up testgird config for pytorch as well |
4f68379 to
f5d6742
Compare
|
/ok-to-test |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cblecker, jlewi, PatrickXYS The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@PatrickXYS: Updated the following 2 configmaps:
DetailsIn 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. |
Given the fact that some of the kubeflow repos exist two bot respond to
/retest,/test all, etc. This is caused by some of the repos migrated to AWS CI, but still rely on upstream prow/tide managed PR.E.g, manifests PR
Thus, disabling some of the kubeflow repos' trigger plugin to avoid two bots multiple response and save developer's confusions
/cc @jlewi @cblecker