From 6bba9c5e7315ad6a3a80d3c23f1f320f9dffab67 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 08:56:44 +0200 Subject: [PATCH 01/39] Add counter markdown --- tests/integration/dashboards/one_counter/000_counter.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/dashboards/one_counter/000_counter.md b/tests/integration/dashboards/one_counter/000_counter.md index 1b644946..5fac24ed 100644 --- a/tests/integration/dashboards/one_counter/000_counter.md +++ b/tests/integration/dashboards/one_counter/000_counter.md @@ -1,5 +1,5 @@ # Counter -Below you see an example counter widget. Counter widgets in dashboards are used to display a single, +On the right side you see an example counter widget. Counter widgets in dashboards are used to display a single, high-level metric or KPI. They provide a quick and easy way for users to see important information at a glance, making it convenient for monitoring and decision-making. \ No newline at end of file From d060cae725391d4acc01de4a89f169e1e1794604 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 09:05:52 +0200 Subject: [PATCH 02/39] Fix typo --- src/databricks/labs/lsql/dashboards.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index e93c093e..b00a15c1 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -91,18 +91,14 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: for path in sorted(dashboard_folder.iterdir()): if path.suffix not in {".sql", ".md"}: continue - if path.suffix == ".sql": - dataset = datasets[dataset_index] - assert dataset.name == path.stem - dataset_index += 1 - try: - widget = self._get_widget(dataset) - except sqlglot.ParseError as e: - logger.warning(f"Error '{e}' when parsing: {dataset.query}") - continue - else: - widget = self._get_text_widget(path) - position = self._get_position(widget, position) + query = Query(dataset_name=dataset.name, fields=fields, disaggregated=True) + # As far as testing went, a NamedQuery should always have "main_query" as name + named_query = NamedQuery(name="main_query", query=query) + # Counters are expected to have one field + counter_field_encoding = CounterFieldEncoding(field_name=fields[0].name, display_name=fields[0].name) + counter_spec = CounterSpec(CounterEncodingMap(value=counter_field_encoding)) + widget = Widget(name=dataset.name, queries=[named_query], spec=counter_spec) + position = self._get_position(counter_spec, position) layout = Layout(widget=widget, position=position) layouts.append(layout) From 4efb933c6366ec32aa09486a390bddea3a82fad0 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 10:34:39 +0200 Subject: [PATCH 03/39] Loop through datasets --- src/databricks/labs/lsql/dashboards.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index b00a15c1..bb283850 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -87,9 +87,11 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: dataset = Dataset(name=query_path.stem, display_name=query_path.stem, query=raw_query) datasets.append(dataset) - dataset_index = 0 - for path in sorted(dashboard_folder.iterdir()): - if path.suffix not in {".sql", ".md"}: + for dataset in datasets: + try: + fields = self._get_fields(dataset.query) + except sqlglot.ParseError as e: + logger.warning(f"Error '{e}' when parsing: {dataset.query}") continue query = Query(dataset_name=dataset.name, fields=fields, disaggregated=True) # As far as testing went, a NamedQuery should always have "main_query" as name From 1f6628b576e193d60b0e6a6e823b180bd9e72734 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 11:02:52 +0200 Subject: [PATCH 04/39] Support text widgets --- src/databricks/labs/lsql/dashboards.py | 42 ++++++++++++++++---------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index bb283850..148ef0fe 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -87,20 +87,30 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: dataset = Dataset(name=query_path.stem, display_name=query_path.stem, query=raw_query) datasets.append(dataset) - for dataset in datasets: - try: - fields = self._get_fields(dataset.query) - except sqlglot.ParseError as e: - logger.warning(f"Error '{e}' when parsing: {dataset.query}") + dataset_index = 0 + for path in sorted(dashboard_folder.iterdir()): + if path.suffix not in {".sql", ".md"}: continue - query = Query(dataset_name=dataset.name, fields=fields, disaggregated=True) - # As far as testing went, a NamedQuery should always have "main_query" as name - named_query = NamedQuery(name="main_query", query=query) - # Counters are expected to have one field - counter_field_encoding = CounterFieldEncoding(field_name=fields[0].name, display_name=fields[0].name) - counter_spec = CounterSpec(CounterEncodingMap(value=counter_field_encoding)) - widget = Widget(name=dataset.name, queries=[named_query], spec=counter_spec) - position = self._get_position(counter_spec, position) + if path.suffix == ".sql": + dataset = datasets[dataset_index] + assert dataset.name == path.stem + try: + fields = self._get_fields(dataset.query) + except sqlglot.ParseError as e: + logger.warning(f"Error '{e}' when parsing: {dataset.query}") + continue + query = Query(dataset_name=dataset.name, fields=fields, disaggregated=True) + # As far as testing went, a NamedQuery should always have "main_query" as name + named_query = NamedQuery(name="main_query", query=query) + # Counters are expected to have one field + counter_field_encoding = CounterFieldEncoding(field_name=fields[0].name, display_name=fields[0].name) + counter_spec = CounterSpec(CounterEncodingMap(value=counter_field_encoding)) + widget = Widget(name=dataset.name, queries=[named_query], spec=counter_spec) + dataset_index += 1 + else: + with path.open("r") as f: + widget = Widget(name=path.stem, textbox_spec=f.read()) + position = self._get_position(widget, position) layout = Layout(widget=widget, position=position) layouts.append(layout) @@ -151,11 +161,11 @@ def _get_width_and_height(self, widget: Widget) -> tuple[int, int]: """Get the width and height for a widget. The tiling logic works if: - - width < self._MAXIMUM_DASHBOARD_WIDTH : heights for widgets on the same row should be equal - - width == self._MAXIMUM_DASHBOARD_WIDTH : any height + - width < self._maximum_dashboard_width : heights for widgets on the same row should be equal + - width == self._maximum_dashboard_width : any height """ if widget.textbox_spec is not None: - return self._MAXIMUM_DASHBOARD_WIDTH, 2 + return self._maximum_dashboard_width, 2 height = 3 if isinstance(widget.spec, CounterSpec): From 5cf45e3394b1e667291adf8170bbb0cb00318fc2 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 11:03:34 +0200 Subject: [PATCH 05/39] Update query files --- tests/integration/dashboards/one_counter/000_counter.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/dashboards/one_counter/000_counter.md b/tests/integration/dashboards/one_counter/000_counter.md index 5fac24ed..1b644946 100644 --- a/tests/integration/dashboards/one_counter/000_counter.md +++ b/tests/integration/dashboards/one_counter/000_counter.md @@ -1,5 +1,5 @@ # Counter -On the right side you see an example counter widget. Counter widgets in dashboards are used to display a single, +Below you see an example counter widget. Counter widgets in dashboards are used to display a single, high-level metric or KPI. They provide a quick and easy way for users to see important information at a glance, making it convenient for monitoring and decision-making. \ No newline at end of file From 55373c03f5ff824ff45145a53b681e446397da05 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 11:16:13 +0200 Subject: [PATCH 06/39] Refactor to reduce create_dashboard complexity --- src/databricks/labs/lsql/dashboards.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 148ef0fe..e99eefd1 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -94,22 +94,14 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: if path.suffix == ".sql": dataset = datasets[dataset_index] assert dataset.name == path.stem + dataset_index += 1 try: - fields = self._get_fields(dataset.query) + widget = self._get_widget(dataset) except sqlglot.ParseError as e: logger.warning(f"Error '{e}' when parsing: {dataset.query}") continue - query = Query(dataset_name=dataset.name, fields=fields, disaggregated=True) - # As far as testing went, a NamedQuery should always have "main_query" as name - named_query = NamedQuery(name="main_query", query=query) - # Counters are expected to have one field - counter_field_encoding = CounterFieldEncoding(field_name=fields[0].name, display_name=fields[0].name) - counter_spec = CounterSpec(CounterEncodingMap(value=counter_field_encoding)) - widget = Widget(name=dataset.name, queries=[named_query], spec=counter_spec) - dataset_index += 1 else: - with path.open("r") as f: - widget = Widget(name=path.stem, textbox_spec=f.read()) + widget = self._get_text_widget(path) position = self._get_position(widget, position) layout = Layout(widget=widget, position=position) layouts.append(layout) @@ -120,7 +112,8 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: @staticmethod def _get_text_widget(path: Path) -> Widget: - widget = Widget(name=path.stem, textbox_spec=path.read_text()) + with path.open("r") as f: + widget = Widget(name=path.stem, textbox_spec=f.read()) return widget def _get_widget(self, dataset: Dataset) -> Widget: From 2dc78c5987a5f135c174a9b23fe81653df49e688 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 12:24:31 +0200 Subject: [PATCH 07/39] Test text widget height --- tests/unit/test_dashboards.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 531c4b77..d2ff955e 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -273,20 +273,6 @@ def test_dashboards_creates_dashboards_where_text_widget_has_expected_width_and_ ws.assert_not_called() -def test_dashboards_creates_dashboards_where_text_widget_has_expected_text(tmp_path): - ws = create_autospec(WorkspaceClient) - - content = "# Description" - with (tmp_path / "description.md").open("w") as f: - f.write(content) - - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - widget = lakeview_dashboard.pages[0].layout[0].widget - - assert widget.textbox_spec == content - ws.assert_not_called() - - def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dashboard_id(): ws = create_autospec(WorkspaceClient) dashboards = Dashboards(ws) From 0ee28608f568a34002b22576f1d952b432a2da2c Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 12:25:58 +0200 Subject: [PATCH 08/39] Test text widget content --- tests/unit/test_dashboards.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index d2ff955e..531c4b77 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -273,6 +273,20 @@ def test_dashboards_creates_dashboards_where_text_widget_has_expected_width_and_ ws.assert_not_called() +def test_dashboards_creates_dashboards_where_text_widget_has_expected_text(tmp_path): + ws = create_autospec(WorkspaceClient) + + content = "# Description" + with (tmp_path / "description.md").open("w") as f: + f.write(content) + + lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + widget = lakeview_dashboard.pages[0].layout[0].widget + + assert widget.textbox_spec == content + ws.assert_not_called() + + def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dashboard_id(): ws = create_autospec(WorkspaceClient) dashboards = Dashboards(ws) From 1ad458e2d148da5f3bf9a7ec46c3726774209903 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 12:28:06 +0200 Subject: [PATCH 09/39] Test widget below text widget --- tests/unit/test_dashboards.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 531c4b77..58a627c4 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -216,15 +216,14 @@ def test_dashboards_creates_dashboard_with_many_widgets_not_on_the_first_row(tmp def test_dashboards_creates_dashboard_with_widget_below_text_widget(tmp_path): ws = create_autospec(WorkspaceClient) - with (tmp_path / "000_counter.md").open("w") as f: + with (tmp_path / f"000_counter.md").open("w") as f: f.write("# Description") - with (tmp_path / "010_counter.sql").open("w") as f: - f.write("SELECT 100 AS count") + with (tmp_path / f"010_counter.sql").open("w") as f: + f.write(f"SELECT 100 AS count") lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) layout = lakeview_dashboard.pages[0].layout - assert len(layout) == 2 assert layout[0].position.y < layout[1].position.y ws.assert_not_called() From 4e1f28c6388c0e7f703c8d04019a4ec4ef5e8caf Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 12:32:00 +0200 Subject: [PATCH 10/39] Test for invalid query not to be the first --- tests/unit/test_dashboards.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 58a627c4..6184d44d 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -66,7 +66,7 @@ def test_dashboards_creates_one_counter_widget_per_query(): def test_dashboards_skips_invalid_query(tmp_path, caplog): ws = create_autospec(WorkspaceClient) - # Test for the invalid query not to be the first or last query + # Test for the invalid query not to be the first query for i in range(0, 3, 2): with (tmp_path / f"{i}_counter.sql").open("w") as f: f.write(f"SELECT {i} AS count") From 816aefa4cdb349693acf7ff8eae3291a7ad5a4b6 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 12:38:27 +0200 Subject: [PATCH 11/39] Make sure two layouts are created for test --- tests/unit/test_dashboards.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 6184d44d..069339c9 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -224,6 +224,7 @@ def test_dashboards_creates_dashboard_with_widget_below_text_widget(tmp_path): lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) layout = lakeview_dashboard.pages[0].layout + assert len(layout) == 2 assert layout[0].position.y < layout[1].position.y ws.assert_not_called() From d3d1aaa47785c8260d695ae0e367d7ecd83da0f2 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 12:38:51 +0200 Subject: [PATCH 12/39] Format --- tests/unit/test_dashboards.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 069339c9..c36ce8d3 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -216,10 +216,10 @@ def test_dashboards_creates_dashboard_with_many_widgets_not_on_the_first_row(tmp def test_dashboards_creates_dashboard_with_widget_below_text_widget(tmp_path): ws = create_autospec(WorkspaceClient) - with (tmp_path / f"000_counter.md").open("w") as f: + with (tmp_path / "000_counter.md").open("w") as f: f.write("# Description") - with (tmp_path / f"010_counter.sql").open("w") as f: - f.write(f"SELECT 100 AS count") + with (tmp_path / "010_counter.sql").open("w") as f: + f.write("SELECT 100 AS count") lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) layout = lakeview_dashboard.pages[0].layout From 53cbc0ee6137658ab45d5a5c6d6ba601be0497bf Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 12:42:24 +0200 Subject: [PATCH 13/39] Update comment --- tests/unit/test_dashboards.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index c36ce8d3..531c4b77 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -66,7 +66,7 @@ def test_dashboards_creates_one_counter_widget_per_query(): def test_dashboards_skips_invalid_query(tmp_path, caplog): ws = create_autospec(WorkspaceClient) - # Test for the invalid query not to be the first query + # Test for the invalid query not to be the first or last query for i in range(0, 3, 2): with (tmp_path / f"{i}_counter.sql").open("w") as f: f.write(f"SELECT {i} AS count") From e60f9b7046cedd4882cc7dd096471f02ddc1960c Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 22:05:34 +0200 Subject: [PATCH 14/39] Capatilize mazimum dashboard width --- src/databricks/labs/lsql/dashboards.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index e99eefd1..b5c822a4 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -154,11 +154,11 @@ def _get_width_and_height(self, widget: Widget) -> tuple[int, int]: """Get the width and height for a widget. The tiling logic works if: - - width < self._maximum_dashboard_width : heights for widgets on the same row should be equal - - width == self._maximum_dashboard_width : any height + - width < self._MAXIMUM_DASHBOARD_WIDTH : heights for widgets on the same row should be equal + - width == self._MAXIMUM_DASHBOARD_WIDTH : any height """ if widget.textbox_spec is not None: - return self._maximum_dashboard_width, 2 + return self._MAXIMUM_DASHBOARD_WIDTH, 2 height = 3 if isinstance(widget.spec, CounterSpec): From 59287cbee44e694e7cbb9df5d9b4399bda52b3d9 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 22:06:04 +0200 Subject: [PATCH 15/39] Use path read text --- src/databricks/labs/lsql/dashboards.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index b5c822a4..e93c093e 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -112,8 +112,7 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: @staticmethod def _get_text_widget(path: Path) -> Widget: - with path.open("r") as f: - widget = Widget(name=path.stem, textbox_spec=f.read()) + widget = Widget(name=path.stem, textbox_spec=path.read_text()) return widget def _get_widget(self, dataset: Dataset) -> Widget: From 6d1ef1995fd0417daa9e6949d085c6bea051122a Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 14:11:31 +0200 Subject: [PATCH 16/39] Add dashboard.yml --- tests/integration/dashboards/one_counter/dashboard.yml | 1 + 1 file changed, 1 insertion(+) create mode 100644 tests/integration/dashboards/one_counter/dashboard.yml 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 From 82211ab2d4a75292993092895750100a399bc292 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 14:13:13 +0200 Subject: [PATCH 17/39] Add display name to dashboard.yml docs --- docs/dashboards.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/dashboards.md b/docs/dashboards.md index 73de6976..546d278a 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. It has the following format: + +```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. From 34a3e3d78cb7b24f47d14ad5f0c7aef9cb087218 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 14:17:33 +0200 Subject: [PATCH 18/39] Add method for getting dashboard configuration --- src/databricks/labs/lsql/dashboards.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index e93c093e..f02c4eec 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,11 @@ logger = logging.getLogger(__name__) +@dataclass +class DashboardConfiguration: + display_name: str + + class Dashboards: _MAXIMUM_DASHBOARD_WIDTH = 6 @@ -79,6 +85,8 @@ def _format_query(query: str) -> str: def create_dashboard(self, dashboard_folder: Path) -> Dashboard: """Create a dashboard from code, i.e. configuration and queries.""" + dashboard_configuration = self._get_dashboard_configuration(dashboard_folder) + position = Position(0, 0, 0, 0) # First widget position datasets, layouts = [], [] for query_path in sorted(dashboard_folder.glob("*.sql")): @@ -106,10 +114,17 @@ 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_configuration.display_name, display_name=dashboard_configuration.display_name, layout=layouts) lakeview_dashboard = Dashboard(datasets=datasets, pages=[page]) return lakeview_dashboard + @staticmethod + def _get_dashboard_configuration(dashboard_folder: Path) -> DashboardConfiguration: + dashboard_path = dashboard_folder / "dashboard.yml" + if not dashboard_path.exists(): + dashboard_configuration = DashboardConfiguration(display_name=dashboard_path.name) + return dashboard_configuration + @staticmethod def _get_text_widget(path: Path) -> Widget: widget = Widget(name=path.stem, textbox_spec=path.read_text()) From 983012af3a5e63eda253bd1a479d6288f864768c Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 14:37:14 +0200 Subject: [PATCH 19/39] Read dashboard from yaml --- src/databricks/labs/lsql/dashboards.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index f02c4eec..61b6b34a 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -35,6 +35,12 @@ class DashboardConfiguration: display_name: str + @classmethod + def from_dict(cls, configuration: dict[str, str]) -> "DashboardConfiguration": + return cls( + display_name=configuration["display_name"], + ) + class Dashboards: _MAXIMUM_DASHBOARD_WIDTH = 6 @@ -125,6 +131,10 @@ def _get_dashboard_configuration(dashboard_folder: Path) -> DashboardConfigurati dashboard_configuration = DashboardConfiguration(display_name=dashboard_path.name) return dashboard_configuration + with dashboard_path.open("r") as f: + raw_configuration = yaml.safe_load(f) + return DashboardConfiguration.from_dict(raw_configuration) + @staticmethod def _get_text_widget(path: Path) -> Widget: widget = Widget(name=path.stem, textbox_spec=path.read_text()) From 852b484fd6be4ebbc7820769897b6f8d1c461e8d Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 14:41:38 +0200 Subject: [PATCH 20/39] Deploy new dashboards with display name from first page --- src/databricks/labs/lsql/dashboards.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 61b6b34a..ef94f02a 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -192,20 +192,18 @@ def _get_width_and_height(self, widget: Widget) -> tuple[int, int]: return width, height def deploy_dashboard( - self, lakeview_dashboard: Dashboard, *, display_name: str | None = None, dashboard_id: str | None = None + 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: From cb2142cac4aef502c43a6f7e1c41fe8a0c359f5d Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 15:04:56 +0200 Subject: [PATCH 21/39] Refactor dashboard_id fixture to make dashboard --- tests/integration/test_dashboards.py | 64 ++++++++++++++++++++++------ 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index cc84a8d9..5a703b15 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -1,55 +1,91 @@ import json +import logging from pathlib import Path import pytest from databricks.labs.lsql.dashboards import Dashboards from databricks.labs.lsql.lakeview.model import Dashboard +from databricks.sdk.core import DatabricksError +from databricks.sdk.service.dashboards import Dashboard as SDKDashboard + + +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()}" + dashboard = ws.lakeview.create(display_name) + return dashboard - yield dashboard.dashboard_id + def delete(dashboard: SDKDashboard) -> None: + ws.lakeview.trash(dashboard.dashboard_id) - ws.lakeview.trash(dashboard.dashboard_id) + yield from factory("dashboard", create, delete) -def test_dashboards_deploys_exported_dashboard_definition(ws, dashboard_id): +def test_dashboards_deploys_exported_dashboard_definition(ws, make_dashboard): + sdk_dashboard = make_dashboard() + 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, make_dashboard, tmp_path): + sdk_dashboard = make_dashboard() -def test_dashboard_deploys_dashboard_with_ten_counters(ws, dashboard_id, tmp_path): 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) From e01be595195ad57cc62798b342d786d9630e473d Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 15:21:12 +0200 Subject: [PATCH 22/39] Add integration test with display name --- tests/integration/test_dashboards.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index 5a703b15..a59965fb 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -40,6 +40,8 @@ def make_dashboard(ws, make_random): 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 @@ -90,6 +92,21 @@ def test_dashboard_deploys_dashboard_with_ten_counters(ws, make_dashboard, tmp_p assert ws.lakeview.get(sdk_dashboard.dashboard_id) +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(f"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, dashboard_id, tmp_path): with (tmp_path / "counter.sql").open("w") as f: f.write("SELECT 10 AS something_else_than_count") From af9f5bbbe6023439a2c91ed934f568a2ccc0dce1 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 15:22:50 +0200 Subject: [PATCH 23/39] Test display name is required --- tests/unit/test_dashboards.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 531c4b77..2287769c 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 DashboardConfiguration, Dashboards from databricks.labs.lsql.lakeview import ( CounterEncodingMap, CounterSpec, @@ -20,6 +20,11 @@ ) +def test_dashboard_configuration_raises_key_error_if_display_name_is_missing(): + with pytest.raises(KeyError): + DashboardConfiguration.from_dict({}) + + def test_dashboards_saves_sql_files_to_folder(tmp_path): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" From 235650c105e16fbe27ebe9920a0e73da70a04949 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 15:24:20 +0200 Subject: [PATCH 24/39] Update docs --- docs/dashboards.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/dashboards.md b/docs/dashboards.md index 546d278a..31ee9848 100644 --- a/docs/dashboards.md +++ b/docs/dashboards.md @@ -198,7 +198,7 @@ write. 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. It has the following format: +files. The file requires the `display_name` field, other fields are optional. See below for the configuration schema: ```yml display_name: From 1c3d25600cc137db0431aece54e99ec271b1aa87 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 15:27:24 +0200 Subject: [PATCH 25/39] Test first page (display) name if dashboard.yml not present --- src/databricks/labs/lsql/dashboards.py | 2 +- tests/unit/test_dashboards.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index ef94f02a..c8ed8fc7 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -128,7 +128,7 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: def _get_dashboard_configuration(dashboard_folder: Path) -> DashboardConfiguration: dashboard_path = dashboard_folder / "dashboard.yml" if not dashboard_path.exists(): - dashboard_configuration = DashboardConfiguration(display_name=dashboard_path.name) + dashboard_configuration = DashboardConfiguration(display_name=dashboard_folder.name) return dashboard_configuration with dashboard_path.open("r") as f: diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 2287769c..deda7181 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -47,6 +47,15 @@ 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_one_dataset_per_query(): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" From d9e5dc1eb65ae36b214e5ddfa88382e8b7eff45f Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 15:28:40 +0200 Subject: [PATCH 26/39] Test custom first page (display) name --- tests/unit/test_dashboards.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index deda7181..24c92eaf 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -56,6 +56,18 @@ def test_dashboards_creates_dashboard_with_first_page_name_after_folder(): 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" + + def test_dashboards_creates_one_dataset_per_query(): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" From 0326b6806d8f59d7c91f240ee63edc3f243549c1 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 15:38:29 +0200 Subject: [PATCH 27/39] Handle invalid dashboard yamls --- src/databricks/labs/lsql/dashboards.py | 18 ++++++++++++++---- tests/unit/test_dashboards.py | 15 +++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index c8ed8fc7..3b579723 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -126,14 +126,24 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: @staticmethod def _get_dashboard_configuration(dashboard_folder: Path) -> DashboardConfiguration: + fallback_configuration = DashboardConfiguration(display_name=dashboard_folder.name) + dashboard_path = dashboard_folder / "dashboard.yml" if not dashboard_path.exists(): - dashboard_configuration = DashboardConfiguration(display_name=dashboard_folder.name) - return dashboard_configuration + return fallback_configuration with dashboard_path.open("r") as f: - raw_configuration = yaml.safe_load(f) - return DashboardConfiguration.from_dict(raw_configuration) + try: + raw_configuration = yaml.safe_load(f) + except yaml.YAMLError as e: + logger.warning(f"Error '{e}' when parsing: {dashboard_path}") + return fallback_configuration + try: + return DashboardConfiguration.from_dict(raw_configuration) + except KeyError as e: + logger.warning(f"Error '{e}' when parsing: {dashboard_path}") + return fallback_configuration + @staticmethod def _get_text_widget(path: Path) -> Widget: diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 24c92eaf..3a10353d 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -68,6 +68,21 @@ def test_dashboards_creates_dashboard_with_custom_first_page_name(tmp_path): 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" From 42d6d77adce14a23fcceede249fe28058ce5aaf4 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 15:38:53 +0200 Subject: [PATCH 28/39] Format --- src/databricks/labs/lsql/dashboards.py | 9 ++++----- tests/integration/test_dashboards.py | 12 +++++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 3b579723..0c82e306 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -120,7 +120,9 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: layout = Layout(widget=widget, position=position) layouts.append(layout) - page = Page(name=dashboard_configuration.display_name, display_name=dashboard_configuration.display_name, layout=layouts) + page = Page( + name=dashboard_configuration.display_name, display_name=dashboard_configuration.display_name, layout=layouts + ) lakeview_dashboard = Dashboard(datasets=datasets, pages=[page]) return lakeview_dashboard @@ -144,7 +146,6 @@ def _get_dashboard_configuration(dashboard_folder: Path) -> DashboardConfigurati logger.warning(f"Error '{e}' when parsing: {dashboard_path}") return fallback_configuration - @staticmethod def _get_text_widget(path: Path) -> Widget: widget = Widget(name=path.stem, textbox_spec=path.read_text()) @@ -201,9 +202,7 @@ 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, *, dashboard_id: str | None = None - ) -> SDKDashboard: + def deploy_dashboard(self, lakeview_dashboard: Dashboard, *, dashboard_id: str | None = None) -> SDKDashboard: """Deploy a lakeview dashboard.""" if dashboard_id is not None: dashboard = self._ws.lakeview.update( diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index a59965fb..9b3b6748 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -3,12 +3,11 @@ from pathlib import Path import pytest - -from databricks.labs.lsql.dashboards import Dashboards -from databricks.labs.lsql.lakeview.model import Dashboard 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__) @@ -75,7 +74,10 @@ def test_dashboard_deploys_dashboard_the_same_as_created_dashboard(ws, make_dash 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(lakeview_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, make_dashboard, tmp_path): @@ -95,7 +97,7 @@ def test_dashboard_deploys_dashboard_with_ten_counters(ws, make_dashboard, tmp_p 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(f"display_name: Counter") + f.write("display_name: Counter") with (tmp_path / "counter.sql").open("w") as f: f.write("SELECT 102132 AS count") From 23034e59815c39968924a0a842e3544e3bde04b1 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 15:44:12 +0200 Subject: [PATCH 29/39] Fix deploy dashboard tests --- tests/unit/test_dashboards.py | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 3a10353d..1a511946 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -328,29 +328,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() From 2db1236bb392ae4f0352a9ace948a283f4089c0c Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Tue, 11 Jun 2024 22:13:03 +0200 Subject: [PATCH 30/39] Fix integration test --- tests/integration/test_dashboards.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index 9b3b6748..da30551b 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -96,6 +96,7 @@ def test_dashboard_deploys_dashboard_with_ten_counters(ws, make_dashboard, tmp_p 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: @@ -109,12 +110,14 @@ def test_dashboard_deploys_dashboard_with_display_name(ws, make_dashboard, tmp_p 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_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) From 9ff23e65dc4104033f08946890c3c7c51493ec91 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 09:52:13 +0200 Subject: [PATCH 31/39] Rename DashboardConfiguration to DashboardMetadata --- src/databricks/labs/lsql/dashboards.py | 16 ++++++++-------- tests/unit/test_dashboards.py | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 0c82e306..bf33d097 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -32,11 +32,11 @@ @dataclass -class DashboardConfiguration: +class DashboardMetadata: display_name: str @classmethod - def from_dict(cls, configuration: dict[str, str]) -> "DashboardConfiguration": + def from_dict(cls, configuration: dict[str, str]) -> "DashboardMetadata": return cls( display_name=configuration["display_name"], ) @@ -127,24 +127,24 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: return lakeview_dashboard @staticmethod - def _get_dashboard_configuration(dashboard_folder: Path) -> DashboardConfiguration: - fallback_configuration = DashboardConfiguration(display_name=dashboard_folder.name) + def _get_dashboard_configuration(dashboard_folder: Path) -> DashboardMetadata: + fallback_metadata = DashboardMetadata(display_name=dashboard_folder.name) dashboard_path = dashboard_folder / "dashboard.yml" if not dashboard_path.exists(): - return fallback_configuration + return fallback_metadata with dashboard_path.open("r") as f: try: raw_configuration = yaml.safe_load(f) except yaml.YAMLError as e: logger.warning(f"Error '{e}' when parsing: {dashboard_path}") - return fallback_configuration + return fallback_metadata try: - return DashboardConfiguration.from_dict(raw_configuration) + return DashboardMetadata.from_dict(raw_configuration) except KeyError as e: logger.warning(f"Error '{e}' when parsing: {dashboard_path}") - return fallback_configuration + return fallback_metadata @staticmethod def _get_text_widget(path: Path) -> Widget: diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 1a511946..dd64da6c 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 DashboardConfiguration, Dashboards +from databricks.labs.lsql.dashboards import DashboardMetadata, Dashboards from databricks.labs.lsql.lakeview import ( CounterEncodingMap, CounterSpec, @@ -22,7 +22,7 @@ def test_dashboard_configuration_raises_key_error_if_display_name_is_missing(): with pytest.raises(KeyError): - DashboardConfiguration.from_dict({}) + DashboardMetadata.from_dict({}) def test_dashboards_saves_sql_files_to_folder(tmp_path): From 15ccb4317a5d8c139f4b81454e4a5af2964634d0 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:07:39 +0200 Subject: [PATCH 32/39] Format --- src/databricks/labs/lsql/dashboards.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index bf33d097..15b67720 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -121,7 +121,9 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: layouts.append(layout) page = Page( - name=dashboard_configuration.display_name, display_name=dashboard_configuration.display_name, layout=layouts + name=dashboard_configuration.display_name, + display_name=dashboard_configuration.display_name, + layout=layouts, ) lakeview_dashboard = Dashboard(datasets=datasets, pages=[page]) return lakeview_dashboard From 1751d755c2fd9163bb2a510f698e9f71d2cee70e Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:20:42 +0200 Subject: [PATCH 33/39] Use read text --- src/databricks/labs/lsql/dashboards.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 15b67720..ab0efbe5 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -136,14 +136,13 @@ def _get_dashboard_configuration(dashboard_folder: Path) -> DashboardMetadata: if not dashboard_path.exists(): return fallback_metadata - with dashboard_path.open("r") as f: - try: - raw_configuration = yaml.safe_load(f) - except yaml.YAMLError as e: - logger.warning(f"Error '{e}' when parsing: {dashboard_path}") - return fallback_metadata try: - return DashboardMetadata.from_dict(raw_configuration) + raw = yaml.safe_load(dashboard_path.read_text()) + except yaml.YAMLError as e: + logger.warning(f"Error '{e}' when parsing: {dashboard_path}") + return fallback_metadata + try: + return DashboardMetadata.from_dict(raw) except KeyError as e: logger.warning(f"Error '{e}' when parsing: {dashboard_path}") return fallback_metadata From 08da3595643a06dabe95266ad16226d4a010fbc6 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:21:02 +0200 Subject: [PATCH 34/39] Rename variable --- src/databricks/labs/lsql/dashboards.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index ab0efbe5..a2542cb2 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -132,19 +132,19 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: def _get_dashboard_configuration(dashboard_folder: Path) -> DashboardMetadata: fallback_metadata = DashboardMetadata(display_name=dashboard_folder.name) - dashboard_path = dashboard_folder / "dashboard.yml" - if not dashboard_path.exists(): + dashboard_metadata_path = dashboard_folder / "dashboard.yml" + if not dashboard_metadata_path.exists(): return fallback_metadata try: - raw = yaml.safe_load(dashboard_path.read_text()) + raw = yaml.safe_load(dashboard_metadata_path.read_text()) except yaml.YAMLError as e: - logger.warning(f"Error '{e}' when parsing: {dashboard_path}") + logger.warning(f"Error '{e}' when parsing: {dashboard_metadata_path}") return fallback_metadata try: return DashboardMetadata.from_dict(raw) except KeyError as e: - logger.warning(f"Error '{e}' when parsing: {dashboard_path}") + logger.warning(f"Error '{e}' when parsing: {dashboard_metadata_path}") return fallback_metadata @staticmethod From c2a1e860d48a5510bcd9873317eee877bd868dbc Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:21:41 +0200 Subject: [PATCH 35/39] Format log message --- src/databricks/labs/lsql/dashboards.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index a2542cb2..2e4f9db5 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -144,7 +144,7 @@ def _get_dashboard_configuration(dashboard_folder: Path) -> DashboardMetadata: try: return DashboardMetadata.from_dict(raw) except KeyError as e: - logger.warning(f"Error '{e}' when parsing: {dashboard_metadata_path}") + logger.warning(f"Parsing {dashboard_metadata_path}: {e}") return fallback_metadata @staticmethod From a829b6d45883693f0c43d8ca41cea8a496d38e5e Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:26:43 +0200 Subject: [PATCH 36/39] Test display name setting --- tests/unit/test_dashboards.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index dd64da6c..683db402 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -25,6 +25,11 @@ def test_dashboard_configuration_raises_key_error_if_display_name_is_missing(): 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_dashboards_saves_sql_files_to_folder(tmp_path): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" From b3eb4b71389316ce038306a5a9f19548713c674a Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:29:49 +0200 Subject: [PATCH 37/39] Add as dict --- src/databricks/labs/lsql/dashboards.py | 3 +++ tests/unit/test_dashboards.py | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 2e4f9db5..5e97c54e 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -41,6 +41,9 @@ def from_dict(cls, configuration: dict[str, str]) -> "DashboardMetadata": display_name=configuration["display_name"], ) + def as_dict(self) -> dict[str, str]: + return dataclasses.asdict(self) + class Dashboards: _MAXIMUM_DASHBOARD_WIDTH = 6 diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 683db402..043c885c 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -30,6 +30,12 @@ def test_dashboard_configuration_sets_display_name_from_dict(): 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" From 1c04e836984f7ae47da62ffbbe02cb9d1d351d35 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:46:50 +0200 Subject: [PATCH 38/39] Rename all configuration references to metadata --- src/databricks/labs/lsql/dashboards.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 5e97c54e..3f3cfafc 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -36,9 +36,9 @@ class DashboardMetadata: display_name: str @classmethod - def from_dict(cls, configuration: dict[str, str]) -> "DashboardMetadata": + def from_dict(cls, raw: dict[str, str]) -> "DashboardMetadata": return cls( - display_name=configuration["display_name"], + display_name=raw["display_name"], ) def as_dict(self) -> dict[str, str]: @@ -93,8 +93,8 @@ 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.""" - dashboard_configuration = self._get_dashboard_configuration(dashboard_folder) + """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 = [], [] @@ -124,15 +124,15 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: layouts.append(layout) page = Page( - name=dashboard_configuration.display_name, - display_name=dashboard_configuration.display_name, + 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 _get_dashboard_configuration(dashboard_folder: Path) -> DashboardMetadata: + def _parse_dashboard_metadata(dashboard_folder: Path) -> DashboardMetadata: fallback_metadata = DashboardMetadata(display_name=dashboard_folder.name) dashboard_metadata_path = dashboard_folder / "dashboard.yml" From 74ce3226b79736c52eaa5c2e74524d69a5d417b2 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:56:10 +0200 Subject: [PATCH 39/39] Fix parsing message --- src/databricks/labs/lsql/dashboards.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 3f3cfafc..3c39f611 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -142,7 +142,7 @@ def _parse_dashboard_metadata(dashboard_folder: Path) -> DashboardMetadata: try: raw = yaml.safe_load(dashboard_metadata_path.read_text()) except yaml.YAMLError as e: - logger.warning(f"Error '{e}' when parsing: {dashboard_metadata_path}") + logger.warning(f"Parsing {dashboard_metadata_path}: {e}") return fallback_metadata try: return DashboardMetadata.from_dict(raw)