Added Functionality to export any dashboards-as-code into CSV #269

jgarciaf106 · 2024-09-10T01:45:29Z

Functionality

added functionality to export to CSV using DashboardMetadata.

Tests

manually tested
added unit tests

nfx · 2024-09-10T10:34:54Z

src/databricks/labs/lsql/dashboards.py

+class ExportDashboard(DashboardMetadata):
+    def __init__(self, sql_backend, config):
+        self._sql_backend = sql_backend
+        self._config = config
+        super().__init__(display_name="")
+
+    def export_dashboard(
+        self,
+        queries_path: Path,
+        export_path: Path,
+        catalog: str = "hive_metastore",
+        database: None = None,
+    ) -> None:


instead of adding the class, create def export_to_zipped_csv(self, sql_backend: SqlBackend) -> Path on DashboardMetadata class. catalog/database replacement is already handled in that class.

I'll refactor to follow your feedback thanks.

Changes Applied

Added a helper function to zip the CSV to avoid increasing the scope of the function.

src/databricks/labs/lsql/dashboards.py

nfx · 2024-09-10T10:37:23Z

src/databricks/labs/lsql/dashboards.py

+            file_name = f"{export_path}/{tile.metadata.id}.csv"
+            with open(file_name, mode="w", newline="", encoding="utf-8") as file:


Suggested change

file_name = f"{export_path}/{tile.metadata.id}.csv"

with open(file_name, mode="w", newline="", encoding="utf-8") as file:

tile_export = export_path / f"{tile.metadata.id}.csv"

with tile_export.open(mode="w", newline="", encoding="utf-8") as hadle:

use pathlib.Path

I'll refactor to follow your feedback thanks.

Changes applied.

nfx · 2024-09-10T15:13:17Z

src/databricks/labs/lsql/dashboards.py

+        for tile in dashboard.tiles:
+            if not tile.metadata.is_query():
+                continue
+            tile_export = export_path / f"{tile.metadata.id}.csv"


why do you need to create temporary CSV files if you can write them directly to the open zip?https://docs.python.org/3/library/zipfile.html#zipfile.ZipFile.open

with ZipFile(target_folder / 'ucx-export.zip', mode='w') as z: ... with z.open(f'{tile.id}.csv') as f: writer = csv.DictWriter(f, fieldnames=headers) writer.writeheader() for row in query_results: ...

this way you don't have to cleanup a file. See the feedback in https://github.com/databrickslabs/ucx/pull/2553/files

Refactored to Use ZipFile and CVS to avoid temp files or cleanup.

nfx · 2024-09-10T15:14:56Z

src/databricks/labs/lsql/dashboards.py

+    def export_to_zipped_csv(
+        self, sql_backend: SqlBackend, queries_path: Path, export_path: Path, catalog: str, database: str
+    ) -> Path:
+        """Export the dashboard queries to CSV files in a ZIP archive."""
+        dashboard = self.from_path(queries_path)
+        dashboard = dashboard.replace_database(catalog=catalog, database=database)
+        for tile in dashboard.tiles:


Suggested change

def export_to_zipped_csv(

self, sql_backend: SqlBackend, queries_path: Path, export_path: Path, catalog: str, database: str

) -> Path:

"""Export the dashboard queries to CSV files in a ZIP archive."""

dashboard = self.from_path(queries_path)

dashboard = dashboard.replace_database(catalog=catalog, database=database)

for tile in dashboard.tiles:

def export_to_zipped_csv(self, sql_backend: SqlBackend, export_path: Path) -> Path:

"""Export the dashboard queries to CSV files in a ZIP archive."""

for tile in self.tiles:

why do you need to call from_path if you already have a dashboard metadata instance?

Function is using the Dashboard Metadata instance to avoid additional parameters.

nfx · 2024-09-10T15:16:22Z

src/databricks/labs/lsql/dashboards.py

+        except FileNotFoundError:
+            logger.warning(f"File {file_path} not found.")
+        except PermissionError:
+            logger.warning(f"Permission denied for {file_path} or {zip_path}.")


Suggested change

except FileNotFoundError:

logger.warning(f"File {file_path} not found.")

except PermissionError:

logger.warning(f"Permission denied for {file_path} or {zip_path}.")

don't swallow errors here.

nfx

lgtm

