feat: add user-driven sidebar links via config #119

ca-scribner · 2023-06-27T21:58:21Z

Summary

This PR extends the dynamic sidebar from #118 to allow for user-defined links to be specified via config.

Usage/Testing

To set this config, use an intermediate yaml file:

sidebar.yaml

- icon: assessment
  link: /1
  text: '1'
  type: item
- icon: assessment
  link: /2
  text: '2'
  type: item

juju config kubeflow-dashboard [email protected]

Result:

To edit the config, do:

juju config kubeflow-dashboard additional-sidebar-links > sidebar2.yaml
# edit sidebar2.yaml to add/remove links
juju config kubeflow-dashboard [email protected]

Can also be used in conjunction with #118's application-driven links (see #118)

Todo before final review

add usage instructions to readme.md --> edit: covered in Update README.md with dynamic sidebar usage #126

Merge after #118

Adds: * KubeflowDashboardSidebarRequirer * KubeflowDashboardSidebarProvider * SidebarItem * associated tests

Used as a helper in unit tests

This change delays the computation of the k8s context until it is needed, instead of in __init__. It does not change the outward function of the charm. This change was implemented because in future, the sidebar links will be obtained from a relation and this should not be done in __init__.

previously, the Provider lib had a bug that would return only the last relation's data, not the entire set of data. This fixes that bug.

Includes unit tests

This adds the feature to get_sidebar_items to, when executed during a relation-broken event for this relation, omit the relation data for the breaking app (the app leaving the relation). For context, see https://chat.charmhub.io/charmhub/pl/ Possibly controversially, this feature uses the Juju environment variables to determine what event type is being executed. This is to avoid having to pass the `BoundEvent` object to every `get_sidebar_items()` call, and to keep this hack entirely in the relation library so the charm does not need to know about it.

Previously, the links portion of the kubeflow dashboard configmap was incorrectly rendering only the menulinks, not all links.

Removes tests for previously defined static links.

When refactoring the template, the links were changed by mistake. This reverts those changes.

This is a precursor step that makes the _context generation more extendable for other sources of links.

This adds a config option additional-sidebar-links which accepts a YAML or JSON formatted input defining additional sidebar links. These are combined with the relation-driven sidebar links.

…branch' into kf-3664-add-user-defined-sidebar-links # Conflicts: # src/charm.py # tests/integration/sidebar_requirer_tester_charm/src/charm.py # tests/integration/test_charm.py # tests/unit/test_operator.py # tests/unit/test_sidebar_lib.py

beliaev-maksim · 2023-06-29T08:17:55Z

just to confirm, did not we want to allow users to shuffle stuff and add new things. While core functionality should not be removed?

eg user should not be able to overwrite pipelines, notebooks, etc

kimwnasptd · 2023-06-29T11:42:32Z

My understanding from the PR is the following, but @ca-scribner please confirm:

The CentralDashboard now will populate links by aggregating the links defined:

In the CentralDashboard's config (juju config ...)
From the relations to the other charms, and this information is extracted with the common library implemented in add kubeflow-dashboard-sidebar interface charm-relation-interfaces#83

By default the CentralDashboard is not configured with any links. So initially the only links shown will be from the Charms that will be related to the CentralDashboard, using the library.

If an admin would like some custom links, that aren't to deployed as Charms, then they should use the CentralDashboard's config.

Lastly admins can configure the ordering from yet another config in the CentralDashboard. This config will have a default value of: '["Notebooks", "Experiments (AutoML)", "Experiments (KFP)", "Pipelines", "Runs", "Recurring Runs", "Volumes", "Tensorboards"]'

beliaev-maksim · 2023-06-29T12:06:19Z

@kimwnasptd so it will be Append instead of replace ?

kimwnasptd · 2023-06-29T12:27:33Z

Yes, it should be append. Since we want the links from the related charms to always be visible.

But if a user would like some extra link, they could do it with config of the CentralDashboard.

ca-scribner · 2023-06-29T15:20:01Z

This function is described more in this spec, so whatever we decide we should make sure it is clear there.

@kimwnasptd has it exactly right. Stating another way just in case it isn't clear, the dashboard charm by default has no sidebar links and sidebar links can be added by:

relating a charm to dashboard
putting a link in the additional-sidebar-links config

Removal of links is done by the reverse of the above two (removing a relation, or removing a link from the config).

main branch of charmed-kubeflow-workflows has a breaking UI change for _quality-checks.yaml that does not work here. Temporarily pinning quality-checks.yaml to a commit

tests/integration/test_charm.py

src/charm.py

…ests

* feat: add user-defined sidebar links This adds a config option additional-sidebar-links which accepts a YAML or JSON formatted input defining additional sidebar links. These are combined with the relation-driven sidebar links. Also: * adds tests for this feature * temporarily pins quality-checks.yaml to a commit because the main branch has a breaking change and previous releases were not versioned

ca-scribner added 22 commits June 23, 2023 14:01

feat: add kubeflow_dashboard_sidebar lib

cadfd27

Adds: * KubeflowDashboardSidebarRequirer * KubeflowDashboardSidebarProvider * SidebarItem * associated tests

feat: add capture_events lib

aca8dd6

Used as a helper in unit tests

fix: handle multiple relations in sidebar Provider

ab795b4

previously, the Provider lib had a bug that would return only the last relation's data, not the entire set of data. This fixes that bug.

fix: handle empty relations in sidebar Provider

ed3e471

feat: add Provider.get_sidebar_items_as_json

f9a713e

feat: implement sidebar Provider in charm

44ccb52

Includes unit tests

fix: rendering of dashboard configmap

82c4543

Previously, the links portion of the kubeflow dashboard configmap was incorrectly rendering only the menulinks, not all links.

feat: add integration tests for dynamic sidebar

102e434

fix: formatting and linting

17b6148

fix: removed unused BASE_SIDEBAR variable

7ac6531

fix: removed unused BASE_SIDEBAR variable from charm.py

17b5cbe

fix: formatting/linting

f50a033

feat: add log dumping to integration CI

7de140c

feat: add log dumping to integration CI

e02bd9a

refactor: renamed interface to kubeflow_dashboard_sidebar

0bc7153

fix: linting and linting configuration

8c15742

fix: typo in dump-charm-debug-artifacts action call

c470658

docs: add missing docstrings, comments.

1e17e01

fix: remove outdate part of dashboard_links integration test

2738bb3

Removes tests for previously defined static links.

fix: formatting

0afc21c

ca-scribner added the enhancement New feature or request label Jun 27, 2023

github-actions bot added the Libraries: Out of sync label Jun 27, 2023

ca-scribner added 2 commits June 28, 2023 10:49

fix: dashboard quickLinks and Documentation links

5b35b9c

When refactoring the template, the links were changed by mistake. This reverts those changes.

doc: add docstring in tests

b1f0e24

ca-scribner force-pushed the kf-3664-add-user-defined-sidebar-links branch from 9e546bb to 9e07d02 Compare June 28, 2023 15:01

ca-scribner added 3 commits June 28, 2023 13:41

dox: capture relation-broken handling in github issue

887bcaf

refactor generation of relation-driven sidebar items

f438eba

This is a precursor step that makes the _context generation more extendable for other sources of links.

feat: add user-defined sidebar links

6ca9c7a

This adds a config option additional-sidebar-links which accepts a YAML or JSON formatted input defining additional sidebar links. These are combined with the relation-driven sidebar links.

ca-scribner added 2 commits June 28, 2023 13:43

feat: add tests for user-driven sidebar links

b03861a

fix: typo in integration test names

0cb4218

ca-scribner force-pushed the kf-3664-add-user-defined-sidebar-links branch from 33c6ee0 to 0cb4218 Compare June 28, 2023 17:44

ca-scribner mentioned this pull request Jun 28, 2023

feat: add user-controlled sidebar link order via config #125

Merged

ca-scribner added 4 commits June 28, 2023 13:50

fix: formatting

3a180ec

fix: library version

7063fab

fix: remove duplicate library version

0ac3a17

ca-scribner force-pushed the kf-3664-add-user-defined-sidebar-links branch from a44e118 to 0ac3a17 Compare June 28, 2023 21:57

fix: rebase error

13577ff

ca-scribner marked this pull request as ready for review June 28, 2023 22:25

ca-scribner requested a review from a team as a code owner June 28, 2023 22:25

fix: tester charm name in integration tests

92b269f

fix: pin reusable CI workflow to commit

67817eb

main branch of charmed-kubeflow-workflows has a breaking UI change for _quality-checks.yaml that does not work here. Temporarily pinning quality-checks.yaml to a commit

NohaIhab reviewed Jul 5, 2023

View reviewed changes

tests/integration/test_charm.py Show resolved Hide resolved

NohaIhab reviewed Jul 5, 2023

View reviewed changes

src/charm.py Outdated Show resolved Hide resolved

ca-scribner added 2 commits July 5, 2023 15:46

doc: add docstring entry about the sidebar_requirer_tester_charm in t…

2f1e18f

…ests

add catch for yaml.YAMLError in charm.py

32d7863

ca-scribner merged commit ef63c6e into kf-3664-dynamic-sidebar-feature-branch Jul 6, 2023

ca-scribner deleted the kf-3664-add-user-defined-sidebar-links branch July 6, 2023 12:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add user-driven sidebar links via config #119

feat: add user-driven sidebar links via config #119

ca-scribner commented Jun 27, 2023 •

edited

Loading

beliaev-maksim commented Jun 29, 2023

kimwnasptd commented Jun 29, 2023 •

edited

Loading

beliaev-maksim commented Jun 29, 2023

kimwnasptd commented Jun 29, 2023

ca-scribner commented Jun 29, 2023

feat: add user-driven sidebar links via config #119

feat: add user-driven sidebar links via config #119

Conversation

ca-scribner commented Jun 27, 2023 • edited Loading

Summary

Usage/Testing

Todo before final review

beliaev-maksim commented Jun 29, 2023

kimwnasptd commented Jun 29, 2023 • edited Loading

beliaev-maksim commented Jun 29, 2023

kimwnasptd commented Jun 29, 2023

ca-scribner commented Jun 29, 2023

ca-scribner commented Jun 27, 2023 •

edited

Loading

kimwnasptd commented Jun 29, 2023 •

edited

Loading