From 8050e579959b36aa9f1f766d83375cbacce186fd Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:40:16 +0200 Subject: [PATCH 01/35] Add integration test for widget with config header --- tests/integration/test_dashboards.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index 3abb4803..f133973b 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -123,15 +123,13 @@ def test_dashboard_deploys_dashboard_with_counter_variation(ws, make_dashboard, assert ws.lakeview.get(sdk_dashboard.dashboard_id) -def test_dashboard_deploys_dashboard_with_big_widget(ws, make_dashboard, tmp_path): - sdk_dashboard = make_dashboard() - +def test_dashboard_deploys_dashboard_with_big_widget(ws, dashboard_id, tmp_path): query = """-- --width 6 --height 3\nSELECT 82917019218921 AS big_number_needs_big_widget""" - with (tmp_path / "counter.sql").open("w") as f: + with (tmp_path / f"counter.sql").open("w") as f: f.write(query) dashboards = Dashboards(ws) lakeview_dashboard = dashboards.create_dashboard(tmp_path) - sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) + sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=dashboard_id) assert ws.lakeview.get(sdk_dashboard.dashboard_id) From 1386c019cb5b135f18f4cbb802bbf279ca77644c Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:43:47 +0200 Subject: [PATCH 02/35] Add unit test for custom sized widget --- tests/unit/test_dashboards.py | 79 ++--------------------------------- 1 file changed, 3 insertions(+), 76 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 33217f2a..4fd1bc5e 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -367,19 +367,11 @@ def test_dashboards_creates_dashboards_where_text_widget_has_expected_text(tmp_p ws.assert_not_called() -@pytest.mark.parametrize( - "header", - [ - "-- --width 6 --height 3", - "/* --width 6 --height 3 */", - "/*\n--width 6\n--height 3 */", - ], -) -def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path, header): +def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path): ws = create_autospec(WorkspaceClient) - query = f"{header}\nSELECT 82917019218921 AS big_number_needs_big_widget" - with (tmp_path / "counter.sql").open("w") as f: + query = """-- --width 6 --height 3\nSELECT 82917019218921 AS big_number_needs_big_widget""" + with (tmp_path / f"counter.sql").open("w") as f: f.write(query) lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) @@ -390,71 +382,6 @@ def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path, header): ws.assert_not_called() -def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): - ws = create_autospec(WorkspaceClient) - - # Typo is on purpose - query = "-- --widh 6 --height 3 \nSELECT 82917019218921 AS big_number_needs_big_widget" - with (tmp_path / "counter.sql").open("w") as f: - f.write(query) - - with caplog.at_level(logging.WARNING, logger="databricks.labs.lsql.dashboards"): - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - - position = lakeview_dashboard.pages[0].layout[0].position - assert position.width == 1 - assert position.height == 3 - assert "--widh" in caplog.text - ws.assert_not_called() - - -@pytest.mark.parametrize( - "query", - [ - "-- --height 5\nSELECT 1 AS count -- --width 6", - "-- --height 5\nSELECT 1 AS count\n-- --width 6", - "-- --height 5\nSELECT 1 AS count\n/* --width 6 */", - "-- --height 5\n-- --width 6\nSELECT 1 AS count", - "-- --height 5\n/* --width 6 */\nSELECT 1 AS count", - "/* --height 5*/\n/* --width 6 */\nSELECT 1 AS count", - "/* --height 5*/\n-- --width 6 */\nSELECT 1 AS count", - ], -) -def test_dashboard_ignores_comment_on_other_lines(tmp_path, query): - ws = create_autospec(WorkspaceClient) - - with (tmp_path / "counter.sql").open("w") as f: - f.write(query) - - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - position = lakeview_dashboard.pages[0].layout[0].position - - assert position.width == 1 - assert position.height == 5 - ws.assert_not_called() - - -@pytest.mark.parametrize( - "query", - [ - "SELECT 1\n-- --width 6 --height 6", - "SELECT 1\n/*\n--width 6\n--height 6*/", - ], -) -def test_dashboard_ignores_non_header_comment(tmp_path, query): - ws = create_autospec(WorkspaceClient) - - with (tmp_path / "counter.sql").open("w") as f: - f.write(query) - - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - position = lakeview_dashboard.pages[0].layout[0].position - - assert position.width == 1 - assert position.height == 3 - ws.assert_not_called() - - def test_dashboards_deploy_calls_create_without_dashboard_id(): ws = create_autospec(WorkspaceClient) dashboards = Dashboards(ws) From 88550c2e9f5c4d299514c6638911149369f61fbf Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:51:53 +0200 Subject: [PATCH 03/35] Add flow for fallback metadata --- src/databricks/labs/lsql/dashboards.py | 60 +------------------------- 1 file changed, 1 insertion(+), 59 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index d3c39756..213392fe 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -53,29 +53,6 @@ class WidgetMetadata: width: int height: int - def as_dict(self) -> dict[str, str]: - return dataclasses.asdict(self) - - @staticmethod - def _get_arguments_parser() -> ArgumentParser: - parser = ArgumentParser("WidgetMetadata", add_help=False, exit_on_error=False) - parser.add_argument("-w", "--width", type=int) - parser.add_argument("-h", "--height", type=int) - return parser - - def replace_from_arguments(self, arguments: list[str]) -> "WidgetMetadata": - parser = self._get_arguments_parser() - try: - args = parser.parse_args(arguments) - except (argparse.ArgumentError, SystemExit) as e: - logger.warning(f"Parsing {arguments}: {e}") - return dataclasses.replace(self) - return dataclasses.replace( - self, - width=args.width or self.width, - height=args.height or self.height, - ) - class Dashboards: _MAXIMUM_DASHBOARD_WIDTH = 6 @@ -151,7 +128,7 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: continue else: widget = self._get_text_widget(path) - widget_metadata = self._parse_widget_metadata(path, widget) + widget_metadata = self._read_widget_metadata(path, widget) position = self._get_position(widget_metadata, position) layout = Layout(widget=widget, position=position) layouts.append(layout) @@ -164,41 +141,6 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: 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 - - def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: - width, height = self._get_width_and_height(widget) - fallback_metadata = WidgetMetadata(width, height) - - try: - parsed_query = sqlglot.parse_one(path.read_text(), dialect=sqlglot.dialects.Databricks) - except sqlglot.ParseError as e: - logger.warning(f"Parsing {path}: {e}") - return fallback_metadata - - if parsed_query.comments is None or len(parsed_query.comments) == 0: - return fallback_metadata - - first_comment = parsed_query.comments[0] - return fallback_metadata.replace_from_arguments(shlex.split(first_comment)) - @staticmethod def _get_text_widget(path: Path) -> Widget: widget = Widget(name=path.stem, textbox_spec=path.read_text()) From b0e59f47bca192bfc1ad949060d538201d495b2f Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 11:41:35 +0200 Subject: [PATCH 04/35] Rename method --- src/databricks/labs/lsql/dashboards.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 213392fe..e7211c33 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: continue else: widget = self._get_text_widget(path) - widget_metadata = self._read_widget_metadata(path, widget) + widget_metadata = self._parse_widget_metadata(path, widget) position = self._get_position(widget_metadata, position) layout = Layout(widget=widget, position=position) layouts.append(layout) @@ -141,6 +141,12 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: lakeview_dashboard = Dashboard(datasets=datasets, pages=[page]) return lakeview_dashboard + def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: + _ = path + width, height = self._get_width_and_height(widget) + fallback_metadata = WidgetMetadata(width, height) + return fallback_metadata + @staticmethod def _get_text_widget(path: Path) -> Widget: widget = Widget(name=path.stem, textbox_spec=path.read_text()) From 1b6fe9418733e2c39dcc063796654ef6c570741d Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 11:55:21 +0200 Subject: [PATCH 05/35] Format --- tests/integration/test_dashboards.py | 2 +- tests/unit/test_dashboards.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index f133973b..67ae818f 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -125,7 +125,7 @@ def test_dashboard_deploys_dashboard_with_counter_variation(ws, make_dashboard, def test_dashboard_deploys_dashboard_with_big_widget(ws, dashboard_id, tmp_path): query = """-- --width 6 --height 3\nSELECT 82917019218921 AS big_number_needs_big_widget""" - with (tmp_path / f"counter.sql").open("w") as f: + with (tmp_path / "counter.sql").open("w") as f: f.write(query) dashboards = Dashboards(ws) lakeview_dashboard = dashboards.create_dashboard(tmp_path) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 4fd1bc5e..e16a68c7 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -371,7 +371,7 @@ def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path): ws = create_autospec(WorkspaceClient) query = """-- --width 6 --height 3\nSELECT 82917019218921 AS big_number_needs_big_widget""" - with (tmp_path / f"counter.sql").open("w") as f: + with (tmp_path / "counter.sql").open("w") as f: f.write(query) lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) From c58b3d84b27dc667c343d471ce0fec3039a9e481 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 11:55:48 +0200 Subject: [PATCH 06/35] Parse flags from query header --- src/databricks/labs/lsql/dashboards.py | 29 +++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index e7211c33..c7fad4f2 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -53,6 +53,22 @@ class WidgetMetadata: width: int height: int + @staticmethod + def _get_arguments_parser() -> ArgumentParser: + parser = ArgumentParser("WidgetMetadata", add_help=False) + parser.add_argument("-w", "--width", type=int) + parser.add_argument("-h", "--height", type=int) + return parser + + def replace_from_arguments(self, arguments: list[str]) -> "WidgetMetadata": + parser = self._get_arguments_parser() + args = parser.parse_args(arguments) + return dataclasses.replace( + self, + width=args.width or self.width, + height=args.height or self.height, + ) + class Dashboards: _MAXIMUM_DASHBOARD_WIDTH = 6 @@ -145,7 +161,18 @@ def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: _ = path width, height = self._get_width_and_height(widget) fallback_metadata = WidgetMetadata(width, height) - return fallback_metadata + + try: + parsed_query = sqlglot.parse_one(path.read_text(), dialect=sqlglot.dialects.Databricks) + except sqlglot.ParseError as e: + logger.warning(f"Parsing {path}: {e}") + return fallback_metadata + + if len(parsed_query.comments) == 0: + return fallback_metadata + + comment = parsed_query.comments[0] + return fallback_metadata.replace_from_arguments(shlex.split(comment)) @staticmethod def _get_text_widget(path: Path) -> Widget: From 0182a1561a712e1e8a4d1d01040b7e8b04d287bf Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 12:02:11 +0200 Subject: [PATCH 07/35] Add unit test for replace WidgetMetadata from arguments --- tests/unit/test_dashboards.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index e16a68c7..777f2937 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -6,11 +6,7 @@ import pytest from databricks.sdk import WorkspaceClient -from databricks.labs.lsql.dashboards import ( - DashboardMetadata, - Dashboards, - WidgetMetadata, -) +from databricks.labs.lsql.dashboards import DashboardMetadata, Dashboards, WidgetMetadata from databricks.labs.lsql.lakeview import ( CounterEncodingMap, CounterSpec, @@ -58,12 +54,6 @@ def test_widget_metadata_replaces_one_attribute(attribute: str): assert all(getattr(updated_metadata, field.name) == 1 for field in other_fields) -def test_widget_metadata_as_dict(): - raw = {"width": 10, "height": 10} - widget_metadata = WidgetMetadata(10, 10) - assert widget_metadata.as_dict() == raw - - def test_dashboards_saves_sql_files_to_folder(tmp_path): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" From 957dc23aa7f9019dccfb8510dd9dfaeeec0317ed Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 12:07:01 +0200 Subject: [PATCH 08/35] Test for multiple headers --- tests/unit/test_dashboards.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 777f2937..cdee0acc 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -357,10 +357,18 @@ def test_dashboards_creates_dashboards_where_text_widget_has_expected_text(tmp_p ws.assert_not_called() -def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path): +@pytest.mark.parametrize( + "header", + [ + "-- --width 6 --height 3", + "/* --width 6 --height 3 */", + "/*\n--width 6\n--height 3 */", + ], +) +def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path, header): ws = create_autospec(WorkspaceClient) - query = """-- --width 6 --height 3\nSELECT 82917019218921 AS big_number_needs_big_widget""" + query = f"{header}\nSELECT 82917019218921 AS big_number_needs_big_widget" with (tmp_path / "counter.sql").open("w") as f: f.write(query) From 7e319cfdfa805b72486b27d7dea3ce8eb4a14996 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 12:18:44 +0200 Subject: [PATCH 09/35] Handle invalid header --- src/databricks/labs/lsql/dashboards.py | 8 ++++++-- tests/unit/test_dashboards.py | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index c7fad4f2..6ee33526 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -55,14 +55,18 @@ class WidgetMetadata: @staticmethod def _get_arguments_parser() -> ArgumentParser: - parser = ArgumentParser("WidgetMetadata", add_help=False) + parser = ArgumentParser("WidgetMetadata", add_help=False, exit_on_error=False) parser.add_argument("-w", "--width", type=int) parser.add_argument("-h", "--height", type=int) return parser def replace_from_arguments(self, arguments: list[str]) -> "WidgetMetadata": parser = self._get_arguments_parser() - args = parser.parse_args(arguments) + try: + args = parser.parse_args(arguments) + except (argparse.ArgumentError, SystemExit) as e: + logger.warning(f"Parsing {arguments}: {e}") + return dataclasses.replace(self) return dataclasses.replace( self, width=args.width or self.width, diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index cdee0acc..b2e28691 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -380,6 +380,24 @@ def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path, header): ws.assert_not_called() +def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): + ws = create_autospec(WorkspaceClient) + + # Typo is on purpose + query = f"-- --widh 6 --height 3 \nSELECT 82917019218921 AS big_number_needs_big_widget" + with (tmp_path / "counter.sql").open("w") as f: + f.write(query) + + with caplog.at_level(logging.WARNING, logger="databricks.labs.lsql.dashboards"): + lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + + position = lakeview_dashboard.pages[0].layout[0].position + assert position.width == 1 + assert position.height == 3 + assert "--widh" in caplog.text + ws.assert_not_called() + + def test_dashboards_deploy_calls_create_without_dashboard_id(): ws = create_autospec(WorkspaceClient) dashboards = Dashboards(ws) From 069eddcf1dc9b5431bad01f4a565f407a01af126 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 13:44:50 +0200 Subject: [PATCH 10/35] Add test for comment being on other line --- tests/unit/test_dashboards.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index b2e28691..1e84f964 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -398,6 +398,21 @@ def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): ws.assert_not_called() +def test_dashboard_ignores_comment_on_other_lines(tmp_path): + ws = create_autospec(WorkspaceClient) + + query = f"-- --height 5 \nSELECT 82917019218921 AS big_number_needs_big_widget -- --width 6" + with (tmp_path / "counter.sql").open("w") as f: + f.write(query) + + lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + position = lakeview_dashboard.pages[0].layout[0].position + + assert position.width == 1 + assert position.height == 5 + ws.assert_not_called() + + def test_dashboards_deploy_calls_create_without_dashboard_id(): ws = create_autospec(WorkspaceClient) dashboards = Dashboards(ws) From 9e66a9fff70e75737e770b6400558cb7665e5881 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 13:52:08 +0200 Subject: [PATCH 11/35] Test first line command only processed --- tests/unit/test_dashboards.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 1e84f964..049bdbe4 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -398,10 +398,21 @@ def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): ws.assert_not_called() -def test_dashboard_ignores_comment_on_other_lines(tmp_path): +@pytest.mark.parametrize( + "query", + [ + f"-- --height 5\nSELECT 1 AS count -- --width 6", + f"-- --height 5\nSELECT 1 AS count\n-- --width 6", + f"-- --height 5\nSELECT 1 AS count\n/* --width 6 */", + f"-- --height 5\n-- --width 6\nSELECT 1 AS count", + f"-- --height 5\n/* --width 6 */\nSELECT 1 AS count", + f"/* --height 5*/\n/* --width 6 */\nSELECT 1 AS count", + f"/* --height 5*/\n-- --width 6 */\nSELECT 1 AS count", + ] +) +def test_dashboard_ignores_comment_on_other_lines(tmp_path, query): ws = create_autospec(WorkspaceClient) - query = f"-- --height 5 \nSELECT 82917019218921 AS big_number_needs_big_widget -- --width 6" with (tmp_path / "counter.sql").open("w") as f: f.write(query) From 4a9112cf2fd255c61a2f7aec59aa2f4d672d7839 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 13:58:32 +0200 Subject: [PATCH 12/35] Test non-header comment --- tests/unit/test_dashboards.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 049bdbe4..07006856 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -424,6 +424,27 @@ def test_dashboard_ignores_comment_on_other_lines(tmp_path, query): ws.assert_not_called() +@pytest.mark.parametrize( + "query", + [ + "SELECT 1\n-- --width 6 --height 6", + "SELECT 1\n/*\n--width 6\n--height 6*/", + ] +) +def test_dashboard_ignores_non_header_comment(tmp_path, query): + ws = create_autospec(WorkspaceClient) + + with (tmp_path / "counter.sql").open("w") as f: + f.write(query) + + lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + position = lakeview_dashboard.pages[0].layout[0].position + + assert position.width == 1 + assert position.height == 3 + ws.assert_not_called() + + def test_dashboards_deploy_calls_create_without_dashboard_id(): ws = create_autospec(WorkspaceClient) dashboards = Dashboards(ws) From c32de8b245eae4188571b947cc466b07b223852f Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 14:02:48 +0200 Subject: [PATCH 13/35] Update docs on the widget arguments --- docs/dashboards.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/dashboards.md b/docs/dashboards.md index 76df1ca8..3772316b 100644 --- a/docs/dashboards.md +++ b/docs/dashboards.md @@ -80,8 +80,8 @@ when the metadata cannot be inferred from the query itself. ### Headers of SQL files The header is used to define widget and vizualization metadata used to render the relevant portion of the dashboard. -Metadata could be defined in a `--` or `/* ... */` comment, which are detected by our SQL parser. The parser only reads -the **comment starting on the top**, which can be a single line using `--` or span multiple lines +Metadata could be defined in `--` and `/* ... */` comments, which are detected by our SQL parser. The parser only reads +the **comment starting on the top** only, which can be a single line using `--` or span multiple lines using `/* ... */`. | Format | Readability | Verbosity | From 18751072b97a283489f3a8049dd1980951f6608b Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 14:06:43 +0200 Subject: [PATCH 14/35] Rename variable --- src/databricks/labs/lsql/dashboards.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 6ee33526..7ce11f80 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -175,8 +175,8 @@ def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: if len(parsed_query.comments) == 0: return fallback_metadata - comment = parsed_query.comments[0] - return fallback_metadata.replace_from_arguments(shlex.split(comment)) + first_comment = parsed_query.comments[0] + return fallback_metadata.replace_from_arguments(shlex.split(first_comment)) @staticmethod def _get_text_widget(path: Path) -> Widget: From 635a3c74f80ed50d31044c06d6f9fc7ec52faeaa Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 14:13:20 +0200 Subject: [PATCH 15/35] Format --- src/databricks/labs/lsql/dashboards.py | 2 +- tests/unit/test_dashboards.py | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 7ce11f80..5149d561 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -172,7 +172,7 @@ def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: logger.warning(f"Parsing {path}: {e}") return fallback_metadata - if len(parsed_query.comments) == 0: + if parsed_query.comments is None or len(parsed_query.comments) == 0: return fallback_metadata first_comment = parsed_query.comments[0] diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 07006856..3721b49d 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -384,7 +384,7 @@ def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): ws = create_autospec(WorkspaceClient) # Typo is on purpose - query = f"-- --widh 6 --height 3 \nSELECT 82917019218921 AS big_number_needs_big_widget" + query = "-- --widh 6 --height 3 \nSELECT 82917019218921 AS big_number_needs_big_widget" with (tmp_path / "counter.sql").open("w") as f: f.write(query) @@ -401,14 +401,14 @@ def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): @pytest.mark.parametrize( "query", [ - f"-- --height 5\nSELECT 1 AS count -- --width 6", - f"-- --height 5\nSELECT 1 AS count\n-- --width 6", - f"-- --height 5\nSELECT 1 AS count\n/* --width 6 */", - f"-- --height 5\n-- --width 6\nSELECT 1 AS count", - f"-- --height 5\n/* --width 6 */\nSELECT 1 AS count", - f"/* --height 5*/\n/* --width 6 */\nSELECT 1 AS count", - f"/* --height 5*/\n-- --width 6 */\nSELECT 1 AS count", - ] + "-- --height 5\nSELECT 1 AS count -- --width 6", + "-- --height 5\nSELECT 1 AS count\n-- --width 6", + "-- --height 5\nSELECT 1 AS count\n/* --width 6 */", + "-- --height 5\n-- --width 6\nSELECT 1 AS count", + "-- --height 5\n/* --width 6 */\nSELECT 1 AS count", + "/* --height 5*/\n/* --width 6 */\nSELECT 1 AS count", + "/* --height 5*/\n-- --width 6 */\nSELECT 1 AS count", + ], ) def test_dashboard_ignores_comment_on_other_lines(tmp_path, query): ws = create_autospec(WorkspaceClient) @@ -429,7 +429,7 @@ def test_dashboard_ignores_comment_on_other_lines(tmp_path, query): [ "SELECT 1\n-- --width 6 --height 6", "SELECT 1\n/*\n--width 6\n--height 6*/", - ] + ], ) def test_dashboard_ignores_non_header_comment(tmp_path, query): ws = create_autospec(WorkspaceClient) From 2780a9d82c0517e3650ed289bfd81afae6e540ff Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 14:52:22 +0200 Subject: [PATCH 16/35] Add as dict method --- src/databricks/labs/lsql/dashboards.py | 3 +++ tests/unit/test_dashboards.py | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 5149d561..d58aa8a1 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -53,6 +53,9 @@ class WidgetMetadata: width: int height: int + def as_dict(self) -> dict[str, str]: + return dataclasses.asdict(self) + @staticmethod def _get_arguments_parser() -> ArgumentParser: parser = ArgumentParser("WidgetMetadata", add_help=False, exit_on_error=False) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 3721b49d..20289daa 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -54,6 +54,11 @@ def test_widget_metadata_replaces_one_attribute(attribute: str): assert all(getattr(updated_metadata, field.name) == 1 for field in other_fields) +def test_widget_metadata_as_dict(): + raw = {"width": 10, "height": 10} + widget_metadata = WidgetMetadata(10, 10) + assert widget_metadata.as_dict() == raw + def test_dashboards_saves_sql_files_to_folder(tmp_path): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" From 58e6f02890190c14c430eb0c618ba07d25280caf Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 15:36:53 +0200 Subject: [PATCH 17/35] Format --- 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 20289daa..95c148b9 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -59,6 +59,7 @@ def test_widget_metadata_as_dict(): widget_metadata = WidgetMetadata(10, 10) assert widget_metadata.as_dict() == raw + def test_dashboards_saves_sql_files_to_folder(tmp_path): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" From 0f9c09bdfc0a26bfb2908571c2a4b3f1683c7055 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 17:35:04 +0200 Subject: [PATCH 18/35] Use make dashboard fixture --- tests/integration/test_dashboards.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index 67ae818f..3abb4803 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -123,13 +123,15 @@ def test_dashboard_deploys_dashboard_with_counter_variation(ws, make_dashboard, assert ws.lakeview.get(sdk_dashboard.dashboard_id) -def test_dashboard_deploys_dashboard_with_big_widget(ws, dashboard_id, tmp_path): +def test_dashboard_deploys_dashboard_with_big_widget(ws, make_dashboard, tmp_path): + sdk_dashboard = make_dashboard() + query = """-- --width 6 --height 3\nSELECT 82917019218921 AS big_number_needs_big_widget""" with (tmp_path / "counter.sql").open("w") as f: f.write(query) 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 da72b6b8d5b48e893fd342d10b304e2e78d94934 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 17:35:24 +0200 Subject: [PATCH 19/35] Set back parse dashboard metadata --- src/databricks/labs/lsql/dashboards.py | 19 +++++++++++++++++++ tests/unit/test_dashboards.py | 6 +++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index d58aa8a1..7debbf46 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -164,6 +164,25 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: 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 + def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: _ = path width, height = self._get_width_and_height(widget) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 95c148b9..33217f2a 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -6,7 +6,11 @@ import pytest from databricks.sdk import WorkspaceClient -from databricks.labs.lsql.dashboards import DashboardMetadata, Dashboards, WidgetMetadata +from databricks.labs.lsql.dashboards import ( + DashboardMetadata, + Dashboards, + WidgetMetadata, +) from databricks.labs.lsql.lakeview import ( CounterEncodingMap, CounterSpec, From f0f5b15151eca9fb77a746250c7819c522909881 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 17:36:50 +0200 Subject: [PATCH 20/35] Fix wording --- docs/dashboards.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/dashboards.md b/docs/dashboards.md index 3772316b..76df1ca8 100644 --- a/docs/dashboards.md +++ b/docs/dashboards.md @@ -80,8 +80,8 @@ when the metadata cannot be inferred from the query itself. ### Headers of SQL files The header is used to define widget and vizualization metadata used to render the relevant portion of the dashboard. -Metadata could be defined in `--` and `/* ... */` comments, which are detected by our SQL parser. The parser only reads -the **comment starting on the top** only, which can be a single line using `--` or span multiple lines +Metadata could be defined in a `--` or `/* ... */` comment, which are detected by our SQL parser. The parser only reads +the **comment starting on the top**, which can be a single line using `--` or span multiple lines using `/* ... */`. | Format | Readability | Verbosity | From 2ff3432eee2c8aed232c284f8839a0d7bb5430ed Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 17:37:50 +0200 Subject: [PATCH 21/35] Fix unused argument --- src/databricks/labs/lsql/dashboards.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 7debbf46..d3c39756 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -184,7 +184,6 @@ def _parse_dashboard_metadata(dashboard_folder: Path) -> DashboardMetadata: return fallback_metadata def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: - _ = path width, height = self._get_width_and_height(widget) fallback_metadata = WidgetMetadata(width, height) From 3f8d25780841a05e9870d55ba545f3808ae78880 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:40:16 +0200 Subject: [PATCH 22/35] Add integration test for widget with config header --- tests/integration/test_dashboards.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index 3abb4803..170dbbad 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -135,3 +135,15 @@ def test_dashboard_deploys_dashboard_with_big_widget(ws, make_dashboard, tmp_pat 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_big_widget(ws, dashboard_id, tmp_path): + query = """-- --width 6 --height 3\nSELECT 82917019218921 AS big_number_needs_big_widget""" + with (tmp_path / f"counter.sql").open("w") as f: + f.write(query) + dashboards = Dashboards(ws) + lakeview_dashboard = dashboards.create_dashboard(tmp_path) + + sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=dashboard_id) + + assert ws.lakeview.get(sdk_dashboard.dashboard_id) From 8dd76df83d3fbce88526db3d0262ae4def750dd2 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:43:47 +0200 Subject: [PATCH 23/35] Add unit test for custom sized widget --- tests/unit/test_dashboards.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 33217f2a..6d7ebd74 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -367,15 +367,22 @@ def test_dashboards_creates_dashboards_where_text_widget_has_expected_text(tmp_p ws.assert_not_called() -@pytest.mark.parametrize( - "header", - [ - "-- --width 6 --height 3", - "/* --width 6 --height 3 */", - "/*\n--width 6\n--height 3 */", - ], -) -def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path, header): +def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path): + ws = create_autospec(WorkspaceClient) + + query = """-- --width 6 --height 3\nSELECT 82917019218921 AS big_number_needs_big_widget""" + with (tmp_path / f"counter.sql").open("w") as f: + f.write(query) + + lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + position = lakeview_dashboard.pages[0].layout[0].position + + assert position.width == 6 + assert position.height == 3 + ws.assert_not_called() + + +def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dashboard_id(): ws = create_autospec(WorkspaceClient) query = f"{header}\nSELECT 82917019218921 AS big_number_needs_big_widget" From c1f958f6fd14ea9a19b6a8a46ed2399d2026d924 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 10:51:53 +0200 Subject: [PATCH 24/35] Add flow for fallback metadata --- src/databricks/labs/lsql/dashboards.py | 46 ++++---------------------- 1 file changed, 6 insertions(+), 40 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index d3c39756..e6ab54d4 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -2,8 +2,6 @@ import dataclasses import json import logging -import shlex -from argparse import ArgumentParser from dataclasses import dataclass from pathlib import Path from typing import TypeVar @@ -34,48 +32,11 @@ 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) - - @dataclass class WidgetMetadata: width: int height: int - def as_dict(self) -> dict[str, str]: - return dataclasses.asdict(self) - - @staticmethod - def _get_arguments_parser() -> ArgumentParser: - parser = ArgumentParser("WidgetMetadata", add_help=False, exit_on_error=False) - parser.add_argument("-w", "--width", type=int) - parser.add_argument("-h", "--height", type=int) - return parser - - def replace_from_arguments(self, arguments: list[str]) -> "WidgetMetadata": - parser = self._get_arguments_parser() - try: - args = parser.parse_args(arguments) - except (argparse.ArgumentError, SystemExit) as e: - logger.warning(f"Parsing {arguments}: {e}") - return dataclasses.replace(self) - return dataclasses.replace( - self, - width=args.width or self.width, - height=args.height or self.height, - ) - class Dashboards: _MAXIMUM_DASHBOARD_WIDTH = 6 @@ -151,7 +112,7 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: continue else: widget = self._get_text_widget(path) - widget_metadata = self._parse_widget_metadata(path, widget) + widget_metadata = self._read_widget_metadata(path, widget) position = self._get_position(widget_metadata, position) layout = Layout(widget=widget, position=position) layouts.append(layout) @@ -164,6 +125,11 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: lakeview_dashboard = Dashboard(datasets=datasets, pages=[page]) return lakeview_dashboard + def _read_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: + width, height = self._get_width_and_height(widget) + fallback_metadata = WidgetMetadata(width, height) + return fallback_metadata + @staticmethod def _parse_dashboard_metadata(dashboard_folder: Path) -> DashboardMetadata: fallback_metadata = DashboardMetadata(display_name=dashboard_folder.name) From f6999d6a496405dfec793bcdb708a326a5dd434d Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 11:41:35 +0200 Subject: [PATCH 25/35] Rename method --- src/databricks/labs/lsql/dashboards.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index e6ab54d4..f5c66c88 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -112,7 +112,7 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: continue else: widget = self._get_text_widget(path) - widget_metadata = self._read_widget_metadata(path, widget) + widget_metadata = self._parse_widget_metadata(path, widget) position = self._get_position(widget_metadata, position) layout = Layout(widget=widget, position=position) layouts.append(layout) @@ -125,7 +125,7 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: lakeview_dashboard = Dashboard(datasets=datasets, pages=[page]) return lakeview_dashboard - def _read_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: + def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: width, height = self._get_width_and_height(widget) fallback_metadata = WidgetMetadata(width, height) return fallback_metadata From c7f88fc07ea222b85400cbeeba15e81cb45af59e Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 11:55:48 +0200 Subject: [PATCH 26/35] Parse flags from query header --- src/databricks/labs/lsql/dashboards.py | 31 +++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index f5c66c88..39ec78cd 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -2,6 +2,8 @@ import dataclasses import json import logging +import shlex +from argparse import ArgumentParser from dataclasses import dataclass from pathlib import Path from typing import TypeVar @@ -37,6 +39,22 @@ class WidgetMetadata: width: int height: int + @staticmethod + def _get_arguments_parser() -> ArgumentParser: + parser = ArgumentParser("WidgetMetadata", add_help=False) + parser.add_argument("-w", "--width", type=int) + parser.add_argument("-h", "--height", type=int) + return parser + + def replace_from_arguments(self, arguments: list[str]) -> "WidgetMetadata": + parser = self._get_arguments_parser() + args = parser.parse_args(arguments) + return dataclasses.replace( + self, + width=args.width or self.width, + height=args.height or self.height, + ) + class Dashboards: _MAXIMUM_DASHBOARD_WIDTH = 6 @@ -128,7 +146,18 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: width, height = self._get_width_and_height(widget) fallback_metadata = WidgetMetadata(width, height) - return fallback_metadata + + try: + parsed_query = sqlglot.parse_one(path.read_text(), dialect=sqlglot.dialects.Databricks) + except sqlglot.ParseError as e: + logger.warning(f"Parsing {path}: {e}") + return fallback_metadata + + if len(parsed_query.comments) == 0: + return fallback_metadata + + comment = parsed_query.comments[0] + return fallback_metadata.replace_from_arguments(shlex.split(comment)) @staticmethod def _parse_dashboard_metadata(dashboard_folder: Path) -> DashboardMetadata: From c1a5a3b38cd843d4e33e53b6e9686a1302a6475b Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 12:02:11 +0200 Subject: [PATCH 27/35] Add unit test for replace WidgetMetadata from arguments --- tests/unit/test_dashboards.py | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 6d7ebd74..b33e2f35 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -6,11 +6,7 @@ import pytest from databricks.sdk import WorkspaceClient -from databricks.labs.lsql.dashboards import ( - DashboardMetadata, - Dashboards, - WidgetMetadata, -) +from databricks.labs.lsql.dashboards import Dashboards, WidgetMetadata from databricks.labs.lsql.lakeview import ( CounterEncodingMap, CounterSpec, @@ -25,22 +21,6 @@ ) -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_widget_metadata_replaces_arguments(): widget_metadata = WidgetMetadata(1, 1) updated_metadata = widget_metadata.replace_from_arguments(["--width", "10", "--height", "10"]) @@ -58,12 +38,6 @@ def test_widget_metadata_replaces_one_attribute(attribute: str): assert all(getattr(updated_metadata, field.name) == 1 for field in other_fields) -def test_widget_metadata_as_dict(): - raw = {"width": 10, "height": 10} - widget_metadata = WidgetMetadata(10, 10) - assert widget_metadata.as_dict() == raw - - def test_dashboards_saves_sql_files_to_folder(tmp_path): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" From 4ab4b487b5f13411195f8269c27a98200c543f78 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 12:18:44 +0200 Subject: [PATCH 28/35] Handle invalid header --- src/databricks/labs/lsql/dashboards.py | 8 ++++++-- tests/unit/test_dashboards.py | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 39ec78cd..6fd6270b 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -41,14 +41,18 @@ class WidgetMetadata: @staticmethod def _get_arguments_parser() -> ArgumentParser: - parser = ArgumentParser("WidgetMetadata", add_help=False) + parser = ArgumentParser("WidgetMetadata", add_help=False, exit_on_error=False) parser.add_argument("-w", "--width", type=int) parser.add_argument("-h", "--height", type=int) return parser def replace_from_arguments(self, arguments: list[str]) -> "WidgetMetadata": parser = self._get_arguments_parser() - args = parser.parse_args(arguments) + try: + args = parser.parse_args(arguments) + except (argparse.ArgumentError, SystemExit) as e: + logger.warning(f"Parsing {arguments}: {e}") + return dataclasses.replace(self) return dataclasses.replace( self, width=args.width or self.width, diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index b33e2f35..4475a130 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -374,6 +374,24 @@ def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dash def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): ws = create_autospec(WorkspaceClient) + # Typo is on purpose + query = f"-- --widh 6 --height 3 \nSELECT 82917019218921 AS big_number_needs_big_widget" + with (tmp_path / "counter.sql").open("w") as f: + f.write(query) + + with caplog.at_level(logging.WARNING, logger="databricks.labs.lsql.dashboards"): + lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + + position = lakeview_dashboard.pages[0].layout[0].position + assert position.width == 1 + assert position.height == 3 + assert "--widh" in caplog.text + ws.assert_not_called() + + +def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dashboard_id(): + ws = create_autospec(WorkspaceClient) + # Typo is on purpose query = "-- --widh 6 --height 3 \nSELECT 82917019218921 AS big_number_needs_big_widget" with (tmp_path / "counter.sql").open("w") as f: From 307c77bf19d834610d88840cd2cc32332b9e1ba2 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 13:44:50 +0200 Subject: [PATCH 29/35] Add test for comment being on other line --- tests/unit/test_dashboards.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 4475a130..f2f4f670 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -389,6 +389,21 @@ def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): ws.assert_not_called() +def test_dashboard_ignores_comment_on_other_lines(tmp_path): + ws = create_autospec(WorkspaceClient) + + query = f"-- --height 5 \nSELECT 82917019218921 AS big_number_needs_big_widget -- --width 6" + with (tmp_path / "counter.sql").open("w") as f: + f.write(query) + + lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + position = lakeview_dashboard.pages[0].layout[0].position + + assert position.width == 1 + assert position.height == 5 + ws.assert_not_called() + + def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dashboard_id(): ws = create_autospec(WorkspaceClient) From c2614defcbe9aa5ffe28df09550515c99f1861f5 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 13:58:32 +0200 Subject: [PATCH 30/35] Test non-header comment --- tests/unit/test_dashboards.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index f2f4f670..1af64a98 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -453,7 +453,7 @@ def test_dashboard_ignores_comment_on_other_lines(tmp_path, query): [ "SELECT 1\n-- --width 6 --height 6", "SELECT 1\n/*\n--width 6\n--height 6*/", - ], + ] ) def test_dashboard_ignores_non_header_comment(tmp_path, query): ws = create_autospec(WorkspaceClient) @@ -469,6 +469,20 @@ def test_dashboard_ignores_non_header_comment(tmp_path, query): ws.assert_not_called() +def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dashboard_id(): + ws = create_autospec(WorkspaceClient) + + with (tmp_path / "counter.sql").open("w") as f: + f.write(query) + + lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + position = lakeview_dashboard.pages[0].layout[0].position + + assert position.width == 1 + assert position.height == 3 + ws.assert_not_called() + + def test_dashboards_deploy_calls_create_without_dashboard_id(): ws = create_autospec(WorkspaceClient) dashboards = Dashboards(ws) From 604acce57c59ec88ca98ad7bb6b5c748f8623d0d Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 14:37:10 +0200 Subject: [PATCH 31/35] Implement widget ordering --- src/databricks/labs/lsql/dashboards.py | 26 +++++++++++++++++--------- tests/unit/test_dashboards.py | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 6fd6270b..4d147a68 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -36,12 +36,14 @@ @dataclass class WidgetMetadata: + order: int width: int height: int @staticmethod def _get_arguments_parser() -> ArgumentParser: parser = ArgumentParser("WidgetMetadata", add_help=False, exit_on_error=False) + parser.add_argument("-o", "--order", type=int) parser.add_argument("-w", "--width", type=int) parser.add_argument("-h", "--height", type=int) return parser @@ -55,6 +57,7 @@ def replace_from_arguments(self, arguments: list[str]) -> "WidgetMetadata": return dataclasses.replace(self) return dataclasses.replace( self, + order=args.order or self.order, width=args.width or self.width, height=args.height or self.height, ) @@ -108,10 +111,7 @@ def _format_query(query: str) -> str: return formatted_query def create_dashboard(self, dashboard_folder: Path) -> Dashboard: - """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 + """Create a dashboard from code, i.e. configuration and queries.""" datasets, layouts = [], [] for query_path in sorted(dashboard_folder.glob("*.sql")): with query_path.open("r") as query_file: @@ -119,8 +119,8 @@ 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()): + dataset_index, widgets = 0, [] + for order, path in enumerate(sorted(dashboard_folder.iterdir())): if path.suffix not in {".sql", ".md"}: continue if path.suffix == ".sql": @@ -134,7 +134,11 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: continue else: widget = self._get_text_widget(path) - widget_metadata = self._parse_widget_metadata(path, widget) + widget_metadata = self._parse_widget_metadata(path, widget, order) + widgets.append((widget, widget_metadata)) + + position = Position(0, 0, 0, 0) # First widget position + for widget, widget_metadata in sorted(widgets, key=lambda w: (w[1].order, w[0].name)): position = self._get_position(widget_metadata, position) layout = Layout(widget=widget, position=position) layouts.append(layout) @@ -147,9 +151,13 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: lakeview_dashboard = Dashboard(datasets=datasets, pages=[page]) return lakeview_dashboard - def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: + def _parse_widget_metadata(self, path: Path, widget: Widget, order: int) -> WidgetMetadata: width, height = self._get_width_and_height(widget) - fallback_metadata = WidgetMetadata(width, height) + fallback_metadata = WidgetMetadata( + order=order, + width=width, + height=height, + ) try: parsed_query = sqlglot.parse_one(path.read_text(), dialect=sqlglot.dialects.Databricks) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 1af64a98..aef0a3c4 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -298,6 +298,25 @@ def test_dashboards_creates_dashboards_with_widgets_sorted_alphanumerically(tmp_ ws.assert_not_called() +def test_dashboards_creates_dashboards_with_widgets_order_overwrite(tmp_path): + ws = create_autospec(WorkspaceClient) + + for query_name in "abcdf": + with (tmp_path / f"{query_name}.sql").open("w") as f: + f.write("SELECT 1 AS count") + + # Move the 'e' inbetween 'b' and 'c' query. Note that the order 1 puts 'e' on the same position as 'b', but with an + # order tiebreaker the query name decides the final order. + with (tmp_path / "e.sql").open("w") as f: + f.write("-- --order 1\nSELECT 1 AS count") + + lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] + + assert "".join(widget_names) == "abecdf" + ws.assert_not_called() + + @pytest.mark.parametrize("query, width, height", [("SELECT 1 AS count", 1, 3)]) def test_dashboards_creates_dashboards_where_widget_has_expected_width_and_height(tmp_path, query, width, height): ws = create_autospec(WorkspaceClient) From f14af8856724c1b3210d728ec60c19d02501d0b2 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 14:38:37 +0200 Subject: [PATCH 32/35] Fix unit tests --- 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 aef0a3c4..09245012 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -22,15 +22,15 @@ def test_widget_metadata_replaces_arguments(): - widget_metadata = WidgetMetadata(1, 1) + widget_metadata = WidgetMetadata(1, 1, 1) updated_metadata = widget_metadata.replace_from_arguments(["--width", "10", "--height", "10"]) assert updated_metadata.width == 10 assert updated_metadata.height == 10 -@pytest.mark.parametrize("attribute", ["width", "height"]) +@pytest.mark.parametrize("attribute", ["order", "width", "height"]) def test_widget_metadata_replaces_one_attribute(attribute: str): - widget_metadata = WidgetMetadata(1, 1) + widget_metadata = WidgetMetadata(1, 1, 1) updated_metadata = widget_metadata.replace_from_arguments([f"--{attribute}", "10"]) other_fields = [field for field in dataclasses.fields(updated_metadata) if field.name != attribute] From b2682e7108bb55838fda08de6b2145bd88eb52b7 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 14:49:35 +0200 Subject: [PATCH 33/35] Add integration test for widget ordering --- tests/integration/test_dashboards.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index 170dbbad..31cf31c0 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -147,3 +147,21 @@ def test_dashboard_deploys_dashboard_with_big_widget(ws, dashboard_id, tmp_path) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=dashboard_id) assert ws.lakeview.get(sdk_dashboard.dashboard_id) + + +def test_dashboards_deploys_dashboard_with_order_overwrite(ws, dashboard_id, tmp_path): + for query_name in range(6): + with (tmp_path / f"{query_name}.sql").open("w") as f: + f.write(f"SELECT {query_name} AS count") + + # Move the '4' inbetween '1' and '2' query. Note that the order 1 puts '4' on the same position as '1', but with an + # order tiebreaker the query name decides the final order. + with (tmp_path / "4.sql").open("w") as f: + f.write("-- --order 1\nSELECT 4 AS count") + + dashboards = Dashboards(ws) + lakeview_dashboard = dashboards.create_dashboard(tmp_path) + + sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=dashboard_id) + + assert ws.lakeview.get(sdk_dashboard.dashboard_id) From 989d2fe86f6321bf81c70b4a0ee5a664dbc49793 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 17:57:57 +0200 Subject: [PATCH 34/35] Fix rebase problems --- src/databricks/labs/lsql/dashboards.py | 83 +++++++++++--------- tests/integration/test_dashboards.py | 16 +--- tests/unit/test_dashboards.py | 102 +++++++++---------------- 3 files changed, 88 insertions(+), 113 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 4d147a68..1d093bf7 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -4,6 +4,7 @@ import logging import shlex from argparse import ArgumentParser +from collections.abc import Iterable from dataclasses import dataclass from pathlib import Path from typing import TypeVar @@ -34,12 +35,29 @@ 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) + + @dataclass class WidgetMetadata: order: int width: int height: int + def as_dict(self) -> dict[str, str]: + return dataclasses.asdict(self) + @staticmethod def _get_arguments_parser() -> ArgumentParser: parser = ArgumentParser("WidgetMetadata", add_help=False, exit_on_error=False) @@ -112,15 +130,31 @@ def _format_query(query: str) -> str: def create_dashboard(self, dashboard_folder: Path) -> Dashboard: """Create a dashboard from code, i.e. configuration and queries.""" - datasets, layouts = [], [] + dashboard_metadata = self._parse_dashboard_metadata(dashboard_folder) + datasets = self._get_datasets(dashboard_folder) + widgets = self._get_widgets(dashboard_folder.iterdir(), datasets) + layouts = self._get_layouts(widgets) + 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 _get_datasets(dashboard_folder: Path) -> list[Dataset]: + datasets = [] for query_path in sorted(dashboard_folder.glob("*.sql")): with query_path.open("r") as query_file: raw_query = query_file.read() dataset = Dataset(name=query_path.stem, display_name=query_path.stem, query=raw_query) datasets.append(dataset) + return datasets + def _get_widgets(self, files: Iterable[Path], datasets: list[Dataset]) -> list[tuple[Widget, WidgetMetadata]]: dataset_index, widgets = 0, [] - for order, path in enumerate(sorted(dashboard_folder.iterdir())): + for order, path in enumerate(sorted(files)): if path.suffix not in {".sql", ".md"}: continue if path.suffix == ".sql": @@ -136,40 +170,15 @@ def create_dashboard(self, dashboard_folder: Path) -> Dashboard: widget = self._get_text_widget(path) widget_metadata = self._parse_widget_metadata(path, widget, order) widgets.append((widget, widget_metadata)) + return widgets - position = Position(0, 0, 0, 0) # First widget position + def _get_layouts(self, widgets: list[tuple[Widget, WidgetMetadata]]) -> list[Layout]: + layouts, position = [], Position(0, 0, 0, 0) # First widget position for widget, widget_metadata in sorted(widgets, key=lambda w: (w[1].order, w[0].name)): position = self._get_position(widget_metadata, position) layout = Layout(widget=widget, position=position) layouts.append(layout) - - 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 - - def _parse_widget_metadata(self, path: Path, widget: Widget, order: int) -> WidgetMetadata: - width, height = self._get_width_and_height(widget) - fallback_metadata = WidgetMetadata( - order=order, - width=width, - height=height, - ) - - try: - parsed_query = sqlglot.parse_one(path.read_text(), dialect=sqlglot.dialects.Databricks) - except sqlglot.ParseError as e: - logger.warning(f"Parsing {path}: {e}") - return fallback_metadata - - if len(parsed_query.comments) == 0: - return fallback_metadata - - comment = parsed_query.comments[0] - return fallback_metadata.replace_from_arguments(shlex.split(comment)) + return layouts @staticmethod def _parse_dashboard_metadata(dashboard_folder: Path) -> DashboardMetadata: @@ -190,9 +199,13 @@ def _parse_dashboard_metadata(dashboard_folder: Path) -> DashboardMetadata: logger.warning(f"Parsing {dashboard_metadata_path}: {e}") return fallback_metadata - def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: + def _parse_widget_metadata(self, path: Path, widget: Widget, order: int) -> WidgetMetadata: width, height = self._get_width_and_height(widget) - fallback_metadata = WidgetMetadata(width, height) + fallback_metadata = WidgetMetadata( + order=order, + width=width, + height=height, + ) try: parsed_query = sqlglot.parse_one(path.read_text(), dialect=sqlglot.dialects.Databricks) @@ -203,8 +216,8 @@ def _parse_widget_metadata(self, path: Path, widget: Widget) -> WidgetMetadata: if parsed_query.comments is None or len(parsed_query.comments) == 0: return fallback_metadata - first_comment = parsed_query.comments[0] - return fallback_metadata.replace_from_arguments(shlex.split(first_comment)) + comment = parsed_query.comments[0] + return fallback_metadata.replace_from_arguments(shlex.split(comment)) @staticmethod def _get_text_widget(path: Path) -> Widget: diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index 31cf31c0..21452fff 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -137,19 +137,9 @@ def test_dashboard_deploys_dashboard_with_big_widget(ws, make_dashboard, tmp_pat assert ws.lakeview.get(sdk_dashboard.dashboard_id) -def test_dashboard_deploys_dashboard_with_big_widget(ws, dashboard_id, tmp_path): - query = """-- --width 6 --height 3\nSELECT 82917019218921 AS big_number_needs_big_widget""" - with (tmp_path / f"counter.sql").open("w") as f: - f.write(query) - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) - - sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=dashboard_id) - - assert ws.lakeview.get(sdk_dashboard.dashboard_id) - +def test_dashboards_deploys_dashboard_with_order_overwrite(ws, make_dashboard, tmp_path): + sdk_dashboard = make_dashboard() -def test_dashboards_deploys_dashboard_with_order_overwrite(ws, dashboard_id, tmp_path): for query_name in range(6): with (tmp_path / f"{query_name}.sql").open("w") as f: f.write(f"SELECT {query_name} AS count") @@ -162,6 +152,6 @@ def test_dashboards_deploys_dashboard_with_order_overwrite(ws, dashboard_id, tmp 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 09245012..103b72c5 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -6,7 +6,11 @@ import pytest from databricks.sdk import WorkspaceClient -from databricks.labs.lsql.dashboards import Dashboards, WidgetMetadata +from databricks.labs.lsql.dashboards import ( + DashboardMetadata, + Dashboards, + WidgetMetadata, +) from databricks.labs.lsql.lakeview import ( CounterEncodingMap, CounterSpec, @@ -21,6 +25,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_widget_metadata_replaces_arguments(): widget_metadata = WidgetMetadata(1, 1, 1) updated_metadata = widget_metadata.replace_from_arguments(["--width", "10", "--height", "10"]) @@ -38,6 +58,12 @@ def test_widget_metadata_replaces_one_attribute(attribute: str): assert all(getattr(updated_metadata, field.name) == 1 for field in other_fields) +def test_widget_metadata_as_dict(): + raw = {"order": 10, "width": 10, "height": 10} + widget_metadata = WidgetMetadata(10, 10, 10) + assert widget_metadata.as_dict() == raw + + def test_dashboards_saves_sql_files_to_folder(tmp_path): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" @@ -360,22 +386,15 @@ def test_dashboards_creates_dashboards_where_text_widget_has_expected_text(tmp_p ws.assert_not_called() -def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path): - ws = create_autospec(WorkspaceClient) - - query = """-- --width 6 --height 3\nSELECT 82917019218921 AS big_number_needs_big_widget""" - with (tmp_path / f"counter.sql").open("w") as f: - f.write(query) - - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - position = lakeview_dashboard.pages[0].layout[0].position - - assert position.width == 6 - assert position.height == 3 - ws.assert_not_called() - - -def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dashboard_id(): +@pytest.mark.parametrize( + "header", + [ + "-- --width 6 --height 3", + "/* --width 6 --height 3 */", + "/*\n--width 6\n--height 3 */", + ], +) +def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path, header): ws = create_autospec(WorkspaceClient) query = f"{header}\nSELECT 82917019218921 AS big_number_needs_big_widget" @@ -393,39 +412,6 @@ def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dash def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): ws = create_autospec(WorkspaceClient) - # Typo is on purpose - query = f"-- --widh 6 --height 3 \nSELECT 82917019218921 AS big_number_needs_big_widget" - with (tmp_path / "counter.sql").open("w") as f: - f.write(query) - - with caplog.at_level(logging.WARNING, logger="databricks.labs.lsql.dashboards"): - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - - position = lakeview_dashboard.pages[0].layout[0].position - assert position.width == 1 - assert position.height == 3 - assert "--widh" in caplog.text - ws.assert_not_called() - - -def test_dashboard_ignores_comment_on_other_lines(tmp_path): - ws = create_autospec(WorkspaceClient) - - query = f"-- --height 5 \nSELECT 82917019218921 AS big_number_needs_big_widget -- --width 6" - with (tmp_path / "counter.sql").open("w") as f: - f.write(query) - - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - position = lakeview_dashboard.pages[0].layout[0].position - - assert position.width == 1 - assert position.height == 5 - ws.assert_not_called() - - -def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dashboard_id(): - ws = create_autospec(WorkspaceClient) - # Typo is on purpose query = "-- --widh 6 --height 3 \nSELECT 82917019218921 AS big_number_needs_big_widget" with (tmp_path / "counter.sql").open("w") as f: @@ -472,7 +458,7 @@ def test_dashboard_ignores_comment_on_other_lines(tmp_path, query): [ "SELECT 1\n-- --width 6 --height 6", "SELECT 1\n/*\n--width 6\n--height 6*/", - ] + ], ) def test_dashboard_ignores_non_header_comment(tmp_path, query): ws = create_autospec(WorkspaceClient) @@ -488,20 +474,6 @@ def test_dashboard_ignores_non_header_comment(tmp_path, query): ws.assert_not_called() -def test_dashboards_deploy_raises_value_error_with_missing_display_name_and_dashboard_id(): - ws = create_autospec(WorkspaceClient) - - with (tmp_path / "counter.sql").open("w") as f: - f.write(query) - - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - position = lakeview_dashboard.pages[0].layout[0].position - - assert position.width == 1 - assert position.height == 3 - ws.assert_not_called() - - def test_dashboards_deploy_calls_create_without_dashboard_id(): ws = create_autospec(WorkspaceClient) dashboards = Dashboards(ws) From 2753d391243cb9c81234c1401aeb5bb481e47e8d Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 12 Jun 2024 21:48:13 +0200 Subject: [PATCH 35/35] Rename variable --- src/databricks/labs/lsql/dashboards.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 1d093bf7..9a41e2f4 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -216,8 +216,8 @@ def _parse_widget_metadata(self, path: Path, widget: Widget, order: int) -> Widg if parsed_query.comments is None or len(parsed_query.comments) == 0: return fallback_metadata - comment = parsed_query.comments[0] - return fallback_metadata.replace_from_arguments(shlex.split(comment)) + first_comment = parsed_query.comments[0] + return fallback_metadata.replace_from_arguments(shlex.split(first_comment)) @staticmethod def _get_text_widget(path: Path) -> Widget: