Read display name from dashboard.yml (#144)

Author: JCZuurmond

docs/dashboards.md

@@ -197,7 +197,12 @@ write.
 ## `dashboard.yml` file
 
 The `dashboard.yml` file is used to define a top-level metadata for the dashboard, such as the display name, warehouse,
-and the list of tile overrides for cases, that cannot be handled with the [high-level metadata](#metadata) in the SQL files.
+and the list of tile overrides for cases, that cannot be handled with the [high-level metadata](#metadata) in the SQL
+files. The file requires the `display_name` field, other fields are optional. See below for the configuration schema:
+
+```yml
+display_name: <display name>
+```
 
 This file may contain extra information about the [widgets](#widget-types), but we aim at mostly [inferring it](#implicit-detection) from the SQL files.
 

src/databricks/labs/lsql/dashboards.py

@@ -1,6 +1,7 @@
 import dataclasses
 import json
 import logging
+from dataclasses import dataclass
 from pathlib import Path
 from typing import TypeVar
 
@@ -30,6 +31,20 @@
 logger = logging.getLogger(__name__)
 
 
+@dataclass
+class DashboardMetadata:
+    display_name: str
+
+    @classmethod
+    def from_dict(cls, raw: dict[str, str]) -> "DashboardMetadata":
+        return cls(
+            display_name=raw["display_name"],
+        )
+
+    def as_dict(self) -> dict[str, str]:
+        return dataclasses.asdict(self)
+
+
 class Dashboards:
     _MAXIMUM_DASHBOARD_WIDTH = 6
 
@@ -78,7 +93,9 @@ def _format_query(query: str) -> str:
         return formatted_query
 
     def create_dashboard(self, dashboard_folder: Path) -> Dashboard:
-        """Create a dashboard from code, i.e. configuration and queries."""
+        """Create a dashboard from code, i.e. metadata and queries."""
+        dashboard_metadata = self._parse_dashboard_metadata(dashboard_folder)
+
         position = Position(0, 0, 0, 0)  # First widget position
         datasets, layouts = [], []
         for query_path in sorted(dashboard_folder.glob("*.sql")):
@@ -106,10 +123,33 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard:
             layout = Layout(widget=widget, position=position)
             layouts.append(layout)
 
-        page = Page(name=dashboard_folder.name, display_name=dashboard_folder.name, layout=layouts)
+        page = Page(
+            name=dashboard_metadata.display_name,
+            display_name=dashboard_metadata.display_name,
+            layout=layouts,
+        )
         lakeview_dashboard = Dashboard(datasets=datasets, pages=[page])
         return lakeview_dashboard
 
+    @staticmethod
+    def _parse_dashboard_metadata(dashboard_folder: Path) -> DashboardMetadata:
+        fallback_metadata = DashboardMetadata(display_name=dashboard_folder.name)
+
+        dashboard_metadata_path = dashboard_folder / "dashboard.yml"
+        if not dashboard_metadata_path.exists():
+            return fallback_metadata
+
+        try:
+            raw = yaml.safe_load(dashboard_metadata_path.read_text())
+        except yaml.YAMLError as e:
+            logger.warning(f"Parsing {dashboard_metadata_path}: {e}")
+            return fallback_metadata
+        try:
+            return DashboardMetadata.from_dict(raw)
+        except KeyError as e:
+            logger.warning(f"Parsing {dashboard_metadata_path}: {e}")
+            return fallback_metadata
+
     @staticmethod
     def _get_text_widget(path: Path) -> Widget:
         widget = Widget(name=path.stem, textbox_spec=path.read_text())
@@ -166,21 +206,17 @@ def _get_width_and_height(self, widget: Widget) -> tuple[int, int]:
             raise NotImplementedError(f"No width defined for spec: {widget}")
         return width, height
 
-    def deploy_dashboard(
-        self, lakeview_dashboard: Dashboard, *, display_name: str | None = None, dashboard_id: str | None = None
-    ) -> SDKDashboard:
+    def deploy_dashboard(self, lakeview_dashboard: Dashboard, *, dashboard_id: str | None = None) -> SDKDashboard:
         """Deploy a lakeview dashboard."""
-        if (display_name is None and dashboard_id is None) or (display_name is not None and dashboard_id is not None):
-            raise ValueError("Give either display_name or dashboard_id.")
-        if display_name is not None:
-            dashboard = self._ws.lakeview.create(
-                display_name, serialized_dashboard=json.dumps(lakeview_dashboard.as_dict())
-            )
-        else:
-            assert dashboard_id is not None
+        if dashboard_id is not None:
             dashboard = self._ws.lakeview.update(
                 dashboard_id, serialized_dashboard=json.dumps(lakeview_dashboard.as_dict())
             )
+        else:
+            display_name = lakeview_dashboard.pages[0].display_name or lakeview_dashboard.pages[0].name
+            dashboard = self._ws.lakeview.create(
+                display_name, serialized_dashboard=json.dumps(lakeview_dashboard.as_dict())
+            )
         return dashboard
 
     def _with_better_names(self, dashboard: Dashboard) -> Dashboard:

tests/integration/dashboards/one_counter/dashboard.yml

@@ -0,0 +1 @@
+display_name: Counter

tests/integration/test_dashboards.py

@@ -1,65 +1,123 @@
 import json
+import logging
 from pathlib import Path
 
 import pytest
+from databricks.sdk.core import DatabricksError
+from databricks.sdk.service.dashboards import Dashboard as SDKDashboard
 
 from databricks.labs.lsql.dashboards import Dashboards
 from databricks.labs.lsql.lakeview.model import Dashboard
 
+logger = logging.getLogger(__name__)
+
+
+def factory(name, create, remove):
+    cleanup = []
+
+    def inner(**kwargs):
+        x = create(**kwargs)
+        logger.debug(f"added {name} fixture: {x}")
+        cleanup.append(x)
+        return x
+
+    yield inner
+    logger.debug(f"clearing {len(cleanup)} {name} fixtures")
+    for x in cleanup:
+        try:
+            logger.debug(f"removing {name} fixture: {x}")
+            remove(x)
+        except DatabricksError as e:
+            # TODO: fix on the databricks-labs-pytester level
+            logger.debug(f"ignoring error while {name} {x} teardown: {e}")
+
 
 @pytest.fixture
-def dashboard_id(ws, make_random):
+def make_dashboard(ws, make_random):
     """Clean the lakeview dashboard"""
 
-    dashboard_display_name = f"created_by_lsql_{make_random()}"
-    dashboard = ws.lakeview.create(dashboard_display_name)
+    def create(display_name: str = "") -> SDKDashboard:
+        if len(display_name) == 0:
+            display_name = f"created_by_lsql_{make_random()}"
+        else:
+            display_name = f"{display_name} ({make_random()})"
+        dashboard = ws.lakeview.create(display_name)
+        return dashboard
+
+    def delete(dashboard: SDKDashboard) -> None:
+        ws.lakeview.trash(dashboard.dashboard_id)
 
-    yield dashboard.dashboard_id
+    yield from factory("dashboard", create, delete)
 
-    ws.lakeview.trash(dashboard.dashboard_id)
 
+def test_dashboards_deploys_exported_dashboard_definition(ws, make_dashboard):
+    sdk_dashboard = make_dashboard()
 
-def test_dashboards_deploys_exported_dashboard_definition(ws, dashboard_id):
     dashboard_file = Path(__file__).parent / "dashboards" / "dashboard.json"
     with dashboard_file.open("r") as f:
         lakeview_dashboard = Dashboard.from_dict(json.load(f))
 
     dashboards = Dashboards(ws)
-    dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=dashboard_id)
+    sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id)
+
+    assert ws.lakeview.get(sdk_dashboard.dashboard_id)
 
-    assert ws.lakeview.get(dashboard.dashboard_id)
 
+def test_dashboard_deploys_dashboard_the_same_as_created_dashboard(ws, make_dashboard, tmp_path):
+    sdk_dashboard = make_dashboard()
 
-def test_dashboard_deploys_dashboard_the_same_as_created_dashboard(tmp_path, ws, dashboard_id):
     with (tmp_path / "counter.sql").open("w") as f:
         f.write("SELECT 10 AS count")
     dashboards = Dashboards(ws)
-    dashboard = dashboards.create_dashboard(tmp_path)
+    lakeview_dashboard = dashboards.create_dashboard(tmp_path)
 
-    sdk_dashboard = dashboards.deploy_dashboard(dashboard, dashboard_id=dashboard_id)
+    sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id)
     new_dashboard = dashboards.get_dashboard(sdk_dashboard.path)
 
-    assert dashboards._with_better_names(dashboard).as_dict() == dashboards._with_better_names(new_dashboard).as_dict()
+    assert (
+        dashboards._with_better_names(lakeview_dashboard).as_dict()
+        == dashboards._with_better_names(new_dashboard).as_dict()
+    )
 
 
-def test_dashboard_deploys_dashboard_with_ten_counters(ws, dashboard_id, tmp_path):
+def test_dashboard_deploys_dashboard_with_ten_counters(ws, make_dashboard, tmp_path):
+    sdk_dashboard = make_dashboard()
+
     for i in range(10):
         with (tmp_path / f"counter_{i}.sql").open("w") as f:
             f.write(f"SELECT {i} AS count")
     dashboards = Dashboards(ws)
     lakeview_dashboard = dashboards.create_dashboard(tmp_path)
 
-    sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=dashboard_id)
+    sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id)
 
     assert ws.lakeview.get(sdk_dashboard.dashboard_id)
 
 
-def test_dashboard_deploys_dashboard_with_counter_variation(ws, dashboard_id, tmp_path):
+def test_dashboard_deploys_dashboard_with_display_name(ws, make_dashboard, tmp_path):
+    sdk_dashboard = make_dashboard(display_name="Counter")
+
+    with (tmp_path / "dashboard.yml").open("w") as f:
+        f.write("display_name: Counter")
+    with (tmp_path / "counter.sql").open("w") as f:
+        f.write("SELECT 102132 AS count")
+
+    dashboards = Dashboards(ws)
+    lakeview_dashboard = dashboards.create_dashboard(tmp_path)
+
+    sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id)
+
+    assert ws.lakeview.get(sdk_dashboard.dashboard_id)
+
+
+def test_dashboard_deploys_dashboard_with_counter_variation(ws, make_dashboard, tmp_path):
+    sdk_dashboard = make_dashboard()
+
     with (tmp_path / "counter.sql").open("w") as f:
         f.write("SELECT 10 AS something_else_than_count")
     dashboards = Dashboards(ws)
     lakeview_dashboard = dashboards.create_dashboard(tmp_path)
 
-    sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=dashboard_id)
+    sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id)
 
     assert ws.lakeview.get(sdk_dashboard.dashboard_id)

tests/unit/test_dashboards.py

@@ -5,7 +5,7 @@
 import pytest
 from databricks.sdk import WorkspaceClient
 
-from databricks.labs.lsql.dashboards import Dashboards
+from databricks.labs.lsql.dashboards import DashboardMetadata, Dashboards
 from databricks.labs.lsql.lakeview import (
     CounterEncodingMap,
     CounterSpec,
@@ -20,6 +20,22 @@
 )
 
 
+def test_dashboard_configuration_raises_key_error_if_display_name_is_missing():
+    with pytest.raises(KeyError):
+        DashboardMetadata.from_dict({})
+
+
+def test_dashboard_configuration_sets_display_name_from_dict():
+    dashboard_metadata = DashboardMetadata.from_dict({"display_name": "test"})
+    assert dashboard_metadata.display_name == "test"
+
+
+def test_dashboard_configuration_from_and_as_dict_is_a_unit_function():
+    raw = {"display_name": "test"}
+    dashboard_metadata = DashboardMetadata.from_dict(raw)
+    assert dashboard_metadata.as_dict() == raw
+
+
 def test_dashboards_saves_sql_files_to_folder(tmp_path):
     ws = create_autospec(WorkspaceClient)
     queries = Path(__file__).parent / "queries"
@@ -42,6 +58,42 @@ def test_dashboards_saves_yml_files_to_folder(tmp_path):
     ws.assert_not_called()
 
 
+def test_dashboards_creates_dashboard_with_first_page_name_after_folder():
+    ws = create_autospec(WorkspaceClient)
+    queries = Path(__file__).parent / "queries"
+    lakeview_dashboard = Dashboards(ws).create_dashboard(queries)
+    page = lakeview_dashboard.pages[0]
+    assert page.name == "queries"
+    assert page.display_name == "queries"
+
+
+def test_dashboards_creates_dashboard_with_custom_first_page_name(tmp_path):
+    with (tmp_path / "dashboard.yml").open("w") as f:
+        f.write("display_name: Custom")
+
+    ws = create_autospec(WorkspaceClient)
+    lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path)
+
+    page = lakeview_dashboard.pages[0]
+    assert page.name == "Custom"
+    assert page.display_name == "Custom"
+
+
+@pytest.mark.parametrize("dashboard_content", ["missing_display_name: true", "invalid:\nyml"])
+def test_dashboards_handles_invalid_dashboard_yml(tmp_path, dashboard_content):
+    queries_path = tmp_path / "queries"
+    queries_path.mkdir()
+    with (queries_path / "dashboard.yml").open("w") as f:
+        f.write(dashboard_content)
+
+    ws = create_autospec(WorkspaceClient)
+    lakeview_dashboard = Dashboards(ws).create_dashboard(queries_path)
+
+    page = lakeview_dashboard.pages[0]
+    assert page.name == "queries"
+    assert page.display_name == "queries"
+
+
 def test_dashboards_creates_one_dataset_per_query():
     ws = create_autospec(WorkspaceClient)
     queries = Path(__file__).parent / "queries"
@@ -287,29 +339,12 @@ def test_dashboards_creates_dashboards_where_text_widget_has_expected_text(tmp_p
     ws.assert_not_called()
 
 
-def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dashboard_id():
+def test_dashboards_deploy_calls_create_without_dashboard_id():
     ws = create_autospec(WorkspaceClient)
     dashboards = Dashboards(ws)
-    dashboard = Dashboard([], [])
-    with pytest.raises(ValueError):
-        dashboards.deploy_dashboard(dashboard)
-    ws.assert_not_called()
 
-
-def test_dashboards_deploy_raises_value_error_with_both_display_name_and_dashboard_id():
-    ws = create_autospec(WorkspaceClient)
-    dashboards = Dashboards(ws)
-    dashboard = Dashboard([], [])
-    with pytest.raises(ValueError):
-        dashboards.deploy_dashboard(dashboard, display_name="test", dashboard_id="test")
-    ws.assert_not_called()
-
-
-def test_dashboards_deploy_calls_create_with_display_name():
-    ws = create_autospec(WorkspaceClient)
-    dashboards = Dashboards(ws)
-    dashboard = Dashboard([], [])
-    dashboards.deploy_dashboard(dashboard, display_name="test")
+    dashboard = Dashboard([], [Page("test", [])])
+    dashboards.deploy_dashboard(dashboard)
 
     ws.lakeview.create.assert_called_once()
     ws.lakeview.update.assert_not_called()