-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Disable issue creation and update README #7618
Conversation
Signed-off-by: Andrey Velichkevich <[email protected]>
b61f02b
to
f0136a5
Compare
@andreyvelich just want to say, we MUST NOT merge this until we have migrated the code to the |
Can you explain what is your major concern to not merge this PR @thesuperzapper ? |
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.
+1
cc @kubeflow/wg-data-leads for feedback |
@thesuperzapper you can still continue with PRs and development, this is just for new issues. |
I recently cleaned up 15 issues there in #7549 (comment) , but they keep coming (again 15+ new ones) |
@andreyvelich I recommend do add this stalebot PR to kubeflow/kubeflow as well kubeflow/manifests#2760 . |
@juliusvonkohout Do you think we still need stalebot if we can just go over 197 issues and close the idle issues ? |
README.md
Outdated
* [Training](https://github.com/kubeflow/community/tree/master/wg-training) | ||
- Use [`kubeflow/manifests`](https://github.com/kubeflow/manifests) for Kubeflow Manifests. | ||
- Use [`kubeflow/dashboard`](https://github.com/kubeflow/dashboard) for Kubeflow Profile Controller, | ||
Central Dashboard, CRUD Web Apps, PVC Viewer, PodDefault, and Access Management components. |
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.
This is incorrect, only the following apps (see #7549) are going to be in kubeflow/dashboard
:
Central Dashboard
Profile Controller
KFAM
PodDefaults
The CRUD Web Apps and PVC Viewer will live in kubeflow/notebooks
.
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.
@thesuperzapper @kimwnasptd Why are we planning to maintain CRUD Web Apps in kubeflow/notebooks
since it is used in almost all Kubeflow UIs (Katib, Models WebApp, Notebooks, PVC Viewer) ?
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.
@andreyvelich we discussed it a lot in #7549, but the gist of it is that the biggest usage of the CRUD code is in Kubeflow Notebooks. Also, stuff like the PVC Viewer is seen as "part of Kubeflow notebooks" so it will also be in kubeflow/notebooks
.
Really all that matters is that the common CRUD code stays maintained, and its easiest for the Notebooks WG to do that in the kubeflow/notebooks
repo.
Finally, the kubeflow/dashboard
repo is for the things which make the dashboard work, it has no dependency on the CRUD code, so makes no sense for the CRUD code to live there.
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.
@thesuperzapper How did you conclude that the biggest usage of CRUD is in kubeflow/notebooks
?
The common web apps handle an ability to communicate between dashboard and other web apps: https://github.com/kubeflow/kubeflow/tree/2898d0f61edf193209c82cf673608cf97c247c3a/components/crud-web-apps/common#frontend.
When @kubeflow/arrikto initiated this component, the intention was to provide unify styles across all Kubeflow web apps. So users will get similar experience by using various Kubeflow UIs.
If we don't maintain it in the dashboard
repository, how can we make sure that other UIs are functional and problems can be addressed ?
For example, Katib UI CI is broken since April 19th by migrating to the latest Kubeflow commit version: kubeflow/katib#2313
cc @orfeas-k
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.
It's not just the Katib web app, let's not forget about KServe web app, which makes extensive use of this common code.
@ederign this conversation is very relevant to our recent discussion. How and where we maintain this common code will influence the success of these various web apps. I think we should align the conversation and if you want to take the lead on defining broad best practices and recommendation, this may be a good place to start
We need to wait (for turning off issues on Since I don't want to block other updates proposed in this PR (like adding links to the components and updating the readme), let's just keep the issues for Notebooks and Dashboard pointed at @kimwnasptd as the other Notebook lead (since we maintain these components) can you please share your opinion. Also, there are lots of people who use the And |
We can create
Why it is not the best place ?
@thesuperzapper Can you explain why do you need it if that will just slow down the migration for us ? |
Sure, this is good idea. I feel that opening GitHub Discussions should be a separate discussion, since first of all we should solve the problem when users ask questions somewhere, and they don't get any response from the Kubeflow community. |
It is about adding a single file and enforces it. If not for kubeflow/kubeflow then at least for the new repositories. |
Yes i think kubeflow/kubeflow should only be a landing page with with redirect information without discussions or issues. |
Yes, for the new repos we should enable it, like we did for other repos. |
Signed-off-by: Andrey Velichkevich <[email protected]>
/lgtm |
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.
/lgtm
/approve
/hold
@terrytangyuan there are still unresolved discussions like #7618 (comment). Separately, as I said in #7618 (comment) we cant disable issue creation on Also |
@thesuperzapper We already made a decision to move this forward to simplify the migration. Otherwise, users will still use
Issues in
What is the main problems for users to use that repo if eventually we will migrate the Notebooks 1.0 code ? |
Please use the following GitHub repositories to open issues and pull requests for | ||
[the different Kubeflow components](https://www.kubeflow.org/docs/started/introduction/#what-are-standalone-kubeflow-components): | ||
|
||
| Component | Source Code | |
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.
Should dashboard be added to this list? The repo was just created.
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.
@james-jwu I added it below, please check: https://github.com/kubeflow/kubeflow/blob/ddbe0ec5b4c42864374cdf2f04164e55b60d2a42/README.md#:~:text=Use%20the%20kubeflow/dashboard%20GitHub%20repository%20for%20the%20Kubeflow%20Profile%20Controller%2C%20Central%20Dashboard%2C%20CRUD%20Web%20Apps%2C%20PVC%20Viewer%2C%20PodDefault%2C%20and%20Access%20Management%20components.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: james-jwu, terrytangyuan 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 |
Since most of our community members agreed with these change, we can just merge this PR and start using new repos for users' issues:
We can always improve the README message in the following PRs. /hold cancel |
Related: #7549
In accordance of @kubeflow/kubeflow-steering-committee suggestion, I disabled issue creation and updated README.
That should help us with migration, since new users won't use
kubeflow/kubeflow
to open issues. After migration of existing issues, we can disable issue creation.I am happy to improve language after we finalize statement in this PR: kubeflow/website#3755.
/assign @kubeflow/kubeflow-steering-committee @kubeflow/wg-manifests-leads @kubeflow/wg-training-leads @kubeflow/wg-pipeline-leads @kubeflow/wg-training-leads @kubeflow/wg-automl-leads @kubeflow/wg-notebooks-leads
/hold for review