* Added Functionality to export any dashboards-as-code into CSV ([#269](#269)). The `DashboardMetadata` class now includes a new method, `export_to_zipped_csv`, which enables exporting any dashboard as CSV files in a ZIP archive. This method accepts `sql_backend` and `export_path` as parameters and exports dashboard queries to CSV files in the specified ZIP archive by iterating through tiles and fetching dashboard queries if the tile is a query. To ensure the proper functioning of this feature, unit tests and manual testing have been conducted. A new test, `test_dashboards_export_to_zipped_csv`, has been added to verify the correct export of dashboard data to a CSV file. * Added support for generic types in `SqlBackend` ([#272](#272)). In this release, we've added support for using rich dataclasses, including those with optional and generic types, in the `SqlBackend` of the `StatementExecutionBackend` class. The new functionality is demonstrated in the `test_supports_complex_types` unit test, which creates a `Nested` dataclass containing various complex data types, such as nested dataclasses, `datetime` objects, `dict`, `list`, and optional fields. This enhancement is achieved by updating the `save_table` method to handle the conversion of complex dataclasses to SQL statements. To facilitate type inference, we've introduced a new `StructInference` class that converts Python dataclasses and built-in types to their corresponding SQL Data Definition Language (DDL) representations. This addition simplifies data definition and manipulation operations while maintaining type safety and compatibility with various SQL data types.

JCZuurmond

@jgarciaf106 : Could you open another PR to address my comments

JCZuurmond · 2024-09-12T08:25:20Z

src/databricks/labs/lsql/dashboards.py

@@ -893,6 +897,35 @@ def _from_dashboard_folder(cls, folder: Path) -> "DashboardMetadata":
            tiles.append(tile)
        return cls(display_name=folder.name, _tiles=tiles)

+    def export_to_zipped_csv(self, sql_backend: SqlBackend, export_path: Path) -> Path:
+        """Export the dashboard queries to CSV files directly into a ZIP archive."""
+        zip_export = export_path / "export_to_zipped_csv.zip"


@jgarciaf106 : Please except the function argument for the zip_export instead of adding the hardcoded "export_to_zipped_csv.zip"

JCZuurmond · 2024-09-12T08:26:30Z

src/databricks/labs/lsql/dashboards.py

+
+        with ZipFile(zip_export, mode="w") as zip_file:
+            for tile in self.tiles:
+                if tile.metadata.is_query():


We more commonly use a pattern with continue

if not tile.metadata.is_query(): continue ...

JCZuurmond · 2024-09-12T08:26:52Z

src/databricks/labs/lsql/dashboards.py

+                if tile.metadata.is_query():
+                    rows = sql_backend.fetch(tile.content)
+
+                    if not rows:


More precies

if len(rows) == 0

JCZuurmond · 2024-09-12T08:27:46Z

src/databricks/labs/lsql/dashboards.py

+
+                    bytes_buffer = BytesIO(buffer.getvalue().encode("utf-8"))
+
+                    with zip_file.open(f"{tile.metadata.id}.csv", "w") as csv_file:


Writing bytes, right?

with zip_file.open(f"{tile.metadata.id}.csv", "wb") as csv_file:

JCZuurmond · 2024-09-12T08:33:33Z

src/databricks/labs/lsql/dashboards.py

+                            writer.writeheader()
+                        writer.writerow(row.asDict())
+
+                    bytes_buffer = BytesIO(buffer.getvalue().encode("utf-8"))


Is redundant, see comment below

JCZuurmond · 2024-09-12T08:34:14Z

src/databricks/labs/lsql/dashboards.py

+                    bytes_buffer = BytesIO(buffer.getvalue().encode("utf-8"))
+
+                    with zip_file.open(f"{tile.metadata.id}.csv", "w") as csv_file:
+                        csv_file.write(bytes_buffer.getvalue())


Bytest buffer is redudant

csv_file.write(buffer.getvalue().encode("utf-8"))

Added Functionality to export any dashboards-as-code into CSV

c731186

nfx requested changes Sep 10, 2024

View reviewed changes

Code changes applied

ad42dd8

nfx requested changes Sep 10, 2024

View reviewed changes

Refactor export_to_zipped_csv

dbc1a65

jgarciaf106 had a problem deploying to runtime September 11, 2024 09:37 — with GitHub Actions Failure

nfx approved these changes Sep 11, 2024

View reviewed changes

nfx merged commit 5dfaaa0 into databrickslabs:main Sep 11, 2024
7 of 8 checks passed

nfx mentioned this pull request Sep 11, 2024

Release v0.10.0 #274

Merged

JCZuurmond reviewed Sep 12, 2024

View reviewed changes

JCZuurmond mentioned this pull request Sep 12, 2024

Added export CLI functionality for assessment results databrickslabs/ucx#2553

Merged

2 tasks

jgarciaf106 deleted the feat/lakeview-export branch October 4, 2024 19:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Functionality to export any dashboards-as-code into CSV #269

Added Functionality to export any dashboards-as-code into CSV #269

jgarciaf106 commented Sep 10, 2024

nfx Sep 10, 2024

jgarciaf106 Sep 10, 2024

jgarciaf106 Sep 10, 2024

jgarciaf106 Sep 10, 2024

nfx Sep 10, 2024

jgarciaf106 Sep 10, 2024

jgarciaf106 Sep 10, 2024

nfx Sep 10, 2024

jgarciaf106 Sep 10, 2024

nfx Sep 10, 2024

jgarciaf106 Sep 10, 2024

nfx Sep 10, 2024

nfx left a comment

JCZuurmond left a comment

JCZuurmond Sep 12, 2024

JCZuurmond Sep 12, 2024

JCZuurmond Sep 12, 2024

JCZuurmond Sep 12, 2024

JCZuurmond Sep 12, 2024

JCZuurmond Sep 12, 2024

		file_name = f"{export_path}/{tile.metadata.id}.csv"
		with open(file_name, mode="w", newline="", encoding="utf-8") as file:


		bytes_buffer = BytesIO(buffer.getvalue().encode("utf-8"))

		with zip_file.open(f"{tile.metadata.id}.csv", "w") as csv_file:

Added Functionality to export any dashboards-as-code into CSV #269

Added Functionality to export any dashboards-as-code into CSV #269

Conversation

jgarciaf106 commented Sep 10, 2024

Functionality

Tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nfx left a comment

Choose a reason for hiding this comment

JCZuurmond left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment