From 06ee2e918f4e61f706e70d7a4c1e7ccccb5bbb14 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 11:10:10 +0200 Subject: [PATCH 01/18] Specify which database to replace in query --- src/databricks/labs/lsql/dashboards.py | 27 +++++++++++++++++++++++--- tests/unit/test_dashboards.py | 25 ++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index bcd0d5df..d5281f7c 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -355,9 +355,30 @@ class MarkdownTile(Tile): _position: Position = Position(0, 0, _MAXIMUM_DASHBOARD_WIDTH, 3) -def replace_database_in_query(node: sqlglot.Expression, *, database: str) -> sqlglot.Expression: - """Replace the database in a query.""" - if isinstance(node, sqlglot.exp.Table) and node.args.get("db") is not None: +def replace_database_in_query( + node: sqlglot.Expression, + *, + database: str, + database_to_replace: str | None = None, +) -> sqlglot.Expression: + """Replace the database in a query. + + Parameters : + node : sqlglot.Expression + The parsed sql query + database : str + The value to replace the database with + database_to_replace : str | None (default: None) + The database to replace, if None, all databases are replaced + + Returns : + The query with the database replaced + """ + if ( + isinstance(node, sqlglot.exp.Table) + and node.args.get("db") is not None + and (database_to_replace is None or getattr(node.args.get("db"), "this", "") == database_to_replace) + ): node.args["db"].set("this", database) return node diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 2f6732b6..d02aaffd 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -762,6 +762,31 @@ def test_query_tile_creates_database_with_database_overwrite(tmp_path, query, qu assert dataset.query == sqlglot.parse_one(query_transformed).sql(pretty=True) +@pytest.mark.parametrize( + "query, query_transformed", + [ + ( + "SELECT left.name FROM database.table AS left JOIN other_database.table AS right ON left.id = right.id", + "SELECT left.name FROM development.table AS left JOIN other_database.table AS right ON left.id = right.id", + ), + ], +) +def test_query_tile_creates_database_with_database_overwrite_where(tmp_path, query, query_transformed): + query_path = tmp_path / "counter.sql" + query_path.write_text(query) + + replace_with_development_database = functools.partial( + replace_database_in_query, + database="development", + database_to_replace="database", + ) + query_tile = QueryTile(TileMetadata.from_path(query_path), query_transformer=replace_with_development_database) + + dataset = next(query_tile.get_datasets()) + + assert dataset.query == sqlglot.parse_one(query_transformed).sql(pretty=True) + + @pytest.mark.parametrize("width", [5, 8, 13]) @pytest.mark.parametrize("height", [4, 8, 12]) @pytest.mark.parametrize("filters", ["", "a", "ab", "abc", "abcde", "abcdefgh"]) From b4f938ba76b04655b7c4a5cff9aeffbc4af4d85c Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 11:12:05 +0200 Subject: [PATCH 02/18] Merge tests --- tests/unit/test_dashboards.py | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index d02aaffd..022b5787 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -734,51 +734,37 @@ def test_query_tile_keeps_original_query(tmp_path): @pytest.mark.parametrize( - "query, query_transformed", + "query, query_transformed, database_to_replace", [ - ("SELECT count FROM table", "SELECT count FROM table"), - ("SELECT count FROM database.table", "SELECT count FROM development.table"), - ("SELECT count FROM catalog.database.table", "SELECT count FROM catalog.development.table"), - ("SELECT database FROM database.table", "SELECT database FROM development.table"), + ("SELECT count FROM table", "SELECT count FROM table", None), + ("SELECT count FROM database.table", "SELECT count FROM development.table", None), + ("SELECT count FROM catalog.database.table", "SELECT count FROM catalog.development.table", None), + ("SELECT database FROM database.table", "SELECT database FROM development.table", None), ( "SELECT * FROM server.database.table, server.other_database.table", "SELECT * FROM server.development.table, server.development.table", + None, ), ( "SELECT left.* FROM server.database.table AS left JOIN server.other_database.table AS right ON left.id = right.id", "SELECT left.* FROM server.development.table AS left JOIN server.development.table AS right ON left.id = right.id", + None, ), - ], -) -def test_query_tile_creates_database_with_database_overwrite(tmp_path, query, query_transformed): - query_path = tmp_path / "counter.sql" - query_path.write_text(query) - - replace_with_development_database = functools.partial(replace_database_in_query, database="development") - query_tile = QueryTile(TileMetadata.from_path(query_path), query_transformer=replace_with_development_database) - - dataset = next(query_tile.get_datasets()) - - assert dataset.query == sqlglot.parse_one(query_transformed).sql(pretty=True) - - -@pytest.mark.parametrize( - "query, query_transformed", - [ ( "SELECT left.name FROM database.table AS left JOIN other_database.table AS right ON left.id = right.id", "SELECT left.name FROM development.table AS left JOIN other_database.table AS right ON left.id = right.id", + "database", ), ], ) -def test_query_tile_creates_database_with_database_overwrite_where(tmp_path, query, query_transformed): +def test_query_tile_creates_database_with_database_overwrite(tmp_path, query, query_transformed, database_to_replace): query_path = tmp_path / "counter.sql" query_path.write_text(query) replace_with_development_database = functools.partial( replace_database_in_query, database="development", - database_to_replace="database", + database_to_replace=database_to_replace, ) query_tile = QueryTile(TileMetadata.from_path(query_path), query_transformer=replace_with_development_database) From 727a614275a3b28477055bdd7a9e07ef747da756 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 15:49:26 +0200 Subject: [PATCH 03/18] Refactor to replace database on dashboard metadata object --- src/databricks/labs/lsql/cli.py | 10 +- src/databricks/labs/lsql/dashboards.py | 122 ++++++------- tests/unit/test_dashboards.py | 236 +++++++++++++------------ 3 files changed, 180 insertions(+), 188 deletions(-) diff --git a/src/databricks/labs/lsql/cli.py b/src/databricks/labs/lsql/cli.py index 85459f27..6465ddda 100644 --- a/src/databricks/labs/lsql/cli.py +++ b/src/databricks/labs/lsql/cli.py @@ -1,4 +1,3 @@ -import functools import webbrowser from pathlib import Path @@ -6,8 +5,7 @@ from databricks.labs.blueprint.entrypoint import get_logger from databricks.sdk import WorkspaceClient -from databricks.labs.lsql import dashboards -from databricks.labs.lsql.dashboards import Dashboards +from databricks.labs.lsql.dashboards import DashboardMetadata, Dashboards logger = get_logger(__file__) lsql = App(__file__) @@ -25,10 +23,10 @@ def create_dashboard( logger.debug("Creating dashboard ...") lakeview_dashboards = Dashboards(w) folder = Path(folder) - replace_database_in_query = None + dashboard_metadata = DashboardMetadata.from_path(folder) if database: - replace_database_in_query = functools.partial(dashboards.replace_database_in_query, database=database) - lakeview_dashboard = lakeview_dashboards.create_dashboard(folder, query_transformer=replace_database_in_query) + dashboard_metadata.replace_database(database) + lakeview_dashboard = lakeview_dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = lakeview_dashboards.deploy_dashboard(lakeview_dashboard) if not no_open: assert sdk_dashboard.dashboard_id is not None diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index d5281f7c..33974574 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -355,34 +355,6 @@ class MarkdownTile(Tile): _position: Position = Position(0, 0, _MAXIMUM_DASHBOARD_WIDTH, 3) -def replace_database_in_query( - node: sqlglot.Expression, - *, - database: str, - database_to_replace: str | None = None, -) -> sqlglot.Expression: - """Replace the database in a query. - - Parameters : - node : sqlglot.Expression - The parsed sql query - database : str - The value to replace the database with - database_to_replace : str | None (default: None) - The database to replace, if None, all databases are replaced - - Returns : - The query with the database replaced - """ - if ( - isinstance(node, sqlglot.exp.Table) - and node.args.get("db") is not None - and (database_to_replace is None or getattr(node.args.get("db"), "this", "") == database_to_replace) - ): - node.args["db"].set("this", database) - return node - - @dataclass class QueryTile(Tile): """A tile based on a sql query.""" @@ -625,10 +597,21 @@ def _get_query_widget_spec(fields: list[Field], *, frame: WidgetFrameSpec | None @dataclass class DashboardMetadata: - """The metadata defining a lakeview dashboard""" + """The metadata defining a lakeview dashboard + + Attributes : + display_name : str + The dashboard display name + tile_metadatas : list[TileMetadata] (default: []) + The tile metadata objects + query_transformer : Callable[[sqlglot.Expression], sqlglot.Expression] | None (default: None) + A sqlglot transformer applied on the queries (SQL files) before creating the tiles. If None, no + transformation is applied. + """ display_name: str tile_metadatas: list[TileMetadata] = dataclasses.field(default_factory=list) + query_transformer: Callable[[sqlglot.Expression], sqlglot.Expression] | None = None def validate(self) -> None: """Validate the dashboard metadata. @@ -674,20 +657,39 @@ def update(self, other: "DashboardMetadata") -> None: tile_metadatas.append(tile_metadata) self.tile_metadatas = tile_metadatas - def _get_tiles( - self, query_transformer: Callable[[sqlglot.Expression], sqlglot.Expression] | None = None - ) -> list[Tile]: + def replace_database( + self, + database: str, + *, + database_to_replace: str | None = None, + ) -> None: + """Replace the database in the queries. + + Parameters : + database : str + The value to replace the database with + database_to_replace : str | None (default: None) + The database to replace, if None, all databases are replaced + """ + + def replace_database_in_query(node: sqlglot.Expression) -> sqlglot.Expression: + if ( + isinstance(node, sqlglot.exp.Table) + and node.args.get("db") is not None + and (database_to_replace is None or getattr(node.args.get("db"), "this", "") == database_to_replace) + ): + node.args["db"].set("this", database) + return node + + self.query_transformer = replace_database_in_query + + def _get_tiles(self) -> list[Tile]: """Get the tiles from the tiles metadata. The order of the tiles is by default the alphanumerically sorted tile ids, however, the order may be overwritten with the `order` key. Hence, the logic to: i) set the order when not specified; ii) sort the tiles using the order field. - - Parameters : - query_transformer : Callable[[sqlglot.Expression], sqlglot.Expression] | None (default: None) - A sqlglot transformer applied on the queries (SQL files) before creating the tiles. If None, no - transformation is applied. """ tile_metadatas_with_order = [] for default_order, tile_metadata in enumerate(sorted(self.tile_metadatas, key=lambda wm: wm.id)): @@ -697,34 +699,24 @@ def _get_tiles( for tile_metadata in sorted(tile_metadatas_with_order, key=lambda t: (t.order, t.id)): tile = Tile.from_tile_metadata(tile_metadata) if isinstance(tile, QueryTile): - tile.query_transformer = query_transformer + tile.query_transformer = self.query_transformer tile.place_after(position) tiles.append(tile) position = tile.position return tiles - def get_datasets( - self, query_transformer: Callable[[sqlglot.Expression], sqlglot.Expression] | None = None - ) -> list[Dataset]: - """Get the datasets for the dashboard. - - See :meth:`DashboardMetadata._get_tiles` for `query_transformer`. - """ + def get_datasets(self) -> list[Dataset]: + """Get the datasets for the dashboard.""" datasets: list[Dataset] = [] - for tile in self._get_tiles(query_transformer): + for tile in self._get_tiles(): if isinstance(tile, QueryTile): datasets.extend(tile.get_datasets()) return datasets - def get_layouts( - self, query_transformer: Callable[[sqlglot.Expression], sqlglot.Expression] | None = None - ) -> list[Layout]: - """Get the layouts for the dashboard. - - See :meth:`DashboardMetadata._get_tiles` for `query_transformer`. - """ + def get_layouts(self) -> list[Layout]: + """Get the layouts for the dashboard.""" layouts: list[Layout] = [] - for tile in self._get_tiles(query_transformer): + for tile in self._get_tiles(): layouts.extend(tile.get_layouts()) return layouts @@ -840,30 +832,22 @@ def _format_query(query: str) -> str: return formatted_query @staticmethod - def create_dashboard( - dashboard_folder: Path, - *, - query_transformer: Callable[[sqlglot.Expression], sqlglot.Expression] | None = None, - ) -> Dashboard: - """Create a dashboard from code, i.e. configuration and queries. + def create_dashboard(dashboard_metadata: DashboardMetadata) -> Dashboard: + """Create a dashboard from the dashboard metadata Parameters : - dashboard_folder : Path - The path to the folder with dashboard files. - query_transformer : Callable[[sqlglot.Expression], sqlglot.Expression] | None (default | None) - A sqlglot transformer applied on the queries (SQL files) before creating the dashboard. If None, no - transformation will be applied. + dashboard_metadata : DashboardMetadata + The dashboard metadata - Raises: + Raises : ValueError : If the dashboard metadata is invalid. Source : https://sqlglot.com/sqlglot/transforms.html """ - dashboard_metadata = DashboardMetadata.from_path(dashboard_folder) dashboard_metadata.validate() - datasets = dashboard_metadata.get_datasets(query_transformer) - layouts = dashboard_metadata.get_layouts(query_transformer) + datasets = dashboard_metadata.get_datasets() + layouts = dashboard_metadata.get_layouts() page = Page( name=dashboard_metadata.display_name, display_name=dashboard_metadata.display_name, diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 022b5787..ec90a6ef 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -1,4 +1,3 @@ -import functools import itertools import json import logging @@ -21,7 +20,6 @@ Tile, TileMetadata, WidgetType, - replace_database_in_query, ) from databricks.labs.lsql.lakeview import ( CounterEncodingMap, @@ -500,8 +498,8 @@ def test_tile_places_tile_below(): def test_dashboards_saves_sql_files_to_folder(tmp_path): ws = create_autospec(WorkspaceClient) - queries = Path(__file__).parent / "queries" - dashboard = Dashboards(ws).create_dashboard(queries) + dashboard_metadata = DashboardMetadata.from_path(Path(__file__).parent / "queries") + dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) Dashboards(ws).save_to_folder(dashboard, tmp_path) @@ -511,8 +509,8 @@ def test_dashboards_saves_sql_files_to_folder(tmp_path): def test_dashboards_saves_yml_files_to_folder(tmp_path): ws = create_autospec(WorkspaceClient) - queries = Path(__file__).parent / "queries" - dashboard = Dashboards(ws).create_dashboard(queries) + dashboard_metadata = DashboardMetadata.from_path(Path(__file__).parent / "queries") + dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) Dashboards(ws).save_to_folder(dashboard, tmp_path) @@ -522,18 +520,19 @@ def test_dashboards_saves_yml_files_to_folder(tmp_path): 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) + dashboard_metadata = DashboardMetadata.from_path(Path(__file__).parent / "queries") + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) page = lakeview_dashboard.pages[0] assert page.name == "queries" assert page.display_name == "queries" def test_dashboards_creates_dashboard_with_custom_first_page_name(tmp_path): - (tmp_path / "dashboard.yml").write_text("display_name: Custom") - ws = create_autospec(WorkspaceClient) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + + (tmp_path / "dashboard.yml").write_text("display_name: Custom") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) page = lakeview_dashboard.pages[0] assert page.name == "Custom" @@ -542,35 +541,38 @@ def test_dashboards_creates_dashboard_with_custom_first_page_name(tmp_path): @pytest.mark.parametrize("dashboard_content", ["missing_display_name: true", "invalid:\nyml"]) def test_dashboards_handles_invalid_dashboard_yml(tmp_path, dashboard_content): + ws = create_autospec(WorkspaceClient) + queries_path = tmp_path / "queries" queries_path.mkdir() (queries_path / "dashboard.yml").write_text(dashboard_content) + dashboard_metadata = DashboardMetadata.from_path(queries_path) - ws = create_autospec(WorkspaceClient) - lakeview_dashboard = Dashboards(ws).create_dashboard(queries_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) page = lakeview_dashboard.pages[0] assert page.name == "queries" assert page.display_name == "queries" + ws.assert_not_called() def test_dashboards_creates_one_dataset_per_query(): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" - dashboard = Dashboards(ws).create_dashboard(queries) + dashboard_metadata = DashboardMetadata.from_path(queries) + dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) assert len(dashboard.datasets) == len([query for query in queries.glob("*.sql")]) def test_dashboard_creates_datasets_using_query(tmp_path): ws = create_autospec(WorkspaceClient) - query = "SELECT count FROM database.table" (tmp_path / "counter.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) dataset = lakeview_dashboard.datasets[0] - assert dataset.query == query ws.assert_not_called() @@ -588,8 +590,9 @@ def test_dashboard_creates_datasets_with_transformed_query(tmp_path): """.lstrip() (tmp_path / "counter.sql").write_text(query) - query_transformer = functools.partial(replace_database_in_query, database="development") - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path, query_transformer=query_transformer) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + dashboard_metadata.replace_database("development") + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) dataset = lakeview_dashboard.datasets[0] @@ -602,7 +605,9 @@ def test_dashboard_creates_datasets_with_transformed_query(tmp_path): def test_dashboards_creates_one_counter_widget_per_query(): ws = create_autospec(WorkspaceClient) queries = Path(__file__).parent / "queries" - dashboard = Dashboards(ws).create_dashboard(queries) + dashboard_metadata = DashboardMetadata.from_path(queries) + + dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) counter_widgets = [] for page in dashboard.pages: @@ -618,15 +623,14 @@ def test_dashboards_creates_text_widget_for_invalid_query(tmp_path, caplog): # 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") + (tmp_path / f"{i}_counter.sql").write_text(f"SELECT {i} AS count") invalid_query = "SELECT COUNT(* AS missing_closing_parenthesis" - with (tmp_path / "1_invalid.sql").open("w") as f: - f.write(invalid_query) + (tmp_path / "1_invalid.sql").write_text(invalid_query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) with caplog.at_level(logging.WARNING, logger="databricks.labs.lsql.dashboards"): - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) markdown_widget = lakeview_dashboard.pages[0].layout[1].widget assert markdown_widget.textbox_spec == invalid_query @@ -635,10 +639,9 @@ def test_dashboards_creates_text_widget_for_invalid_query(tmp_path, caplog): def test_dashboards_does_not_create_widget_for_yml_file(tmp_path, caplog): ws = create_autospec(WorkspaceClient) - (tmp_path / "dashboard.yml").write_text("display_name: Git based dashboard") - - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) assert len(lakeview_dashboard.pages[0].layout) == 0 @@ -741,13 +744,13 @@ def test_query_tile_keeps_original_query(tmp_path): ("SELECT count FROM catalog.database.table", "SELECT count FROM catalog.development.table", None), ("SELECT database FROM database.table", "SELECT database FROM development.table", None), ( - "SELECT * FROM server.database.table, server.other_database.table", - "SELECT * FROM server.development.table, server.development.table", + "SELECT a FROM server.database.table, server.other_database.table", + "SELECT a FROM server.development.table, server.development.table", None, ), ( - "SELECT left.* FROM server.database.table AS left JOIN server.other_database.table AS right ON left.id = right.id", - "SELECT left.* FROM server.development.table AS left JOIN server.development.table AS right ON left.id = right.id", + "SELECT left.a FROM server.database.table AS left JOIN server.other_database.table AS right ON left.id = right.id", + "SELECT left.a FROM server.development.table AS left JOIN server.development.table AS right ON left.id = right.id", None, ), ( @@ -758,19 +761,14 @@ def test_query_tile_keeps_original_query(tmp_path): ], ) def test_query_tile_creates_database_with_database_overwrite(tmp_path, query, query_transformed, database_to_replace): - query_path = tmp_path / "counter.sql" - query_path.write_text(query) - - replace_with_development_database = functools.partial( - replace_database_in_query, - database="development", - database_to_replace=database_to_replace, - ) - query_tile = QueryTile(TileMetadata.from_path(query_path), query_transformer=replace_with_development_database) + (tmp_path / "counter.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + dashboard_metadata.replace_database("development", database_to_replace=database_to_replace) - dataset = next(query_tile.get_datasets()) + datasets = dashboard_metadata.get_datasets() - assert dataset.query == sqlglot.parse_one(query_transformed).sql(pretty=True) + assert len(datasets) == 1 + assert datasets[0].query == sqlglot.parse_one(query_transformed).sql(pretty=True) @pytest.mark.parametrize("width", [5, 8, 13]) @@ -814,11 +812,11 @@ def test_table_tile_becomes_wider_with_more_columns(tmp_path): def test_dashboards_creates_dashboard_with_expected_counter_field_encoding_names(tmp_path): - with (tmp_path / "query.sql").open("w") as f: - f.write("SELECT 1 AS amount") - ws = create_autospec(WorkspaceClient) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + (tmp_path / "query.sql").write_text("SELECT 1 AS amount") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) counter_spec = lakeview_dashboard.pages[0].layout[0].widget.spec assert isinstance(counter_spec, CounterSpec) @@ -837,10 +835,11 @@ def test_dashboards_creates_dashboard_with_expected_counter_field_encoding_names ], ) def test_dashboards_infers_query_spec(tmp_path, query, spec_expected): + ws = create_autospec(WorkspaceClient) (tmp_path / "query.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - ws = create_autospec(WorkspaceClient) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) spec = lakeview_dashboard.pages[0].layout[0].widget.spec assert isinstance(spec, spec_expected) @@ -848,11 +847,12 @@ def test_dashboards_infers_query_spec(tmp_path, query, spec_expected): def test_dashboards_overrides_show_empty_title_in_query_header(tmp_path): + ws = create_autospec(WorkspaceClient) query = '-- --overrides \'{"spec": {"frame": {"showTitle": true}}}\'\nSELECT 102132 AS count' (tmp_path / "query.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - ws = create_autospec(WorkspaceClient) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) frame = lakeview_dashboard.pages[0].layout[0].widget.spec.frame assert frame.show_title @@ -875,8 +875,9 @@ def test_dashboards_overrides_show_empty_title_in_dashboard_yml(tmp_path): """.strip() (tmp_path / "dashboard.yml").write_text(dashboard_content) (tmp_path / "query.sql").write_text("SELECT 20") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) frame = lakeview_dashboard.pages[0].layout[0].widget.spec.frame assert frame.show_title @@ -885,10 +886,11 @@ def test_dashboards_overrides_show_empty_title_in_dashboard_yml(tmp_path): def test_dashboards_creates_dashboard_with_expected_table_field_encodings(tmp_path): + ws = create_autospec(WorkspaceClient) (tmp_path / "query.sql").write_text("select 1 as first, 2 as second") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - ws = create_autospec(WorkspaceClient) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) table_spec = lakeview_dashboard.pages[0].layout[0].widget.spec assert isinstance(table_spec, TableV1Spec) @@ -902,12 +904,12 @@ def test_dashboards_creates_dashboards_with_second_widget_to_the_right_of_the_fi for i in range(2): (tmp_path / f"counter_{i}.sql").write_text(f"SELECT {i} AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) layout = lakeview_dashboard.pages[0].layout first_position, second_position = layout[0].position, layout[1].position - assert first_position.x < second_position.x assert first_position.y == second_position.y ws.assert_not_called() @@ -916,26 +918,25 @@ def test_dashboards_creates_dashboards_with_second_widget_to_the_right_of_the_fi def test_dashboards_creates_dashboard_with_many_widgets_not_on_the_first_row(tmp_path): ws = create_autospec(WorkspaceClient) for i in range(10): - with (tmp_path / f"counter_{i}.sql").open("w") as f: - f.write(f"SELECT {i} AS count") + (tmp_path / f"counter_{i}.sql").write_text(f"SELECT {i} AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - layout = lakeview_dashboard.pages[0].layout + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) + layout = lakeview_dashboard.pages[0].layout assert layout[-1].position.y > 0 ws.assert_not_called() 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: - f.write("# Description") - with (tmp_path / "010_counter.sql").open("w") as f: - f.write("SELECT 100 AS count") + (tmp_path / "000_counter.md").write_text("# Description") + (tmp_path / "010_counter.sql").write_text("SELECT 100 AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - layout = lakeview_dashboard.pages[0].layout + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) + layout = lakeview_dashboard.pages[0].layout assert len(layout) == 2 assert layout[0].position.y < layout[1].position.y ws.assert_not_called() @@ -955,9 +956,10 @@ def test_dashboards_creates_dashboard_with_id_collisions_raises_value_error(tmp_ (tmp_path / "counter.md").write_text("# Description") (tmp_path / "counter.sql").write_text("SELECT 100 AS count") (tmp_path / "header_overwrite.sql").write_text("-- --id counter\nSELECT 100 AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) with pytest.raises(ValueError) as e: - Dashboards(ws).create_dashboard(tmp_path) + Dashboards(ws).create_dashboard(dashboard_metadata) assert "Duplicate id: counter" in str(e.value) @@ -966,12 +968,12 @@ def test_dashboards_creates_dashboards_with_widgets_sorted_alphanumerically(tmp_ ws = create_autospec(WorkspaceClient) for query_name in query_names: - with (tmp_path / f"{query_name}.sql").open("w") as f: - f.write("SELECT 1 AS count") + (tmp_path / f"{query_name}.sql").write_text("SELECT 1 AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) + widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] assert widget_names == query_names ws.assert_not_called() @@ -984,10 +986,11 @@ def test_dashboards_creates_dashboards_with_widgets_order_overwrite(tmp_path): (tmp_path / "e.sql").write_text("-- --order 1\nSELECT 1 AS count") for query_name in "abcdf": (tmp_path / f"{query_name}.sql").write_text("SELECT 1 AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) + widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] assert "".join(widget_names) == "abecdf" ws.assert_not_called() @@ -1000,10 +1003,11 @@ def test_dashboards_creates_dashboards_with_widgets_order_overwrite_zero(tmp_pat (tmp_path / "e.sql").write_text("-- --order 0\nSELECT 1 AS count") for query_name in "abcdf": (tmp_path / f"{query_name}.sql").write_text("SELECT 1 AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) + widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] assert "".join(widget_names) == "aebcdf" ws.assert_not_called() @@ -1014,15 +1018,18 @@ def test_dashboards_creates_dashboards_with_widget_ordered_using_id(tmp_path): (tmp_path / "z.sql").write_text("-- --id a\nSELECT 1 AS count") # Should be first because id is 'a' for query_name in "bcdef": (tmp_path / f"{query_name}.sql").write_text("SELECT 1 AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) + widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] assert "".join(widget_names) == "abcdef" ws.assert_not_called() def test_dashboards_creates_dashboard_with_widget_order_overwrite_from_dashboard_yaml(tmp_path): + ws = create_autospec(WorkspaceClient) + content = """ display_name: Ordering @@ -1033,9 +1040,9 @@ def test_dashboards_creates_dashboard_with_widget_order_overwrite_from_dashboard (tmp_path / "dashboard.yml").write_text(content) for query_name in "abcdef": (tmp_path / f"{query_name}.sql").write_text("SELECT 1 AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - ws = create_autospec(WorkspaceClient) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] assert "".join(widget_names) == "eabcdf" @@ -1043,6 +1050,8 @@ def test_dashboards_creates_dashboard_with_widget_order_overwrite_from_dashboard def test_dashboards_creates_dashboard_where_widget_order_in_header_takes_precedence(tmp_path): + ws = create_autospec(WorkspaceClient) + content = """ display_name: Ordering @@ -1053,9 +1062,9 @@ def test_dashboards_creates_dashboard_where_widget_order_in_header_takes_precede (tmp_path / "dashboard.yml").write_text(content) for index in range(3): (tmp_path / f"query_{index}.sql").write_text(f"-- --order {index}\nSELECT {index} AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - ws = create_autospec(WorkspaceClient) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) widget_names = [layout.widget.name for layout in lakeview_dashboard.pages[0].layout] assert widget_names == ["query_0", "query_1", "query_2"] @@ -1072,8 +1081,9 @@ def test_dashboards_creates_dashboard_where_widget_order_in_header_takes_precede def test_dashboards_creates_dashboards_where_widget_has_expected_width_and_height(tmp_path, query, width, height): ws = create_autospec(WorkspaceClient) (tmp_path / "query.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) position = lakeview_dashboard.pages[0].layout[0].position assert position.width == width @@ -1083,13 +1093,12 @@ def test_dashboards_creates_dashboards_where_widget_has_expected_width_and_heigh def test_dashboards_creates_dashboards_where_text_widget_has_expected_width_and_height(tmp_path): ws = create_autospec(WorkspaceClient) + (tmp_path / "description.md").write_text("# Description") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - with (tmp_path / "description.md").open("w") as f: - f.write("# Description") + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) - 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() @@ -1097,14 +1106,13 @@ def test_dashboards_creates_dashboards_where_text_widget_has_expected_width_and_ 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) + (tmp_path / "description.md").write_text(content) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - widget = lakeview_dashboard.pages[0].layout[0].widget + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) + widget = lakeview_dashboard.pages[0].layout[0].widget assert widget.textbox_spec == content ws.assert_not_called() @@ -1119,14 +1127,13 @@ def test_dashboards_creates_dashboards_where_text_widget_has_expected_text(tmp_p ) 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" - with (tmp_path / "counter.sql").open("w") as f: - f.write(query) + (tmp_path / "counter.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) - position = lakeview_dashboard.pages[0].layout[0].position + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) + position = lakeview_dashboard.pages[0].layout[0].position assert position.width == 6 assert position.height == 3 ws.assert_not_called() @@ -1134,11 +1141,11 @@ def test_dashboard_creates_dashboard_with_custom_sized_widget(tmp_path, header): def test_dashboard_creates_dashboard_with_title(tmp_path): ws = create_autospec(WorkspaceClient) - query = "-- --title 'Count me in'\nSELECT 2918" (tmp_path / "counter.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) frame = lakeview_dashboard.pages[0].layout[0].widget.spec.frame assert frame.title == "Count me in" @@ -1148,10 +1155,10 @@ def test_dashboard_creates_dashboard_with_title(tmp_path): def test_dashboard_creates_dashboard_without_title(tmp_path): ws = create_autospec(WorkspaceClient) - (tmp_path / "counter.sql").write_text("SELECT 109") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) frame = lakeview_dashboard.pages[0].layout[0].widget.spec.frame assert frame.title == "" @@ -1161,11 +1168,11 @@ def test_dashboard_creates_dashboard_without_title(tmp_path): def test_dashboard_creates_dashboard_with_description(tmp_path): ws = create_autospec(WorkspaceClient) - query = "-- --description 'Only when it counts'\nSELECT 2918" (tmp_path / "counter.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) frame = lakeview_dashboard.pages[0].layout[0].widget.spec.frame assert frame.description == "Only when it counts" @@ -1175,10 +1182,10 @@ def test_dashboard_creates_dashboard_with_description(tmp_path): def test_dashboard_creates_dashboard_without_description(tmp_path): ws = create_autospec(WorkspaceClient) - (tmp_path / "counter.sql").write_text("SELECT 190219") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) frame = lakeview_dashboard.pages[0].layout[0].widget.spec.frame assert frame.description == "" @@ -1188,12 +1195,12 @@ def test_dashboard_creates_dashboard_without_description(tmp_path): def test_dashboard_creates_dashboard_with_filter(tmp_path): ws = create_autospec(WorkspaceClient) - filter_column = "City" query = f"-- --filter {filter_column}\nSELECT Address, City, Province, Country FROM europe" (tmp_path / "table.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) layouts = lakeview_dashboard.pages[0].layout assert any(f"filter_{filter_column}" in layout.widget.name for layout in layouts) @@ -1212,9 +1219,10 @@ def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): query = "-- --widh 6 --height 5 \nSELECT 82917019218921 AS big_number_needs_big_widget" query_path = tmp_path / "counter.sql" query_path.write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) with caplog.at_level(logging.WARNING, logger="databricks.labs.lsql.dashboards"): - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) position = lakeview_dashboard.pages[0].layout[0].position assert position.width == 1 @@ -1226,9 +1234,10 @@ def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): def test_create_dashboard_raises_not_implemented_error_for_select_star(tmp_path): ws = create_autospec(WorkspaceClient) (tmp_path / "star.sql").write_text("SELECT * FROM table") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) with pytest.raises(NotImplementedError) as e: - Dashboards(ws).create_dashboard(tmp_path) + Dashboards(ws).create_dashboard(dashboard_metadata) assert "star" in str(e) ws.assert_not_called() @@ -1241,8 +1250,9 @@ def test_dashboard_creates_dashboard_based_on_markdown_header(tmp_path): (tmp_path / f"{query_name}.sql").write_text("SELECT 1 AS count") content = "---\norder: -1\nwidth: 6\nheight: 3\n---\n# Description" (tmp_path / "widget.md").write_text(content) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) position = lakeview_dashboard.pages[0].layout[0].position assert position.width == 6 @@ -1252,11 +1262,11 @@ def test_dashboard_creates_dashboard_based_on_markdown_header(tmp_path): def test_dashboard_uses_metadata_above_select_when_query_has_cte(tmp_path): ws = create_autospec(WorkspaceClient) - query = "WITH data AS (SELECT 1 AS count)\n" "-- --width 6 --height 6\n" "SELECT count FROM data" (tmp_path / "widget.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) position = lakeview_dashboard.pages[0].layout[0].position assert position.width == 6 @@ -1266,11 +1276,11 @@ def test_dashboard_uses_metadata_above_select_when_query_has_cte(tmp_path): def test_dashboard_ignores_first_line_metadata_when_query_has_cte(tmp_path): ws = create_autospec(WorkspaceClient) - query = "-- --width 6 --height 6\n" "WITH data AS (SELECT 1 AS count)\n" "SELECT count FROM data" (tmp_path / "widget.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) - lakeview_dashboard = Dashboards(ws).create_dashboard(tmp_path) + lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) position = lakeview_dashboard.pages[0].layout[0].position assert position.width != 6 From 5d2d2f6cfca96285ef4b3b32476e46b4faa9d04a Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 16:11:03 +0200 Subject: [PATCH 04/18] Let replace database return new instance of DashboardMetadata --- src/databricks/labs/lsql/cli.py | 2 +- src/databricks/labs/lsql/dashboards.py | 4 ++-- tests/unit/test_dashboards.py | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/databricks/labs/lsql/cli.py b/src/databricks/labs/lsql/cli.py index 6465ddda..015b5cea 100644 --- a/src/databricks/labs/lsql/cli.py +++ b/src/databricks/labs/lsql/cli.py @@ -25,7 +25,7 @@ def create_dashboard( folder = Path(folder) dashboard_metadata = DashboardMetadata.from_path(folder) if database: - dashboard_metadata.replace_database(database) + dashboard_metadata = dashboard_metadata.replace_database(database) lakeview_dashboard = lakeview_dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = lakeview_dashboards.deploy_dashboard(lakeview_dashboard) if not no_open: diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 33974574..05cd9b3f 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -662,7 +662,7 @@ def replace_database( database: str, *, database_to_replace: str | None = None, - ) -> None: + ) -> "DashboardMetadata": """Replace the database in the queries. Parameters : @@ -681,7 +681,7 @@ def replace_database_in_query(node: sqlglot.Expression) -> sqlglot.Expression: node.args["db"].set("this", database) return node - self.query_transformer = replace_database_in_query + return dataclasses.replace(self, query_transformer=replace_database_in_query) def _get_tiles(self) -> list[Tile]: """Get the tiles from the tiles metadata. diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index ec90a6ef..4921b1cc 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -590,8 +590,7 @@ def test_dashboard_creates_datasets_with_transformed_query(tmp_path): """.lstrip() (tmp_path / "counter.sql").write_text(query) - dashboard_metadata = DashboardMetadata.from_path(tmp_path) - dashboard_metadata.replace_database("development") + dashboard_metadata = DashboardMetadata.from_path(tmp_path).replace_database("development") lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) dataset = lakeview_dashboard.datasets[0] @@ -762,8 +761,9 @@ def test_query_tile_keeps_original_query(tmp_path): ) def test_query_tile_creates_database_with_database_overwrite(tmp_path, query, query_transformed, database_to_replace): (tmp_path / "counter.sql").write_text(query) - dashboard_metadata = DashboardMetadata.from_path(tmp_path) - dashboard_metadata.replace_database("development", database_to_replace=database_to_replace) + dashboard_metadata = DashboardMetadata.from_path(tmp_path).replace_database( + "development", database_to_replace=database_to_replace + ) datasets = dashboard_metadata.get_datasets() From 42615edae5fffd94d16f22c24935fb6b4ab0d0c9 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 16:22:41 +0200 Subject: [PATCH 05/18] Support replacing catalog in dataset --- src/databricks/labs/lsql/dashboards.py | 27 ++++++++++++------- tests/unit/test_dashboards.py | 36 +++++++++++++++++--------- 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 05cd9b3f..7cd9a342 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -659,29 +659,38 @@ def update(self, other: "DashboardMetadata") -> None: def replace_database( self, - database: str, + catalog: str | None = None, + database: str | None = None, *, + catalog_to_replace: str | None = None, database_to_replace: str | None = None, ) -> "DashboardMetadata": """Replace the database in the queries. Parameters : + catalog : str + The value to replace the catalog with database : str The value to replace the database with + catalog_to_replace : str | None (default: None) + The catalog to replace, if None, all catalogs are replaced database_to_replace : str | None (default: None) The database to replace, if None, all databases are replaced """ - def replace_database_in_query(node: sqlglot.Expression) -> sqlglot.Expression: - if ( - isinstance(node, sqlglot.exp.Table) - and node.args.get("db") is not None - and (database_to_replace is None or getattr(node.args.get("db"), "this", "") == database_to_replace) - ): - node.args["db"].set("this", database) + def replace_catalog_and_database_in_query(node: sqlglot.Expression) -> sqlglot.Expression: + if isinstance(node, sqlglot.exp.Table): + if node.args.get("catalog") is not None and ( + catalog_to_replace is None or getattr(node.args.get("catalog"), "this", "") == catalog_to_replace + ): + node.args["catalog"].set("this", catalog) + if node.args.get("db") is not None and ( + database_to_replace is None or getattr(node.args.get("db"), "this", "") == database_to_replace + ): + node.args["db"].set("this", database) return node - return dataclasses.replace(self, query_transformer=replace_database_in_query) + return dataclasses.replace(self, query_transformer=replace_catalog_and_database_in_query) def _get_tiles(self) -> list[Tile]: """Get the tiles from the tiles metadata. diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 4921b1cc..70a65884 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -590,7 +590,7 @@ def test_dashboard_creates_datasets_with_transformed_query(tmp_path): """.lstrip() (tmp_path / "counter.sql").write_text(query) - dashboard_metadata = DashboardMetadata.from_path(tmp_path).replace_database("development") + dashboard_metadata = DashboardMetadata.from_path(tmp_path).replace_database(database="development") lakeview_dashboard = Dashboards(ws).create_dashboard(dashboard_metadata) dataset = lakeview_dashboard.datasets[0] @@ -736,33 +736,45 @@ def test_query_tile_keeps_original_query(tmp_path): @pytest.mark.parametrize( - "query, query_transformed, database_to_replace", + "query, query_transformed, catalog_to_replace, database_to_replace", [ - ("SELECT count FROM table", "SELECT count FROM table", None), - ("SELECT count FROM database.table", "SELECT count FROM development.table", None), - ("SELECT count FROM catalog.database.table", "SELECT count FROM catalog.development.table", None), - ("SELECT database FROM database.table", "SELECT database FROM development.table", None), + ("SELECT count FROM table", "SELECT count FROM table", None, None), + ("SELECT count FROM database.table", "SELECT count FROM development.table", None, None), + ("SELECT count FROM catalog.database.table", "SELECT count FROM catalog.development.table", None, None), + ("SELECT database FROM database.table", "SELECT database FROM development.table", None, None), ( - "SELECT a FROM server.database.table, server.other_database.table", - "SELECT a FROM server.development.table, server.development.table", + "SELECT a FROM server.database.table, remote_server.other_database.table", + "SELECT a FROM catalog.development.table, remote_server.development.table", + "server", None, ), ( - "SELECT left.a FROM server.database.table AS left JOIN server.other_database.table AS right ON left.id = right.id", - "SELECT left.a FROM server.development.table AS left JOIN server.development.table AS right ON left.id = right.id", + "SELECT left.a FROM hive_metastore.database.table AS left JOIN hive_metastore.other_database.table AS right ON left.id = right.id", + "SELECT left.a FROM catalog.development.table AS left JOIN catalog.development.table AS right ON left.id = right.id", + None, None, ), ( "SELECT left.name FROM database.table AS left JOIN other_database.table AS right ON left.id = right.id", "SELECT left.name FROM development.table AS left JOIN other_database.table AS right ON left.id = right.id", + None, "database", ), ], ) -def test_query_tile_creates_database_with_database_overwrite(tmp_path, query, query_transformed, database_to_replace): +def test_query_tile_creates_database_with_database_overwrite( + tmp_path, + query, + query_transformed, + catalog_to_replace, + database_to_replace, +): (tmp_path / "counter.sql").write_text(query) dashboard_metadata = DashboardMetadata.from_path(tmp_path).replace_database( - "development", database_to_replace=database_to_replace + "catalog", + "development", + catalog_to_replace=catalog_to_replace, + database_to_replace=database_to_replace, ) datasets = dashboard_metadata.get_datasets() From 30f674d1547645916ce2ef7b9667c57650595fe4 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 16:30:24 +0200 Subject: [PATCH 06/18] Add replace catalog functionality to create-dashboard command --- labs.yml | 4 ++++ src/databricks/labs/lsql/cli.py | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/labs.yml b/labs.yml index 8fe9424d..e9c86f68 100644 --- a/labs.yml +++ b/labs.yml @@ -9,6 +9,10 @@ commands: flags: - name: folder description: The folder with dashboard files. By default, the current working directory. + - name: catalog + description: | + Overwrite the catalog in query with given value. Useful when developing with seperated catalogs, for + example, for production and development. - name: database description: | Overwrite the database in query with given value. Useful when developing with seperated databases, for diff --git a/src/databricks/labs/lsql/cli.py b/src/databricks/labs/lsql/cli.py index 015b5cea..b17b90c6 100644 --- a/src/databricks/labs/lsql/cli.py +++ b/src/databricks/labs/lsql/cli.py @@ -16,6 +16,7 @@ def create_dashboard( w: WorkspaceClient, folder: Path = Path.cwd(), *, + catalog: str = "", database: str = "", no_open: bool = False, ): @@ -23,9 +24,9 @@ def create_dashboard( logger.debug("Creating dashboard ...") lakeview_dashboards = Dashboards(w) folder = Path(folder) - dashboard_metadata = DashboardMetadata.from_path(folder) - if database: - dashboard_metadata = dashboard_metadata.replace_database(database) + dashboard_metadata = DashboardMetadata.from_path(folder).replace_database( + catalog=catalog or None, database=database or None + ) lakeview_dashboard = lakeview_dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = lakeview_dashboards.deploy_dashboard(lakeview_dashboard) if not no_open: From c566ea1fda8c9a8208a6f6d50d58590d0ed08abd Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 16:32:36 +0200 Subject: [PATCH 07/18] Move attribute docs --- src/databricks/labs/lsql/dashboards.py | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 7cd9a342..00d9101c 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -597,20 +597,11 @@ def _get_query_widget_spec(fields: list[Field], *, frame: WidgetFrameSpec | None @dataclass class DashboardMetadata: - """The metadata defining a lakeview dashboard - - Attributes : - display_name : str - The dashboard display name - tile_metadatas : list[TileMetadata] (default: []) - The tile metadata objects - query_transformer : Callable[[sqlglot.Expression], sqlglot.Expression] | None (default: None) - A sqlglot transformer applied on the queries (SQL files) before creating the tiles. If None, no - transformation is applied. - """ + """The metadata defining a lakeview dashboard""" - display_name: str - tile_metadatas: list[TileMetadata] = dataclasses.field(default_factory=list) + display_name: str # The dashboard display name + tile_metadatas: list[TileMetadata] = dataclasses.field(default_factory=list) # The tile metadata objects + # A sqlglot transformer applied on the queries before creating the tiles. If None, no transformation is applied query_transformer: Callable[[sqlglot.Expression], sqlglot.Expression] | None = None def validate(self) -> None: From c2800e1281b11c1bbe3492c9bdc12113e92f6512 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 16:39:49 +0200 Subject: [PATCH 08/18] Update integration test with new API --- tests/integration/test_dashboards.py | 103 +++++++++++++-------------- 1 file changed, 50 insertions(+), 53 deletions(-) diff --git a/tests/integration/test_dashboards.py b/tests/integration/test_dashboards.py index e0d429e9..d71f1971 100644 --- a/tests/integration/test_dashboards.py +++ b/tests/integration/test_dashboards.py @@ -8,7 +8,7 @@ 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.dashboards import DashboardMetadata, Dashboards from databricks.labs.lsql.lakeview.model import Dashboard logger = logging.getLogger(__name__) @@ -71,12 +71,12 @@ def tmp_path(tmp_path, make_random): def test_dashboards_deploys_exported_dashboard_definition(ws, make_dashboard): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() dashboard_file = Path(__file__).parent / "dashboards" / "dashboard.lvdash.json" lakeview_dashboard = Dashboard.from_dict(json.loads(dashboard_file.read_text())) - dashboards = Dashboards(ws) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) new_dashboard = dashboards.get_dashboard(sdk_dashboard.path) @@ -87,11 +87,12 @@ def test_dashboards_deploys_exported_dashboard_definition(ws, make_dashboard): def test_dashboard_deploys_dashboard_the_same_as_created_dashboard(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() (tmp_path / "counter.sql").write_text("SELECT 10 AS count") - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) new_dashboard = dashboards.get_dashboard(sdk_dashboard.path) @@ -103,12 +104,13 @@ def test_dashboard_deploys_dashboard_the_same_as_created_dashboard(ws, make_dash def test_dashboard_deploys_dashboard_with_ten_counters(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() for i in range(10): (tmp_path / f"counter_{i}.sql").write_text(f"SELECT {i} AS count") - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -116,15 +118,13 @@ 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): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard(display_name="Counter") - with (tmp_path / "dashboard.yml").open("w") as f: - f.write("display_name: Counter") - with (tmp_path / "counter.sql").open("w") as f: - f.write("SELECT 102132 AS count") - - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + (tmp_path / "dashboard.yml").write_text("display_name: Counter") + (tmp_path / "counter.sql").write_text("SELECT 102132 AS count") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -132,11 +132,12 @@ def test_dashboard_deploys_dashboard_with_display_name(ws, make_dashboard, tmp_p def test_dashboard_deploys_dashboard_with_counter_variation(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() (tmp_path / "counter.sql").write_text("SELECT 10 AS `Something Else Than Count`") - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -144,13 +145,13 @@ def test_dashboard_deploys_dashboard_with_counter_variation(ws, make_dashboard, def test_dashboard_deploys_dashboard_with_big_widget(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) 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) + (tmp_path / "counter.sql").write_text(query) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -158,17 +159,16 @@ def test_dashboard_deploys_dashboard_with_big_widget(ws, make_dashboard, tmp_pat def test_dashboards_deploys_dashboard_with_order_overwrite_in_query_header(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() for query_name in range(6): (tmp_path / f"{query_name}.sql").write_text(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. (tmp_path / "4.sql").write_text("-- --order 1\nSELECT 4 AS count") - - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -176,6 +176,7 @@ def test_dashboards_deploys_dashboard_with_order_overwrite_in_query_header(ws, m def test_dashboards_deploys_dashboard_with_order_overwrite_in_dashboard_yaml(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() # Move the '4' inbetween '1' and '2' query. Note that the order 1 puts '4' on the same position as '1', but with an @@ -190,9 +191,8 @@ def test_dashboards_deploys_dashboard_with_order_overwrite_in_dashboard_yaml(ws, (tmp_path / "dashboard.yml").write_text(content) for query_name in range(6): (tmp_path / f"query_{query_name}.sql").write_text(f"SELECT {query_name} AS count") - - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -200,11 +200,12 @@ def test_dashboards_deploys_dashboard_with_order_overwrite_in_dashboard_yaml(ws, def test_dashboard_deploys_dashboard_with_table(ws, make_dashboard): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() dashboard_folder = Path(__file__).parent / "dashboards" / "one_table" - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(dashboard_folder) + dashboard_metadata = DashboardMetadata.from_path(dashboard_folder) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -212,16 +213,14 @@ def test_dashboard_deploys_dashboard_with_table(ws, make_dashboard): def test_dashboards_deploys_dashboard_with_invalid_query(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() 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") - with (tmp_path / "4.sql").open("w") as f: - f.write("SELECT COUNT(* AS invalid_column") - - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + (tmp_path / f"{query_name}.sql").write_text(f"SELECT {query_name} AS count") + (tmp_path / "4.sql").write_text("SELECT COUNT(* AS invalid_column") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -229,16 +228,14 @@ def test_dashboards_deploys_dashboard_with_invalid_query(ws, make_dashboard, tmp def test_dashboards_deploys_dashboard_with_markdown_header(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() for count, query_name in enumerate("abcdef"): (tmp_path / f"{query_name}.sql").write_text(f"SELECT {count} AS count") - - description = "---\norder: -1\n---\nBelow you see counters." - (tmp_path / "z_description.md").write_text(description) - - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + (tmp_path / "z_description.md").write_text("---\norder: -1\n---\nBelow you see counters.") + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -246,13 +243,13 @@ def test_dashboards_deploys_dashboard_with_markdown_header(ws, make_dashboard, t def test_dashboards_deploys_dashboard_with_widget_title_and_description(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() description = "-- --title 'Counting' --description 'The answer to life'\nSELECT 42" (tmp_path / "counter.sql").write_text(description) - - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -260,6 +257,7 @@ def test_dashboards_deploys_dashboard_with_widget_title_and_description(ws, make def test_dashboards_deploys_dashboard_from_query_with_cte(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() table_query_path = Path(__file__).parent / "dashboards/one_table/databricks_office_locations.sql" @@ -270,9 +268,8 @@ def test_dashboards_deploys_dashboard_from_query_with_cte(ws, make_dashboard, tm "SELECT Address, State, Country FROM data" ) (tmp_path / "table.sql").write_text(query_with_cte) - - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -280,14 +277,14 @@ def test_dashboards_deploys_dashboard_from_query_with_cte(ws, make_dashboard, tm def test_dashboards_deploys_dashboard_with_filters(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() table_query_path = Path(__file__).parent / "dashboards/one_table/databricks_office_locations.sql" office_locations = table_query_path.read_text() (tmp_path / "table.sql").write_text(f"-- --width 2 --filter City State Country\n{office_locations}") - - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) @@ -295,13 +292,13 @@ def test_dashboards_deploys_dashboard_with_filters(ws, make_dashboard, tmp_path) def test_dashboard_deploys_dashboard_with_empty_title(ws, make_dashboard, tmp_path): + dashboards = Dashboards(ws) sdk_dashboard = make_dashboard() query = '-- --overrides \'{"spec": {"frame": {"showTitle": true}}}\'\nSELECT 102132 AS count' (tmp_path / "counter.sql").write_text(query) - - dashboards = Dashboards(ws) - lakeview_dashboard = dashboards.create_dashboard(tmp_path) + dashboard_metadata = DashboardMetadata.from_path(tmp_path) + lakeview_dashboard = dashboards.create_dashboard(dashboard_metadata) sdk_dashboard = dashboards.deploy_dashboard(lakeview_dashboard, dashboard_id=sdk_dashboard.dashboard_id) From 46de0bb1af277b9dc4fee00e4e217e4b0442c459 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 17:08:17 +0200 Subject: [PATCH 09/18] Add content to Tile --- src/databricks/labs/lsql/dashboards.py | 20 ++++++++++---------- tests/unit/test_dashboards.py | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 00d9101c..d7dbc246 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -302,6 +302,7 @@ class Tile: """A dashboard tile.""" metadata: TileMetadata + content: str = "" _position: Position = Position(0, 0, 0, 0) @@ -313,8 +314,7 @@ def position(self) -> Position: def get_layouts(self) -> Iterable[Layout]: """Get the layout(s) reflecting this tile in the dashboard.""" - _, text = self.metadata.handler.split() - widget = Widget(name=self.metadata.id, textbox_spec=text) + widget = Widget(name=self.metadata.id, textbox_spec=self.content) layout = Layout(widget=widget, position=self.position) yield layout @@ -336,15 +336,16 @@ def place_after(self, position: Position) -> None: @classmethod def from_tile_metadata(cls, tile_metadata: TileMetadata) -> "Tile": """Create a tile given the tile metadata.""" + _, content = tile_metadata.handler.split() if tile_metadata.is_markdown(): - return MarkdownTile(tile_metadata) - query_tile = QueryTile(tile_metadata) + return MarkdownTile(tile_metadata, content) + query_tile = QueryTile(tile_metadata, content) spec_type = query_tile.infer_spec_type() if spec_type is None: - return MarkdownTile(tile_metadata) + return MarkdownTile(tile_metadata, content) if spec_type == CounterSpec: - return CounterTile(tile_metadata) - return TableTile(tile_metadata) + return CounterTile(tile_metadata, content) + return TableTile(tile_metadata, content) def __repr__(self): return f"Tile<{self.metadata.id}>" @@ -365,11 +366,10 @@ class QueryTile(Tile): _FILTER_HEIGHT = 1 def _get_abstract_syntax_tree(self) -> sqlglot.Expression | None: - _, query = self.metadata.handler.split() try: - return sqlglot.parse_one(query, dialect=self._DIALECT) + return sqlglot.parse_one(self.content, dialect=self._DIALECT) except sqlglot.ParseError as e: - logger.warning(f"Parsing {query}: {e}") + logger.warning(f"Parsing {self.content}: {e}") return None def _get_query(self) -> str: diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 70a65884..74765d2d 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -715,7 +715,7 @@ def test_query_tile_finds_fields(tmp_path, query, names): query_file.write_text(query) tile_metadata = TileMetadata(query_file, 1, 1, 1) - tile = QueryTile(tile_metadata) + tile = QueryTile.from_tile_metadata(tile_metadata) fields = tile._find_fields() # pylint: disable=protected-access From a746993cd3ccaa5718c6c09b37590d1844f1a217 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 17:49:48 +0200 Subject: [PATCH 10/18] Refactor DashboardMetadata to have tiles instead of tile metadata --- src/databricks/labs/lsql/dashboards.py | 104 +++++++++++++------------ tests/unit/test_dashboards.py | 67 ++++++++-------- 2 files changed, 89 insertions(+), 82 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index d7dbc246..6a7092a8 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -97,8 +97,8 @@ def _get_arguments_parser() -> ArgumentParser: parser = ArgumentParser("TileMetadata", add_help=False, exit_on_error=False) parser.add_argument("--id", type=str) parser.add_argument("-o", "--order", type=int) - parser.add_argument("-w", "--width", type=int) - parser.add_argument("-h", "--height", type=int) + parser.add_argument("-w", "--width", type=int, default=0) + parser.add_argument("-h", "--height", type=int, default=0) parser.add_argument("-t", "--title", type=str, default="") parser.add_argument("-d", "--description", type=str, default="") parser.add_argument( @@ -124,6 +124,7 @@ def _get_arguments_parser() -> ArgumentParser: "--overrides", type=json.loads, help="Override the low-level Lakeview entities with a json payload.", + default={}, ) return parser @@ -600,10 +601,33 @@ class DashboardMetadata: """The metadata defining a lakeview dashboard""" display_name: str # The dashboard display name - tile_metadatas: list[TileMetadata] = dataclasses.field(default_factory=list) # The tile metadata objects # A sqlglot transformer applied on the queries before creating the tiles. If None, no transformation is applied query_transformer: Callable[[sqlglot.Expression], sqlglot.Expression] | None = None + _tiles: list[Tile] = dataclasses.field(default_factory=list) # The dashboard tiles + + @property + def tiles(self) -> list[Tile]: + """The ordered dashboard tiles + + The order of the tiles is by default the alphanumerically sorted tile ids, however, the order may be overwritten + with the `order` key. Hence, the logic to: + i) set the order when not specified; + ii) sort the tiles using the order field. + """ + tiles_with_order = [] + for default_order, tile in enumerate(sorted(self._tiles, key=lambda tile: tile.metadata.id)): + order = tile.metadata.order if tile.metadata.order is not None else default_order + tiles_with_order.append((order, tile)) + tiles, position = [], Position(0, 0, 0, 0) # Position of first tile + for _, tile in sorted(tiles_with_order, key=lambda el: (el[0], el[1].metadata.id)): + if isinstance(tile, QueryTile): + tile.query_transformer = self.query_transformer + tile.place_after(position) + tiles.append(tile) + position = tile.position + return tiles + def validate(self) -> None: """Validate the dashboard metadata. @@ -611,10 +635,10 @@ def validate(self) -> None: ValueError : If the dashboard metadata is invalid. """ tile_ids = [] - for tile in self.tile_metadatas: - if tile.path is None: - raise ValueError(f"Tile path is required: {tile}") - tile_ids.append(tile.id) + for tile in self._tiles: + if len(tile.content) == 0: + raise ValueError(f"Tile has empty content: {tile}") + tile_ids.append(tile.metadata.id) counter = collections.Counter(tile_ids) for tile_id, id_count in counter.items(): if id_count > 1: @@ -634,19 +658,20 @@ def update(self, other: "DashboardMetadata") -> None: self.display_name = other.display_name tile_metadatas = [] - metadata_mapping = {tile.id: tile for tile in self.tile_metadatas} - for tile_metadata in other.tile_metadatas: - tile_metadata_existing = metadata_mapping.get(tile_metadata.id) + metadata_mapping = {tile.metadata.id: tile.metadata for tile in self._tiles} + for tile in other.tiles: + tile_metadata_existing = metadata_mapping.get(tile.metadata.id) if tile_metadata_existing is not None: - tile_metadata_existing.update(tile_metadata) + tile_metadata_existing.update(tile.metadata) tile_metadatas.append(tile_metadata_existing) else: - tile_metadatas.append(tile_metadata) - metadata_mapping_other = {tile.id: tile for tile in other.tile_metadatas} - for tile_metadata in self.tile_metadatas: - if tile_metadata.id not in metadata_mapping_other: - tile_metadatas.append(tile_metadata) - self.tile_metadatas = tile_metadatas + tile_metadatas.append(tile.metadata) + metadata_mapping_other = {tile.metadata.id: tile.metadata for tile in other._tiles} + for tile in self._tiles: + if tile.metadata.id not in metadata_mapping_other: + tile_metadatas.append(tile.metadata) + tiles = [Tile.from_tile_metadata(tile_metadata) for tile_metadata in tile_metadatas] + self._tiles = tiles def replace_database( self, @@ -683,32 +708,10 @@ def replace_catalog_and_database_in_query(node: sqlglot.Expression) -> sqlglot.E return dataclasses.replace(self, query_transformer=replace_catalog_and_database_in_query) - def _get_tiles(self) -> list[Tile]: - """Get the tiles from the tiles metadata. - - The order of the tiles is by default the alphanumerically sorted tile ids, however, the order may be overwritten - with the `order` key. Hence, the logic to: - i) set the order when not specified; - ii) sort the tiles using the order field. - """ - tile_metadatas_with_order = [] - for default_order, tile_metadata in enumerate(sorted(self.tile_metadatas, key=lambda wm: wm.id)): - order = tile_metadata.order if tile_metadata.order is not None else default_order - tile_metadatas_with_order.append(dataclasses.replace(tile_metadata, order=order)) - tiles, position = [], Position(0, 0, 0, 0) # Position of first tile - for tile_metadata in sorted(tile_metadatas_with_order, key=lambda t: (t.order, t.id)): - tile = Tile.from_tile_metadata(tile_metadata) - if isinstance(tile, QueryTile): - tile.query_transformer = self.query_transformer - tile.place_after(position) - tiles.append(tile) - position = tile.position - return tiles - def get_datasets(self) -> list[Dataset]: """Get the datasets for the dashboard.""" datasets: list[Dataset] = [] - for tile in self._get_tiles(): + for tile in self.tiles: if isinstance(tile, QueryTile): datasets.extend(tile.get_datasets()) return datasets @@ -716,7 +719,7 @@ def get_datasets(self) -> list[Dataset]: def get_layouts(self) -> list[Layout]: """Get the layouts for the dashboard.""" layouts: list[Layout] = [] - for tile in self._get_tiles(): + for tile in self.tiles: layouts.extend(tile.get_layouts()) return layouts @@ -728,24 +731,25 @@ def from_dict(cls, raw: dict) -> "DashboardMetadata": if not isinstance(tile_raw, dict): logger.warning(f"Parsing invalid tile metadata in dashboard.yml: tiles.{tile_id}.{tile_raw}") continue - tile = TileMetadata(id=tile_id) + tile_metadata = TileMetadata(id=tile_id) for tile_key, tile_value in tile_raw.items(): if tile_key == "id": logger.warning(f"Parsing unsupported field in dashboard.yml: tiles.{tile_id}.id") continue try: - tile_new = TileMetadata.from_dict({tile_key: tile_value}) - tile.update(tile_new) + tile_metadata_new = TileMetadata.from_dict({tile_key: tile_value}) + tile_metadata.update(tile_metadata_new) except TypeError: logger.warning(f"Parsing unsupported field in dashboard.yml: tiles.{tile_id}.{tile_key}") continue + tile = Tile.from_tile_metadata(tile_metadata) tiles.append(tile) - return cls(display_name=display_name, tile_metadatas=tiles) + return cls(display_name=display_name, _tiles=tiles) def as_dict(self) -> dict: raw: dict = {"display_name": self.display_name} - if self.tile_metadatas: - raw["tiles"] = {tile.id: tile.as_dict() for tile in self.tile_metadatas} + if self.tiles: + raw["tiles"] = {tile.metadata.id: tile.metadata.as_dict() for tile in self.tiles} return raw @classmethod @@ -782,8 +786,10 @@ def _from_dashboard_folder(cls, folder: Path) -> "DashboardMetadata": for path in folder.iterdir(): if path.suffix not in {".sql", ".md"}: continue - tiles.append(TileMetadata.from_path(path)) - return cls(display_name=folder.name, tile_metadatas=tiles) + tile_metadata = TileMetadata.from_path(path) + tile = Tile.from_tile_metadata(tile_metadata) + tiles.append(tile) + return cls(display_name=folder.name, _tiles=tiles) class Dashboards: diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 74765d2d..c20efcce 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -46,12 +46,21 @@ def test_dashboard_metadata_sets_display_name_from_dict(): assert dashboard_metadata.display_name == "test" -def test_dashboard_metadata_sets_tiles_from_dict(): - tile_metadata = TileMetadata(Path("test.sql")) - raw = {"display_name": "test", "tiles": {"test": {"path": "test.sql"}}} +def test_dashboard_metadata_from_path_raises_not_implemented_error_for_select_star(tmp_path): + (tmp_path / "star.sql").write_text("SELECT * FROM table") + with pytest.raises(NotImplementedError) as e: + DashboardMetadata.from_path(tmp_path) + assert "star" in str(e) + + +def test_dashboard_metadata_sets_tiles_from_dict(tmp_path): + query_path = tmp_path / "test.sql" + query_path.touch() + tile_metadata = TileMetadata.from_path(query_path) + raw = {"display_name": "test", "tiles": {"test": {"path": query_path.as_posix()}}} dashboard_metadata = DashboardMetadata.from_dict(raw) - assert len(dashboard_metadata.tile_metadatas) == 1 - assert dashboard_metadata.tile_metadatas[0] == tile_metadata + assert len(dashboard_metadata.tiles) == 1 + assert dashboard_metadata.tiles[0].metadata == tile_metadata def test_dashboard_metadata_ignores_id_overwrite(caplog): @@ -60,20 +69,24 @@ def test_dashboard_metadata_ignores_id_overwrite(caplog): with caplog.at_level(logging.WARNING, logger="databricks.labs.lsql.dashboards"): dashboard_metadata = DashboardMetadata.from_dict(raw) - assert len(dashboard_metadata.tile_metadatas) == 1 - assert dashboard_metadata.tile_metadatas[0].id == "test" + assert len(dashboard_metadata.tiles) == 1 + assert dashboard_metadata.tiles[0].metadata.id == "test" assert "Parsing unsupported field in dashboard.yml: tiles.test.id" in caplog.text -def test_dashboard_metadata_from_and_as_dict_is_a_unit_function(): - raw_tile = {"path": "test.sql", "id": "test", "height": 0, "width": 0, "widget_type": "AUTO"} +def test_dashboard_metadata_from_and_as_dict_is_a_unit_function(tmp_path): + query_path = tmp_path / "test.sql" + query_path.touch() + raw_tile = {"path": query_path.as_posix(), "id": "test", "height": 0, "width": 0, "widget_type": "AUTO"} raw = {"display_name": "test", "tiles": {"test": raw_tile}} dashboard_metadata = DashboardMetadata.from_dict(raw) assert dashboard_metadata.as_dict() == raw def test_dashboard_metadata_from_raw(tmp_path): - raw_tile = {"path": "test.sql", "id": "test", "height": 0, "width": 0, "widget_type": "AUTO", "order": 0} + query_path = tmp_path / "test.sql" + query_path.touch() + raw_tile = {"path": query_path.as_posix(), "id": "test", "height": 0, "width": 0, "widget_type": "AUTO", "order": 0} raw = {"display_name": "test", "tiles": {"test": raw_tile}} path = tmp_path / "dashboard.yml" @@ -123,11 +136,11 @@ def test_dashboard_metadata_handles_partial_invalid_yml(tmp_path, caplog): dashboard_metadata = DashboardMetadata.from_path(tmp_path) assert dashboard_metadata.display_name == "name" - assert len(dashboard_metadata.tile_metadatas) == 2 - assert dashboard_metadata.tile_metadatas[0].id == "correct" - assert dashboard_metadata.tile_metadatas[0].order == 1 - assert dashboard_metadata.tile_metadatas[1].id == "partial_correct" - assert dashboard_metadata.tile_metadatas[1].order == 3 + assert len(dashboard_metadata.tiles) == 2 + assert dashboard_metadata.tiles[0].metadata.id == "correct" + assert dashboard_metadata.tiles[0].metadata.order == 1 + assert dashboard_metadata.tiles[1].metadata.id == "partial_correct" + assert dashboard_metadata.tiles[1].metadata.order == 3 assert "Parsing invalid tile metadata in dashboard.yml: tiles.incorrect.[{'order': 2}]" in caplog.text assert "Parsing unsupported field in dashboard.yml: tiles.partial_correct.non_existing_key" in caplog.text @@ -141,7 +154,7 @@ def test_dashboard_metadata_validate_valid(tmp_path): order: 1 """.lstrip() (tmp_path / "dashboard.yml").write_text(dashboard_content) - (tmp_path / "correct.sql").touch() + (tmp_path / "correct.sql").write_text("SELECT 1") dashboard_metadata = DashboardMetadata.from_path(tmp_path) @@ -167,11 +180,11 @@ def test_dashboard_metadata_validate_misses_tile_path(tmp_path): with pytest.raises(ValueError) as e: dashboard_metadata.validate() - assert "Tile path is required: TileMetadata" in str(e.value) + assert "Tile has empty content:" in str(e.value) def test_dashboard_metadata_validate_finds_duplicate_query_id(tmp_path): - (tmp_path / "query.sql").touch() + (tmp_path / "query.sql").write_text("SELECT 1") query_content = """-- --id query\nSELECT 1""" (tmp_path / "not_query.sql").write_text(query_content) @@ -183,8 +196,8 @@ def test_dashboard_metadata_validate_finds_duplicate_query_id(tmp_path): def test_dashboard_metadata_validate_finds_duplicate_widget_id(tmp_path): - (tmp_path / "widget.sql").touch() - (tmp_path / "widget.md").touch() + (tmp_path / "widget.sql").write_text("SELECT 1") + (tmp_path / "widget.md").write_text("# Widget") dashboard_metadata = DashboardMetadata.from_path(tmp_path) @@ -266,7 +279,7 @@ def test_query_handler_ignores_comment_on_other_lines(tmp_path, query): header = handler.parse_header() - assert header["width"] is None + assert header["width"] == 0 assert header["height"] == 5 @@ -1243,18 +1256,6 @@ def test_dashboard_handles_incorrect_query_header(tmp_path, caplog): ws.assert_not_called() -def test_create_dashboard_raises_not_implemented_error_for_select_star(tmp_path): - ws = create_autospec(WorkspaceClient) - (tmp_path / "star.sql").write_text("SELECT * FROM table") - dashboard_metadata = DashboardMetadata.from_path(tmp_path) - - with pytest.raises(NotImplementedError) as e: - Dashboards(ws).create_dashboard(dashboard_metadata) - - assert "star" in str(e) - ws.assert_not_called() - - def test_dashboard_creates_dashboard_based_on_markdown_header(tmp_path): ws = create_autospec(WorkspaceClient) From 3023afa045cd8fb8b30a4a1ad340dc5c86705f1d Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 17:56:32 +0200 Subject: [PATCH 11/18] Make Tile content hidden --- src/databricks/labs/lsql/dashboards.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 6a7092a8..cea747d7 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -303,10 +303,17 @@ class Tile: """A dashboard tile.""" metadata: TileMetadata - content: str = "" + _content: str = "" _position: Position = Position(0, 0, 0, 0) + @property + def content(self) -> str: + if len(self._content) == 0: + _, content = self.metadata.handler.split() + self._content = content + return self._content + @property def position(self) -> Position: width = self.metadata.width or self._position.width @@ -337,16 +344,15 @@ def place_after(self, position: Position) -> None: @classmethod def from_tile_metadata(cls, tile_metadata: TileMetadata) -> "Tile": """Create a tile given the tile metadata.""" - _, content = tile_metadata.handler.split() if tile_metadata.is_markdown(): - return MarkdownTile(tile_metadata, content) - query_tile = QueryTile(tile_metadata, content) + return MarkdownTile(tile_metadata) + query_tile = QueryTile(tile_metadata) spec_type = query_tile.infer_spec_type() if spec_type is None: - return MarkdownTile(tile_metadata, content) + return MarkdownTile(tile_metadata) if spec_type == CounterSpec: - return CounterTile(tile_metadata, content) - return TableTile(tile_metadata, content) + return CounterTile(tile_metadata) + return TableTile(tile_metadata) def __repr__(self): return f"Tile<{self.metadata.id}>" From b97a63c0196468b2fd915b284f9cda3449e59b19 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 18:06:52 +0200 Subject: [PATCH 12/18] Remove update from DashboardMetadata --- src/databricks/labs/lsql/dashboards.py | 51 +++++++++++--------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index cea747d7..e8db354d 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -650,34 +650,23 @@ def validate(self) -> None: if id_count > 1: raise ValueError(f"Duplicate id: {tile_id}") - def update(self, other: "DashboardMetadata") -> None: - """Update the dashboard metadata with another dashboard metadata. - - The other takes precedence, similar to merging dictionaries. - - Resources: - - https://docs.python.org/3/library/stdtypes.html#dict.update : Similar to the update method of a dictionary. - """ - if not isinstance(other, DashboardMetadata): - raise TypeError(f"Can not merge with {other}") - - self.display_name = other.display_name - - tile_metadatas = [] - metadata_mapping = {tile.metadata.id: tile.metadata for tile in self._tiles} - for tile in other.tiles: - tile_metadata_existing = metadata_mapping.get(tile.metadata.id) - if tile_metadata_existing is not None: - tile_metadata_existing.update(tile.metadata) - tile_metadatas.append(tile_metadata_existing) + @classmethod + def _merge_tile_metadatas(cls, left: list[TileMetadata], right: list[TileMetadata]) -> list[TileMetadata]: + """Merge tile metdatas where the right takes precedence over the left.""" + metadata_mapping_left = {metadata.id: metadata for metadata in left} + metadata_mapping_right = {metadata.id: metadata for metadata in right} + metadatas = [] + for metadata in right: + metadata_existing = metadata_mapping_left.get(metadata.id) + if metadata_existing is not None: + metadata_existing.update(metadata) + metadatas.append(metadata_existing) else: - tile_metadatas.append(tile.metadata) - metadata_mapping_other = {tile.metadata.id: tile.metadata for tile in other._tiles} - for tile in self._tiles: - if tile.metadata.id not in metadata_mapping_other: - tile_metadatas.append(tile.metadata) - tiles = [Tile.from_tile_metadata(tile_metadata) for tile_metadata in tile_metadatas] - self._tiles = tiles + metadatas.append(metadata) + for metadata in left: + if metadata.id not in metadata_mapping_right: + metadatas.append(metadata) + return metadatas def replace_database( self, @@ -762,10 +751,12 @@ def as_dict(self) -> dict: def from_path(cls, path: Path) -> "DashboardMetadata": """Read the dashboard metadata from the dashboard folder.""" dashboard_metadata_yml = cls._from_dashboard_path(path / "dashboard.yml") - display_name = dashboard_metadata_yml.display_name # Display name in dashboard.yml takes precedence dashboard_metadata_folder = cls._from_dashboard_folder(path) - dashboard_metadata_yml.update(dashboard_metadata_folder) # Metadata from file headers takes precedence - return dataclasses.replace(dashboard_metadata_yml, display_name=display_name) + tile_metadatas_yml = [tile.metadata for tile in dashboard_metadata_yml.tiles] + tile_metadatas_folder = [tile.metadata for tile in dashboard_metadata_folder.tiles] + tile_metadatas = cls._merge_tile_metadatas(tile_metadatas_yml, tile_metadatas_folder) + tiles = [Tile.from_tile_metadata(tile_metadata) for tile_metadata in tile_metadatas] + return cls(dashboard_metadata_yml.display_name, _tiles=tiles) @classmethod def _from_dashboard_path(cls, path: Path) -> "DashboardMetadata": From cc4ce27314e1dd792d3c67c9aab65a1b88baa1ba Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Wed, 10 Jul 2024 18:16:57 +0200 Subject: [PATCH 13/18] Move replace_database to QueryTile --- src/databricks/labs/lsql/dashboards.py | 113 ++++++++++++------------- tests/unit/test_dashboards.py | 2 +- 2 files changed, 57 insertions(+), 58 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index e8db354d..ef1b7b88 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -379,23 +379,6 @@ def _get_abstract_syntax_tree(self) -> sqlglot.Expression | None: logger.warning(f"Parsing {self.content}: {e}") return None - def _get_query(self) -> str: - _, query = self.metadata.handler.split() - if self.query_transformer is None: - return query - syntax_tree = self._get_abstract_syntax_tree() - if syntax_tree is None: - return query - query_transformed = syntax_tree.transform(self.query_transformer).sql( - dialect=self._DIALECT, - # A transformer requires to (re)define how to output SQL - normalize=True, # normalize identifiers to lowercase - pretty=True, # format the produced SQL string - normalize_functions="upper", # normalize function names to uppercase - max_text_width=80, # wrap text at 80 characters - ) - return query_transformed - def _find_fields(self) -> list[Field]: """Find the fields in a query. @@ -416,6 +399,52 @@ def _find_fields(self) -> list[Field]: fields.append(field) return fields + def replace_database( + self, + catalog: str | None = None, + database: str | None = None, + *, + catalog_to_replace: str | None = None, + database_to_replace: str | None = None, + ) -> "QueryTile": + """Replace the database in the query. + + Parameters : + catalog : str + The value to replace the catalog with + database : str + The value to replace the database with + catalog_to_replace : str | None (default: None) + The catalog to replace, if None, all catalogs are replaced + database_to_replace : str | None (default: None) + The database to replace, if None, all databases are replaced + """ + + def replace_catalog_and_database_in_query(node: sqlglot.Expression) -> sqlglot.Expression: + if isinstance(node, sqlglot.exp.Table): + if node.args.get("catalog") is not None and ( + catalog_to_replace is None or getattr(node.args.get("catalog"), "this", "") == catalog_to_replace + ): + node.args["catalog"].set("this", catalog) + if node.args.get("db") is not None and ( + database_to_replace is None or getattr(node.args.get("db"), "this", "") == database_to_replace + ): + node.args["db"].set("this", database) + return node + + syntax_tree = self._get_abstract_syntax_tree() + if syntax_tree is None: + return dataclasses.replace(self, _content=self.content) + content_transformed = syntax_tree.transform(replace_catalog_and_database_in_query).sql( + dialect=self._DIALECT, + # A transformer requires to (re)define how to output SQL + normalize=True, # normalize identifiers to lowercase + pretty=True, # format the produced SQL string + normalize_functions="upper", # normalize function names to uppercase + max_text_width=80, # wrap text at 80 characters + ) + return dataclasses.replace(self, _content=content_transformed) + def infer_spec_type(self) -> type[WidgetSpec] | None: """Infer the spec type from the query.""" if self.metadata.widget_type != WidgetType.AUTO: @@ -429,8 +458,7 @@ def infer_spec_type(self) -> type[WidgetSpec] | None: def get_datasets(self) -> Iterable[Dataset]: """Get the dataset belonging to the query.""" - query = self._get_query() - dataset = Dataset(name=self.metadata.id, display_name=self.metadata.id, query=query) + dataset = Dataset(name=self.metadata.id, display_name=self.metadata.id, query=self.content) yield dataset def _merge_nested_dictionaries(self, left: dict, right: dict) -> dict: @@ -607,8 +635,6 @@ class DashboardMetadata: """The metadata defining a lakeview dashboard""" display_name: str # The dashboard display name - # A sqlglot transformer applied on the queries before creating the tiles. If None, no transformation is applied - query_transformer: Callable[[sqlglot.Expression], sqlglot.Expression] | None = None _tiles: list[Tile] = dataclasses.field(default_factory=list) # The dashboard tiles @@ -627,8 +653,6 @@ def tiles(self) -> list[Tile]: tiles_with_order.append((order, tile)) tiles, position = [], Position(0, 0, 0, 0) # Position of first tile for _, tile in sorted(tiles_with_order, key=lambda el: (el[0], el[1].metadata.id)): - if isinstance(tile, QueryTile): - tile.query_transformer = self.query_transformer tile.place_after(position) tiles.append(tile) position = tile.position @@ -668,40 +692,15 @@ def _merge_tile_metadatas(cls, left: list[TileMetadata], right: list[TileMetadat metadatas.append(metadata) return metadatas - def replace_database( - self, - catalog: str | None = None, - database: str | None = None, - *, - catalog_to_replace: str | None = None, - database_to_replace: str | None = None, - ) -> "DashboardMetadata": - """Replace the database in the queries. - - Parameters : - catalog : str - The value to replace the catalog with - database : str - The value to replace the database with - catalog_to_replace : str | None (default: None) - The catalog to replace, if None, all catalogs are replaced - database_to_replace : str | None (default: None) - The database to replace, if None, all databases are replaced - """ - - def replace_catalog_and_database_in_query(node: sqlglot.Expression) -> sqlglot.Expression: - if isinstance(node, sqlglot.exp.Table): - if node.args.get("catalog") is not None and ( - catalog_to_replace is None or getattr(node.args.get("catalog"), "this", "") == catalog_to_replace - ): - node.args["catalog"].set("this", catalog) - if node.args.get("db") is not None and ( - database_to_replace is None or getattr(node.args.get("db"), "this", "") == database_to_replace - ): - node.args["db"].set("this", database) - return node - - return dataclasses.replace(self, query_transformer=replace_catalog_and_database_in_query) + def replace_database(self, *args, **kwargs) -> "DashboardMetadata": + """Wrapper around :method:QueryTile.replace_database""" + tiles: list[Tile] = [] + for tile in self.tiles: + if isinstance(tile, QueryTile): + tiles.append(tile.replace_database(*args, **kwargs)) + else: + tiles.append(tile) + return dataclasses.replace(self, _tiles=tiles) def get_datasets(self) -> list[Dataset]: """Get the datasets for the dashboard.""" diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index c20efcce..9c3c4ae0 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -741,7 +741,7 @@ def test_query_tile_keeps_original_query(tmp_path): query_path.write_text(query) tile_metadata = TileMetadata.from_path(query_path) - query_tile = QueryTile(tile_metadata) + query_tile = QueryTile.from_tile_metadata(tile_metadata) dataset = next(query_tile.get_datasets()) From 3c4ae396b63ec358ec5a4dd994426e50f8cb72b1 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Thu, 11 Jul 2024 08:35:45 +0200 Subject: [PATCH 14/18] Update command description --- labs.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/labs.yml b/labs.yml index e9c86f68..897f6879 100644 --- a/labs.yml +++ b/labs.yml @@ -11,11 +11,11 @@ commands: description: The folder with dashboard files. By default, the current working directory. - name: catalog description: | - Overwrite the catalog in query with given value. Useful when developing with seperated catalogs, for - example, for production and development. + Overwrite the catalog in the queries' `FROM` clauses with given value. + Useful when developing with seperated catalogs, for example, for production and development. - name: database description: | - Overwrite the database in query with given value. Useful when developing with seperated databases, for - example, for production and development. + Overwrite the database in the queries' `FROM` clauses with given value. + Useful when developing with seperated databases, for example, for production and development. - name: no-open description: Do not open the dashboard in the browser after creating From 2cb6de770b7a58a0d56dd949c3c452d110bd9b12 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Thu, 11 Jul 2024 08:37:10 +0200 Subject: [PATCH 15/18] Update docstring --- 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 ef1b7b88..ea8dd032 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -407,7 +407,7 @@ def replace_database( catalog_to_replace: str | None = None, database_to_replace: str | None = None, ) -> "QueryTile": - """Replace the database in the query. + """Replace the catalog and/or database in the query. Parameters : catalog : str From 26e24b9a9cb2c64f02241fa7e23536c39ec49bd4 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Thu, 11 Jul 2024 08:39:36 +0200 Subject: [PATCH 16/18] Loop over tiles --- 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 ea8dd032..96fe0e67 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -665,7 +665,7 @@ def validate(self) -> None: ValueError : If the dashboard metadata is invalid. """ tile_ids = [] - for tile in self._tiles: + for tile in self.tiles: if len(tile.content) == 0: raise ValueError(f"Tile has empty content: {tile}") tile_ids.append(tile.metadata.id) From 9d8d3749b82e0805cdb1665d811f82b3074aace6 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Thu, 11 Jul 2024 08:45:42 +0200 Subject: [PATCH 17/18] Refactor TileMetadata.update to merge --- src/databricks/labs/lsql/dashboards.py | 34 ++++++++++++++------------ tests/unit/test_dashboards.py | 10 ++++---- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index 96fe0e67..ba720c35 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -217,8 +217,8 @@ def __post_init__(self): if not self.id: self.id = self.path.stem if self.path is not None else "" - def update(self, other: "TileMetadata") -> None: - """Update the tile metadata with another tile metadata. + def merge(self, other: "TileMetadata") -> "TileMetadata": + """Merge the tile metadata with another tile metadata. Precedence: - The other takes precedences, similar to merging dictionaries. @@ -232,16 +232,20 @@ def update(self, other: "TileMetadata") -> None: widget_type = other.widget_type if other.widget_type != WidgetType.AUTO else self.widget_type - self.path = other.path or self.path - self.order = other.order if other.order is not None else self.order - self.width = other.width or self.width - self.height = other.height or self.height - self.id = other.id or self.id - self.title = other.title or self.title - self.description = other.description or self.description - self.widget_type = widget_type - self.filters = other.filters or self.filters - self.overrides = other.overrides or self.overrides + merged = dataclasses.replace( + self, + path=other.path or self.path, + order=other.order if other.order is not None else self.order, + width=other.width or self.width, + height=other.height or self.height, + id=other.id or self.id, + title=other.title or self.title, + description=other.description or self.description, + widget_type=widget_type, + filters=other.filters or self.filters, + overrides=other.overrides or self.overrides, + ) + return merged def is_markdown(self) -> bool: return self.path is not None and self.path.suffix == ".md" @@ -683,8 +687,8 @@ def _merge_tile_metadatas(cls, left: list[TileMetadata], right: list[TileMetadat for metadata in right: metadata_existing = metadata_mapping_left.get(metadata.id) if metadata_existing is not None: - metadata_existing.update(metadata) - metadatas.append(metadata_existing) + metadata_merged = metadata_existing.merge(metadata) + metadatas.append(metadata_merged) else: metadatas.append(metadata) for metadata in left: @@ -732,7 +736,7 @@ def from_dict(cls, raw: dict) -> "DashboardMetadata": continue try: tile_metadata_new = TileMetadata.from_dict({tile_key: tile_value}) - tile_metadata.update(tile_metadata_new) + tile_metadata = tile_metadata.merge(tile_metadata_new) except TypeError: logger.warning(f"Parsing unsupported field in dashboard.yml: tiles.{tile_id}.{tile_key}") continue diff --git a/tests/unit/test_dashboards.py b/tests/unit/test_dashboards.py index 9c3c4ae0..d063544d 100644 --- a/tests/unit/test_dashboards.py +++ b/tests/unit/test_dashboards.py @@ -221,11 +221,11 @@ def test_tile_metadata_is_query(): def test_tile_metadata_merges(): left = TileMetadata(Path("left.sql"), filters=["a"], width=10, widget_type=WidgetType.TABLE) right = TileMetadata(Path("right.sql"), widget_type=WidgetType.COUNTER) - left.update(right) - assert left.id == "right" - assert left.width == 10 - assert left.filters == ["a"] - assert left.widget_type == WidgetType.COUNTER + merged = left.merge(right) + assert merged.id == "right" + assert merged.width == 10 + assert merged.filters == ["a"] + assert merged.widget_type == WidgetType.COUNTER def test_base_handler_parses_empty_header(tmp_path): From 2563c961107154d20c1070098c1f938f365320c6 Mon Sep 17 00:00:00 2001 From: Cor Zuurmond Date: Thu, 11 Jul 2024 09:08:57 +0200 Subject: [PATCH 18/18] Add comment --- src/databricks/labs/lsql/dashboards.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/databricks/labs/lsql/dashboards.py b/src/databricks/labs/lsql/dashboards.py index ba720c35..7c936bb6 100644 --- a/src/databricks/labs/lsql/dashboards.py +++ b/src/databricks/labs/lsql/dashboards.py @@ -730,6 +730,7 @@ def from_dict(cls, raw: dict) -> "DashboardMetadata": logger.warning(f"Parsing invalid tile metadata in dashboard.yml: tiles.{tile_id}.{tile_raw}") continue tile_metadata = TileMetadata(id=tile_id) + # The loop below allows for partial parsing by skipping unsupported fields for tile_key, tile_value in tile_raw.items(): if tile_key == "id": logger.warning(f"Parsing unsupported field in dashboard.yml: tiles.{tile_id}.id")