Improve object oriented dashboard-as-code implementation#208
Merged
Conversation
23 tasks
|
✅ 35/35 passed, 3 flaky, 2 skipped, 11m54s total Flaky tests:
Running from acceptance #281 |
nfx
requested changes
Jul 9, 2024
Collaborator
nfx
left a comment
nfx
left a comment
There was a problem hiding this comment.
some severe bugs, otherwise looks way cleaner
| width = self._metadata.width or default_width | ||
| height = self._metadata.height or default_height | ||
| self.position = Position(0, 0, width, height) | ||
| _position: Position = Position(0, 0, 0, 0) |
Collaborator
There was a problem hiding this comment.
Suggested change
| _position: Position = Position(0, 0, 0, 0) | |
| position: Position = Position(0, 0, 0, 0) |
otherwise looks a bit ugly... rename def position to def position_or_default
| if id_count > 1: | ||
| raise ValueError(f"Duplicate id: {tile_id}") | ||
|
|
||
| def update(self, other: "DashboardMetadata") -> None: |
Collaborator
There was a problem hiding this comment.
Suggested change
| def update(self, other: "DashboardMetadata") -> None: | |
| def merge(self, other: "DashboardMetadata") -> None: |
did you mean merge?
Contributor
Author
There was a problem hiding this comment.
Named this after the dict.update, see resources in docstring
Merged
nfx
added a commit
that referenced
this pull request
Jul 11, 2024
* 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.
Merged
nfx
added a commit
that referenced
this pull request
Jul 11, 2024
* 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Covers the outstanding comments in #201