Hot Fix for Dashboard Names #1476
Open
Conversation
This file contains 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
|
✅ 13/13 passed, 1 skipped, 15s total Running from acceptance #341 |
sundarshankar89
requested review from
sriram251-code,
bishwajit-db,
ericvergnaud and
Copilot
and removed request for
a team
There was a problem hiding this comment.
PR Overview
This PR delivers a hot fix for Dashboard names and refines installation logic for the transpiler. Key changes include:
- Updating dashboard name formatting from bracketed names to underscore‐separated names.
- Revising test functions for installation and dashboard deployment, including new patch decorators.
- Adjusting dependency version constraints and restructuring installation method calls.
Reviewed Changes
| File | Description |
|---|---|
| tests/unit/test_install.py | Added a new test function for installation override with patch decorators for install_rct and install_morpheus. |
| src/databricks/labs/remorph/deployment/job.py | Changed dashboard name formatting to use underscore-separated names. |
| pyproject.toml | Updated dependency versions, notably for databricks-sdk and databricks-labs-lsql. |
| src/databricks/labs/remorph/deployment/dashboard.py | Updated dashboard name formatting similarly to job.py for consistency. |
| tests/unit/deployment/test_dashboard.py | Modified dashboard query extraction and test signature to match new dashboard data structure. |
| src/databricks/labs/remorph/install.py | Reorganized calls to install_rct and install_morpheus, moving them into the “transpile” configuration path. |
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
src/databricks/labs/remorph/deployment/job.py:147
- [nitpick] The new dashboard name formatting uses an inline replace to convert spaces to underscores. Consider extracting a dedicated sanitization function to improve code clarity and consistency across the codebase.
return f"{prefix.upper()}_{name}".replace(" ", "_")
src/databricks/labs/remorph/deployment/dashboard.py:120
- [nitpick] The updated dashboard name formatting mirrors changes in job.py. A refactored helper function for name sanitization might reduce repetition and improve maintainability.
return f"{prefix.upper()}_{name}".replace(" ", "_")
tests/unit/deployment/test_dashboard.py:17
- The function assumes that dashboard is not None. Consider adding an explicit check for a None value before accessing its 'serialized_dashboard' attribute.
def _get_dashboard_query(dashboard: Dashboard | None):
src/databricks/labs/remorph/install.py:283
- The removal of install_rct and install_morpheus calls from the run method and their addition into the 'transpile' configuration case changes the installation order. Ensure that this restructuring does not cause duplicate installations or affect modules other than 'transpile'.
self.install_rct() and self.install_morpheus()
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This fixes the Installation issue for Dashboard Deployment
and also #1474