Extend replace database in query #210

JCZuurmond · 2024-07-10T09:13:11Z

Improve the database replacement in query by:

Making it part of the DashboardMetadata class
Allow specifying which database to replace
Support replacing catalog
Allow specifying which catalog to replace

See https://github.com/databrickslabs/ucx/pull/1920/files/fae429e4d73eceebd7259c9bcaa390e6c7aa4395#r1670960522

Partially resolves #212

github-actions · 2024-07-10T09:18:50Z

✅ 35/35 passed, 7 flaky, 2 skipped, 25m25s total

Flaky tests:

🤪 test_dashboard_deploys_dashboard_with_display_name (10.166s)
🤪 test_dashboard_deploys_dashboard_with_counter_variation (12.713s)
🤪 test_dashboard_deploys_dashboard_the_same_as_created_dashboard (13.657s)
🤪 test_dashboard_deploys_dashboard_with_table (11.965s)
🤪 test_dashboards_deploys_dashboard_with_markdown_header (20.283s)
🤪 test_dashboard_deploys_dashboard_with_big_widget (22.139s)
🤪 test_dashboards_deploys_dashboard_with_invalid_query (20.833s)

_{Running from acceptance #301}

nfx · 2024-07-10T10:07:40Z

src/databricks/labs/lsql/dashboards.py

-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(


rewrite this as a public method on DashboardMetadata: def replace_database(self, catalog_name: str, schema_name: str) -> DashboardMetadata:, which would create a copy of DashboardMetadata, but with all of the queries rewritten with the target catalog/db

src/databricks/labs/lsql/dashboards.py

JCZuurmond · 2024-07-11T07:17:35Z

src/databricks/labs/lsql/dashboards.py

@@ -216,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":


@nfx : Refactored this method also to an implementation that is stateless

yep, the right way

nfx · 2024-07-11T09:01:56Z

src/databricks/labs/lsql/cli.py

-        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 = DashboardMetadata.from_path(folder).replace_database(
+        catalog=catalog or None, database=database or None


Suggested change

catalog=catalog or None, database=database or None

catalog=catalog or None, database=database or None,

nit

nfx · 2024-07-11T09:02:21Z

src/databricks/labs/lsql/dashboards.py

@@ -216,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":


yep, the right way

nfx · 2024-07-11T09:12:12Z

src/databricks/labs/lsql/dashboards.py

-        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:


it looks like the entire create_dashboard method from here should be converted to a method on DashboardMetadata:

def as_lakeview(self) -> Dashboard: self.validate() datasets = self.get_datasets() # make get_datasets private layouts = self.get_layouts() # make get_layouts private page = Page( name=dashboard_metadata.display_name, display_name=dashboard_metadata.display_name, layout=layouts, ) return Dashboard(datasets=datasets, pages=[page])

…ines (#214) Implementing feedback: #210 (comment)

* Added method to dashboards to get dashboard url ([#211](#211)). In this release, we have added a new method `get_url` to the `lakeview_dashboards` object in the `laksedashboard` library. This method utilizes the Databricks SDK to retrieve the dashboard URL, simplifying the code and making it more maintainable. Previously, the dashboard URL was constructed by concatenating the host and dashboard ID, but this new method ensures that the URL is obtained correctly, even if the format changes in the future. Additionally, a new unit test has been added for a method that gets the dashboard URL using the workspace client. This new functionality allows users to easily retrieve the URL for a dashboard using its ID and the workspace client. * Extend replace database in query ([#210](#210)). This commit extends the database replacement functionality in the `DashboardMetadata` class, allowing users to specify which database and catalog to replace. The enhancement includes support for catalog replacement and a new `replace_database` method in the `DashboardMetadata` class, which replaces the catalog and/or database in the query based on provided parameters. These changes enhance the flexibility and customization of the database replacement feature in queries, making it easier for users to control how their data is displayed in the dashboard. The `create_dashboard` function has also been updated to use the new method for replacing the database and catalog. Additionally, the `TileMetadata` update method has been replaced with a new merge method, and the `QueryTile` and `Tile` classes have new properties and methods for handling content, width, height, and position. The commit also includes several unit tests to ensure the new functionality works as expected. * Improve object oriented dashboard-as-code implementation ([#208](#208)). In this release, the object-oriented implementation of the dashboard-as-code feature has been significantly improved, addressing previous pull request comments ([#201](#201)). The `TileMetadata` dataclass now includes methods for updating and comparing tile metadata, and the `DashboardMetadata` class has been removed and its functionality incorporated into the `Dashboards` class. The `Dashboards` class now generates tiles, datasets, and layouts for dashboards using the provided `query_transformer`. The code's readability and maintainability have been further enhanced by replacing the use of the `copy` module with `dataclasses.replace` for creating object copies. Additionally, updates have been made to the unit tests for dashboard functionality in the project, with new methods and attributes added to check for valid dashboard metadata and handle duplicate query or widget IDs, as well as to specify the order in which tiles and widgets should be displayed in the dashboard.

JCZuurmond requested a review from nfx July 10, 2024 09:13

JCZuurmond self-assigned this Jul 10, 2024

JCZuurmond temporarily deployed to runtime July 10, 2024 09:13 — with GitHub Actions Inactive

JCZuurmond mentioned this pull request Jul 10, 2024

Use lsql Lakeview dashboard-as-code implementation to create dashboards databrickslabs/ucx#1920

Merged

5 tasks

nfx requested changes Jul 10, 2024

View reviewed changes

JCZuurmond had a problem deploying to runtime July 10, 2024 13:51 — with GitHub Actions Failure

nfx requested changes Jul 10, 2024

View reviewed changes

src/databricks/labs/lsql/dashboards.py Outdated Show resolved Hide resolved

src/databricks/labs/lsql/dashboards.py Outdated Show resolved Hide resolved

src/databricks/labs/lsql/dashboards.py Outdated Show resolved Hide resolved

JCZuurmond had a problem deploying to runtime July 10, 2024 14:23 — with GitHub Actions Failure

JCZuurmond requested a review from nfx July 10, 2024 14:23

JCZuurmond changed the title ~~Optionally specify which database to replace in query~~ Extend replace database in query Jul 10, 2024

JCZuurmond had a problem deploying to runtime July 10, 2024 14:26 — with GitHub Actions Failure

JCZuurmond force-pushed the feat/replace-database-in-query-where-existing-database branch 2 times, most recently from 0b51df7 to 7a7366f Compare July 10, 2024 14:29

JCZuurmond had a problem deploying to runtime July 10, 2024 14:29 — with GitHub Actions Failure

JCZuurmond force-pushed the feat/replace-database-in-query-where-existing-database branch 2 times, most recently from bbc3cb6 to cba91fe Compare July 10, 2024 14:30

JCZuurmond had a problem deploying to runtime July 10, 2024 14:32 — with GitHub Actions Failure

JCZuurmond had a problem deploying to runtime July 10, 2024 14:35 — with GitHub Actions Failure

JCZuurmond had a problem deploying to runtime July 10, 2024 14:40 — with GitHub Actions Failure

JCZuurmond force-pushed the feat/replace-database-in-query-where-existing-database branch from c2b2083 to 1ff8635 Compare July 10, 2024 14:42

JCZuurmond had a problem deploying to runtime July 10, 2024 14:44 — with GitHub Actions Failure

JCZuurmond added 8 commits July 10, 2024 17:11

Specify which database to replace in query

06ee2e9

Merge tests

b4f938b

Refactor to replace database on dashboard metadata object

727a614

Let replace database return new instance of DashboardMetadata

5d2d2f6

Support replacing catalog in dataset

42615ed

Add replace catalog functionality to create-dashboard command

30f674d

Move attribute docs

c566ea1

Update integration test with new API

c2800e1

JCZuurmond added 5 commits July 10, 2024 17:11

Add content to Tile

46de0bb

Refactor DashboardMetadata to have tiles instead of tile metadata

a746993

Make Tile content hidden

3023afa

Remove update from DashboardMetadata

b97a63c

Move replace_database to QueryTile

cc4ce27

JCZuurmond force-pushed the feat/replace-database-in-query-where-existing-database branch from 1ff8635 to cc4ce27 Compare July 10, 2024 16:17

JCZuurmond temporarily deployed to runtime July 10, 2024 16:17 — with GitHub Actions Inactive

JCZuurmond added 5 commits July 11, 2024 08:35

Update command description

3c4ae39

Update docstring

2cb6de7

Loop over tiles

26e24b9

Refactor TileMetadata.update to merge

9d8d374

Add comment

2563c96

JCZuurmond temporarily deployed to runtime July 11, 2024 07:12 — with GitHub Actions Inactive

JCZuurmond commented Jul 11, 2024

View reviewed changes

nfx requested changes Jul 11, 2024

View reviewed changes

nfx merged commit 46875e7 into main Jul 11, 2024
8 checks passed

nfx deleted the feat/replace-database-in-query-where-existing-database branch July 11, 2024 09:14

This was referenced Jul 11, 2024

Expand arguments when calling replace_database in cli to multiple lines #214

Merged

[TECH DEBT] Move create_dashboards to DashboardMetadata.as_lakeview #215

Closed

nfx pushed a commit that referenced this pull request Jul 11, 2024

Expand arguments when calling replace_database in cli to multiple l…

a2323f8

…ines (#214) Implementing feedback: #210 (comment)

nfx mentioned this pull request Jul 11, 2024

Release v0.6.0 #217

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend replace database in query #210

Extend replace database in query #210

JCZuurmond commented Jul 10, 2024 •

edited

Loading

github-actions bot commented Jul 10, 2024 •

edited

Loading

nfx Jul 10, 2024

JCZuurmond Jul 11, 2024

nfx Jul 11, 2024

nfx Jul 11, 2024

JCZuurmond Jul 11, 2024

nfx Jul 11, 2024

nfx Jul 11, 2024

	catalog=catalog or None, database=database or None
	catalog=catalog or None, database=database or None,

Extend replace database in query #210

Extend replace database in query #210

Conversation

JCZuurmond commented Jul 10, 2024 • edited Loading

github-actions bot commented Jul 10, 2024 • edited Loading

nfx Jul 10, 2024

Choose a reason for hiding this comment

JCZuurmond Jul 11, 2024

Choose a reason for hiding this comment

nfx Jul 11, 2024

Choose a reason for hiding this comment

nfx Jul 11, 2024

Choose a reason for hiding this comment

JCZuurmond Jul 11, 2024

Choose a reason for hiding this comment

nfx Jul 11, 2024

Choose a reason for hiding this comment

nfx Jul 11, 2024

Choose a reason for hiding this comment

JCZuurmond commented Jul 10, 2024 •

edited

Loading

github-actions bot commented Jul 10, 2024 •

edited

Loading