Skip to content
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

jwa(front): Create distinct notebook details page #6769

Conversation

orfeas-k
Copy link
Contributor

@orfeas-k orfeas-k commented Nov 24, 2022

This PR is part of the effort to add details pages for objects in our WAs. It creates a new notebook details page where a user can navigate and see key information about each notebook. More specifically, this tab contains an overview of the .status of the object plus a selection of the most important fields from the spec. These are Volumes, Applied Configurations from PodDefaults, Notebook type, ENV Vars, Minimum/Maximum CPU/RAM, Docker image, Conditions. We will also introduce soon LOGS, EVENTS and YAML tab.

Route

Regarding the link that this page will be served on, we need to be able to extract from the link information about the notebook's name we 're currently viewing and the current namespace. Thus, we ended up with notebook/details/namespace/notebook-name?ns=namespace .

Notebook's Conditions

Our plan here was to add a Conditions table inside the OVERVIEW tab in order to show the .status.conditions of the notebook.

Problem
However, we found out that the notebook’s .status.conditions are fundamentally broken. Right now the Notebook Controller is blindly appending Pod conditions on the Notebook's conditions https://github.com/kubeflow/kubeflow/blob/master/components/notebook-controller/controllers/notebook_controller.go#L247.
The conditions are expected to provide a summary of the state of an object for a specific point in time, not use the list to combine the conditions to get to the final state. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties

Workaround
Since currently, there is no point in exposing the notebook .status.conditions, we decided, as a temporary workaround, to show the Notebook’s underlying Pod conditions and to be explicit about which conditions the table refers to.

Sidenote

For clickable items - links, we followed the style and what we do in general in KFP (e.g. Output Artifacts)

Screenshots

image
image
image

cc @kimwnasptd

Copy link
Member

@kimwnasptd kimwnasptd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @orfeas-k!

Had some proposals mostly around using Notebook's Conditions and not the Pod ones. We've ensured the Controller is setting the Notebook Conditions properly from the previous release cc @apo-ger

Comment on lines 9 to 12
if label_selector is None:
return v1_core.list_namespaced_pod(namespace)
else:
return v1_core.list_namespaced_pod(namespace = namespace, label_selector = label_selector)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this if/else case and just do:

return v1_core.list_namespaced_pod(namespace = namespace, label_selector=label_selector)

in both cases. The default value is None so we should be good, since we use None in the function definition

<ng-container *ngIf="!podRequestCompleted">
<lib-heading-row
class="heading-row"
heading="Pod Conditions"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do Conditions here. We should be able to use Notebook conditions

<ng-container *ngIf="podRequestCompleted && pod">
<lib-conditions-table
*ngIf="pod?.status"
[conditions]="pod.status?.conditions"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the conditions from the Notebook status.

We've updated the Notebook Controller to properly set the conditions in #6628

@kimwnasptd
Copy link
Member

Also, could you do a rebase on top of the latest changes? We've pushed some commits that updated the versions in packge.json

In this commit:

 - Add logic in the backend to fetch a single notebook and its
   underlying pod.
 - Make list_pods function in Kubeflow common code to accept a
   label_selector parameter and use it to filter out results when
   available.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
In this commit:

 - Create OVERVIEW tab to show key information about the notebook
   and its underlying pod.
 - Create Content List Item component in Kubeflow commonn library to
   be able to encapsulate any content/component in the form of a
   list item.
 - Create Variables Group component in Kubeflow common library to
   show groups of variables in the form of chips.
 - Small UI tweak in Details list Item component from Kubeflow
   common library in order to stop chips from overlapping with each
   other.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
@orfeas-k orfeas-k force-pushed the feature-orfeas-k-jwa-notebook-details-page branch from f36bc87 to f3265ba Compare November 29, 2022 08:04
@orfeas-k
Copy link
Contributor Author

@kimwnasptd Had to force push since I rebased to resolve the conflicts but the fixes for the comments above are separate commits on their own.

@kimwnasptd
Copy link
Member

Changes look good, thanks for driving this @orfeas-k!

/lgtm
/approve

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kimwnasptd

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 6b899b2 into kubeflow:master Nov 29, 2022
@kimwnasptd kimwnasptd deleted the feature-orfeas-k-jwa-notebook-details-page branch November 29, 2022 08:20
maroroman pushed a commit to maroroman/kubeflow that referenced this pull request Feb 7, 2023
* jwa(back): Get a single notebook and its pod

In this commit:

 - Add logic in the backend to fetch a single notebook and its
   underlying pod.
 - Make list_pods function in Kubeflow common code to accept a
   label_selector parameter and use it to filter out results when
   available.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Create Notebook details page with OVERVIEW tab

In this commit:

 - Create OVERVIEW tab to show key information about the notebook
   and its underlying pod.
 - Create Content List Item component in Kubeflow commonn library to
   be able to encapsulate any content/component in the form of a
   list item.
 - Create Variables Group component in Kubeflow common library to
   show groups of variables in the form of chips.
 - Small UI tweak in Details list Item component from Kubeflow
   common library in order to stop chips from overlapping with each
   other.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* Fix linting errors

* Include Kubeflow common library's new package-lock.json

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Replace pod conditions with notebook status conditions

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* web-apps(back): Backend fetch pods fix

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
Adembc pushed a commit to Adembc/notebooks that referenced this pull request Jun 22, 2024
…6769)

* jwa(back): Get a single notebook and its pod

In this commit:

 - Add logic in the backend to fetch a single notebook and its
   underlying pod.
 - Make list_pods function in Kubeflow common code to accept a
   label_selector parameter and use it to filter out results when
   available.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Create Notebook details page with OVERVIEW tab

In this commit:

 - Create OVERVIEW tab to show key information about the notebook
   and its underlying pod.
 - Create Content List Item component in Kubeflow commonn library to
   be able to encapsulate any content/component in the form of a
   list item.
 - Create Variables Group component in Kubeflow common library to
   show groups of variables in the form of chips.
 - Small UI tweak in Details list Item component from Kubeflow
   common library in order to stop chips from overlapping with each
   other.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* Fix linting errors

* Include Kubeflow common library's new package-lock.json

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Replace pod conditions with notebook status conditions

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* web-apps(back): Backend fetch pods fix

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
Adembc pushed a commit to Adembc/notebooks that referenced this pull request Jun 22, 2024
…6769)

* jwa(back): Get a single notebook and its pod

In this commit:

 - Add logic in the backend to fetch a single notebook and its
   underlying pod.
 - Make list_pods function in Kubeflow common code to accept a
   label_selector parameter and use it to filter out results when
   available.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Create Notebook details page with OVERVIEW tab

In this commit:

 - Create OVERVIEW tab to show key information about the notebook
   and its underlying pod.
 - Create Content List Item component in Kubeflow commonn library to
   be able to encapsulate any content/component in the form of a
   list item.
 - Create Variables Group component in Kubeflow common library to
   show groups of variables in the form of chips.
 - Small UI tweak in Details list Item component from Kubeflow
   common library in order to stop chips from overlapping with each
   other.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* Fix linting errors

* Include Kubeflow common library's new package-lock.json

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Replace pod conditions with notebook status conditions

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* web-apps(back): Backend fetch pods fix

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
Adembc pushed a commit to Adembc/notebooks that referenced this pull request Jun 22, 2024
…6769)

* jwa(back): Get a single notebook and its pod

In this commit:

 - Add logic in the backend to fetch a single notebook and its
   underlying pod.
 - Make list_pods function in Kubeflow common code to accept a
   label_selector parameter and use it to filter out results when
   available.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Create Notebook details page with OVERVIEW tab

In this commit:

 - Create OVERVIEW tab to show key information about the notebook
   and its underlying pod.
 - Create Content List Item component in Kubeflow commonn library to
   be able to encapsulate any content/component in the form of a
   list item.
 - Create Variables Group component in Kubeflow common library to
   show groups of variables in the form of chips.
 - Small UI tweak in Details list Item component from Kubeflow
   common library in order to stop chips from overlapping with each
   other.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* Fix linting errors

* Include Kubeflow common library's new package-lock.json

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Replace pod conditions with notebook status conditions

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* web-apps(back): Backend fetch pods fix

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
Adembc pushed a commit to Adembc/notebooks that referenced this pull request Jun 23, 2024
…6769)

* jwa(back): Get a single notebook and its pod

In this commit:

 - Add logic in the backend to fetch a single notebook and its
   underlying pod.
 - Make list_pods function in Kubeflow common code to accept a
   label_selector parameter and use it to filter out results when
   available.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Create Notebook details page with OVERVIEW tab

In this commit:

 - Create OVERVIEW tab to show key information about the notebook
   and its underlying pod.
 - Create Content List Item component in Kubeflow commonn library to
   be able to encapsulate any content/component in the form of a
   list item.
 - Create Variables Group component in Kubeflow common library to
   show groups of variables in the form of chips.
 - Small UI tweak in Details list Item component from Kubeflow
   common library in order to stop chips from overlapping with each
   other.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* Fix linting errors

* Include Kubeflow common library's new package-lock.json

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Replace pod conditions with notebook status conditions

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* web-apps(back): Backend fetch pods fix

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
Adembc pushed a commit to Adembc/notebooks that referenced this pull request Jun 23, 2024
…6769)

* jwa(back): Get a single notebook and its pod

In this commit:

 - Add logic in the backend to fetch a single notebook and its
   underlying pod.
 - Make list_pods function in Kubeflow common code to accept a
   label_selector parameter and use it to filter out results when
   available.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Create Notebook details page with OVERVIEW tab

In this commit:

 - Create OVERVIEW tab to show key information about the notebook
   and its underlying pod.
 - Create Content List Item component in Kubeflow commonn library to
   be able to encapsulate any content/component in the form of a
   list item.
 - Create Variables Group component in Kubeflow common library to
   show groups of variables in the form of chips.
 - Small UI tweak in Details list Item component from Kubeflow
   common library in order to stop chips from overlapping with each
   other.

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* Fix linting errors

* Include Kubeflow common library's new package-lock.json

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* jwa(front): Replace pod conditions with notebook status conditions

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

* web-apps(back): Backend fetch pods fix

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants