diff --git a/docs/dashboards.md b/docs/dashboards.md index 73de6976..31ee9848 100644 --- a/docs/dashboards.md +++ b/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: +``` This file may contain extra information about the [widgets](#widget-types), but we aim at mostly [inferring it](#implicit-detection) from the SQL files. diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index e93c093e..3c39f611 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/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: diff --git a/tests/integration/dashboards/one_counter/dashboard.yml b/tests/integration/dashboards/one_counter/dashboard.yml new file mode 100644 index 00000000..a03e0197 --- /dev/null +++ b/tests/integration/dashboards/one_counter/dashboard.yml @@ -0,0 +1 @@ +display_name: Counter \ No newline at end of file diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index cc84a8d9..da30551b 100644 --- a/tests/integration/test_dashboards.py +++ b/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) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 531c4b77..043c885c 100644 --- a/tests/unit/test_dashboards.py +++ b/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()