Remove JupyterHub dependencies #1211

DaoDaoNoCode · 2023-05-09T07:45:44Z

Closes #534

Description

Remove dependencies related to JupyterHub, most of the YAML docs updates will be addressed in #1636

How Has This Been Tested?

Make sure you installs the apps and docs in updated manifests/apps/jupyter and make sure your cluster has removed jupyterhub OdhApplication CR
Set enabled to true in notebookController field in the dashboard config CR
Make sure everything works as normal
Set enabled to false in notebookController field in the dashboard config CR, and wait for 2 mins
Make sure you cannot see any docs, quickstart on the Resources page, Make sure you cannot see the Jupyter app card on the Enabled and Explore page
Make sure you cannot access the notebook controller via URL
Make sure the Launch Jupyter button is hidden on the data science projects table
Make sure the toleration settings section is hidden in the cluster settings

Test Impact

N/A

Request review criteria:

Self checklist (all need to be checked):

The developer has manually tested the changes and verified that the changes work
Commits have been squashed into descriptive, self-contained units of work (e.g. 'WIP' and 'Implements feedback' style messages have been removed)
Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has added tests or explained why testing cannot be added (unit tests & storybook for related changes)

If you have UI changes:

Included any necessary screenshots or gifs if it was a UI change.
Included tags to the UX team if it was a UI/UX change.

After the PR is posted & before it merges:

The developer has tested their solution on a cluster by using the image produced by the PR to main

DaoDaoNoCode · 2023-05-09T07:46:32Z

/hold Might want to wait until https://github.com/opendatahub-io/odh-manifests/issues/805 is solved.

manifests/apps/jupyter/create-jupyter-notebook-quickstart.yaml

lucferbux · 2023-08-03T10:40:12Z

/hold

@DaoDaoNoCode are we gonna wait until we find the right text for the quickstart right?

DaoDaoNoCode · 2023-08-03T12:24:59Z

@lucferbux Yes, I will hold this PR until next release.

lucferbux · 2023-08-28T10:32:22Z

@DaoDaoNoCode I think we can unhold this now, but it would need a rebase

lucferbux · 2023-08-28T10:32:27Z

/unhold

DaoDaoNoCode · 2023-08-28T13:08:14Z

@lucferbux I think I still need to make some changes to the PR, looks like they don't want to remove the enable flag in notebookController config because I saw some customers are still using it to hide the Jupyter tile card. I will update this PR later.

manifests/crd/odhdashboardconfigs.opendatahub.io.crd.yaml

docs/dashboard_config.md

DaoDaoNoCode · 2023-09-07T17:53:45Z

@lucferbux I have updated the PR and it's ready for review!

lucferbux

Check the logic, maybe you can install more resources to make it more explicit, I got all the resources we have for rhods in odh-manifest and isntalled all of them in opendatahub.

backend/src/routes/api/cluster-settings/clusterSettingsUtils.ts

lucferbux · 2023-09-14T10:57:11Z

backend/src/routes/api/components/list.ts

@@ -8,7 +8,7 @@ export const listComponents = async (
  request: FastifyRequest,
 ): Promise<OdhApplication[]> => {
  const applications = getApplications().filter(
-    (component) => component.metadata.name !== (checkJupyterEnabled() ? 'jupyterhub' : 'jupyter'),
+    (component) => !checkJupyterEnabled() && component.metadata.name !== 'jupyter',


Same with this check, we might want to filter jupyter IF notebook controller is not enable, this will make all the applications dissapear if you have notebook controller enabled

backend/src/routes/api/docs/list.ts

backend/src/routes/api/quickstarts/list.ts

backend/src/utils/resourceUtils.ts

frontend/src/app/AppRoutes.tsx

lucferbux · 2023-09-20T15:46:25Z

Ok, removing checkJupyterEnabled migth be tricker than I thought cause it might need a migration upgrade path. In that case, we might wanna either create a new ticket or get that with #1108

lucferbux

Just a couple of nits

frontend/src/app/AppRoutes.tsx

lucferbux · 2023-09-21T12:19:22Z

frontend/src/app/AppRoutes.tsx

@@ -59,7 +61,10 @@ const AppRoutes: React.FC = () => {

        <Route path="/projects/*" element={<ProjectViewRoutes />} />

-        <Route path="/notebookController/*" element={<NotebookController />} />
+        {isJupyterEnabled && (
+          <Route path="/notebookController/*" element={<NotebookController />} />


If we are hiding away /notebookController, why aren't we hiding the workbenches section in projects?

The /notebookController is only the Jupyter tile on the Enabled page, it's nothing to do with the project workbenches, they are different features.

mmmm so we don't have a way to hide workbenches right now in projects?

I don't think we have a way to do that, we can only disable projects, but not the workbench section only.

backend/src/routes/api/quickstarts/list.ts

lucferbux

/lgtm

openshift-ci · 2023-09-21T14:26:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lucferbux

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:

~~OWNERS~~ [lucferbux]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

shalberd · 2023-11-09T09:29:56Z

nice work, thank you all.

@DaoDaoNoCode especially so the Jupyterhub-Tile disappears (even the Applications menu if no applications are in that section?) when notebookController.enabled is set to false.

@lucferbux Out of curiosity: Does the applications menu disappear in the GUI when there are no Applications / OdhApplications present at all?

openshift-ci bot requested review from alexcreasy and lucferbux May 9, 2023 07:45

openshift-ci bot added the do-not-merge/hold This PR is hold for some reason label May 9, 2023

DaoDaoNoCode commented May 9, 2023

View reviewed changes

manifests/apps/jupyter/create-jupyter-notebook-quickstart.yaml Outdated Show resolved Hide resolved

DaoDaoNoCode changed the title ~~[DO NOT MERGE]: Remove JupyterHub dependencies~~ Remove JupyterHub dependencies May 15, 2023

lucferbux added do-not-merge/hold This PR is hold for some reason and removed do-not-merge/hold This PR is hold for some reason labels Jun 5, 2023

openshift-merge-robot added the needs-rebase PR needs to be rebased label Jun 16, 2023

shalberd reviewed Jul 31, 2023

View reviewed changes

manifests/apps/jupyter/create-jupyter-notebook-quickstart.yaml Outdated Show resolved Hide resolved

openshift-ci bot removed the do-not-merge/hold This PR is hold for some reason label Aug 28, 2023

andrewballantyne reviewed Aug 28, 2023

View reviewed changes

manifests/crd/odhdashboardconfigs.opendatahub.io.crd.yaml Outdated Show resolved Hide resolved

shalberd mentioned this pull request Aug 28, 2023

ds-10105-update-gs-doc-links-in-quickstarts #1636

Closed

7 tasks

shalberd reviewed Aug 28, 2023

View reviewed changes

docs/dashboard_config.md Outdated Show resolved Hide resolved

DaoDaoNoCode force-pushed the upstream-issue-534 branch from 779db86 to 77bc703 Compare August 29, 2023 15:54

openshift-merge-robot removed the needs-rebase PR needs to be rebased label Aug 29, 2023

DaoDaoNoCode changed the base branch from main to f/notebook-controller August 29, 2023 20:13

DaoDaoNoCode force-pushed the upstream-issue-534 branch from 77bc703 to df9833b Compare August 30, 2023 16:16

andrewballantyne added the pr/no-tests-allowed Added by an official approver - this PR is allowed no tests. Omitted, a test must accompany the PR label Sep 7, 2023

DaoDaoNoCode force-pushed the upstream-issue-534 branch 2 times, most recently from 2b7334d to de57c10 Compare September 7, 2023 17:53

lucferbux requested changes Sep 14, 2023

View reviewed changes

openshift-ci bot assigned lucferbux Sep 14, 2023

DaoDaoNoCode mentioned this pull request Sep 19, 2023

[Bug]: Admin UI Idle Culler settings is enabled to 0 min when culler_timeout does not exist in the jupyterhub-cfg configMap #278

Closed

1 task

remove JupyterHub dependencies

1fd4d10

DaoDaoNoCode force-pushed the upstream-issue-534 branch from de57c10 to 1fd4d10 Compare September 20, 2023 17:34

lucferbux requested changes Sep 21, 2023

View reviewed changes

lucferbux approved these changes Sep 21, 2023

View reviewed changes

openshift-ci bot added the lgtm label Sep 21, 2023

openshift-ci bot added the approved label Sep 21, 2023

openshift-merge-robot merged commit 10ad0e6 into opendatahub-io:f/notebook-controller Sep 21, 2023
3 checks passed

shalberd mentioned this pull request Nov 9, 2023

Update Jupyter Quickstart #1725

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove JupyterHub dependencies #1211

Remove JupyterHub dependencies #1211

DaoDaoNoCode commented May 9, 2023 •

edited

Loading

DaoDaoNoCode commented May 9, 2023

lucferbux commented Aug 3, 2023

DaoDaoNoCode commented Aug 3, 2023

lucferbux commented Aug 28, 2023

lucferbux commented Aug 28, 2023

DaoDaoNoCode commented Aug 28, 2023

DaoDaoNoCode commented Sep 7, 2023

lucferbux left a comment

lucferbux Sep 14, 2023

lucferbux commented Sep 20, 2023

lucferbux left a comment

lucferbux Sep 21, 2023

DaoDaoNoCode Sep 21, 2023

lucferbux Sep 21, 2023

DaoDaoNoCode Sep 21, 2023

lucferbux left a comment

openshift-ci bot commented Sep 21, 2023

shalberd commented Nov 9, 2023

Remove JupyterHub dependencies #1211

Remove JupyterHub dependencies #1211

Conversation

DaoDaoNoCode commented May 9, 2023 • edited Loading

Description

How Has This Been Tested?

Test Impact

Request review criteria:

DaoDaoNoCode commented May 9, 2023

lucferbux commented Aug 3, 2023

DaoDaoNoCode commented Aug 3, 2023

lucferbux commented Aug 28, 2023

lucferbux commented Aug 28, 2023

DaoDaoNoCode commented Aug 28, 2023

DaoDaoNoCode commented Sep 7, 2023

lucferbux left a comment

Choose a reason for hiding this comment

lucferbux Sep 14, 2023

Choose a reason for hiding this comment

lucferbux commented Sep 20, 2023

lucferbux left a comment

Choose a reason for hiding this comment

lucferbux Sep 21, 2023

Choose a reason for hiding this comment

DaoDaoNoCode Sep 21, 2023

Choose a reason for hiding this comment

lucferbux Sep 21, 2023

Choose a reason for hiding this comment

DaoDaoNoCode Sep 21, 2023

Choose a reason for hiding this comment

lucferbux left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Sep 21, 2023

shalberd commented Nov 9, 2023

DaoDaoNoCode commented May 9, 2023 •

edited

Loading