-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Put back dashboard names #3808
Merged
JCZuurmond
merged 2 commits into
main
from
revert-3795-revert/dashboard-names-with-special-characters
Mar 3, 2025
Merged
Put back dashboard names #3808
JCZuurmond
merged 2 commits into
main
from
revert-3795-revert/dashboard-names-with-special-characters
Mar 3, 2025
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
✅ 25/25 passed, 23m3s total Running from acceptance #8450 |
44f68b8
to
270cdf0
Compare
pritishpai
approved these changes
Mar 3, 2025
gueniai
added a commit
that referenced
this pull request
Mar 5, 2025
* Convert UCX job ids to `int` before passing to `JobsCrawler` ([#3816](#3816)). In this release, we have addressed issue [#3722](#3722) and improved the robustness of the open-source library by modifying the `jobs_crawler` method to handle job IDs more effectively. Previously, job IDs were passed directly to the `exclude_job_ids` parameter, which could cause issues if they were not integers. To address this problem, we have updated the `jobs_crawler` method to convert all job IDs to integers using a list comprehension before passing them to the method. This change ensures that only valid integer job IDs are used, thereby enhancing the reliability of the method. The commit includes a manual test to confirm the correct behavior of this modification. In summary, this modification improves the robustness of the code by ensuring that integer job IDs are utilized correctly in the `JobsCrawler` method. * Exclude UCX jobs from crawling ([#3733](#3733)). In this release, we have made modifications to the `JobsCrawler` and the existing `assessment` workflow to exclude UCX jobs from crawling, avoiding confusion for users when they appear in assessment reports. This change addresses issues [#3656](#3656) and [#3722](#3722), and is a follow-up to previous issue [#3732](#3732). We have also incorporated updates from pull requests [#3767](#3767) and [#3759](#3759) to improve integration tests and linting. Additionally, a retry mechanism has been added to wait for grants to exist before crawling, addressing issue [#3758](#3758). The changes include the addition of unit and integration tests to ensure the correctness of the modifications. A new `exclude_job_ids` parameter has been added to the `JobsCrawler` constructor, which is initialized with the list of UCX job IDs, ensuring that UCX jobs are not included in the assessment report. The `_list_jobs` method now excludes jobs based on the provided `exclude_job_ids` and `include_job_ids` arguments. The `_crawl` method now uses the `_list_jobs` method to list the jobs to be crawled. The `_assess_jobs` method has been updated to take into account the exclusion of specific job IDs. The `test_grant_detail` file, an integration test for the Hive Metastore grants functionality, has been updated to include a retry mechanism to wait for grants to exist before crawling and to check if the SELECT permission on ANY FILE is present in the grants. * Let `WorkflowLinter.refresh_report` lint jobs from `JobsCrawler` ([#3732](#3732)). In this release, the `WorkflowLinter.refresh_report` method has been updated to lint jobs from the `JobsCrawler` class, ensuring that only jobs within the scope of the crawler are processed. This change resolves issue [#3662](#3662) and progresses issue [#3722](#3722). The workflow linting code, the `assessment` workflow, and the `JobsCrawler` class have been modified. The `JobsCrawler` class now includes a `snapshot` method, which is used in the `WorkflowLinter.refresh_report` method to retrieve necessary data about jobs. Unit and integration tests have been updated correspondingly, with the integration test for workflows now verifying that all rows returned from a query to the `workflow_problems` table have a valid `path` field. The `WorkflowLinter` constructor now includes an instance of `JobsCrawler`, allowing for more targeted linting of jobs. The introduction of the `JobsCrawler` class enables more efficient and precise linting of jobs, improving the overall accuracy of workflow assessment. * Let dashboard name adhere to naming convention ([#3789](#3789)). In this release, the naming convention for dashboard names in the `ucx` library has been enforced, restricting them to alphanumeric characters, hyphens, and underscores. This change replaces any non-conforming characters in existing dashboard names with hyphens or underscores, addressing several issues ([#3761](#3761) through [#3788](#3788)). A temporary fix has been added to the `_create_dashboard` method to ensure newly created dashboard names adhere to the new naming convention, indicated by a TODO comment. This release also resolves a test failure in a specific GitHub Actions run and addresses a total of 29 issues. The specifics of the modification made to the `databricks labs install ucx` command and the changes to existing functionality are not detailed, making it difficult to assess their scope. The commit includes the deletion of a file called `02_0_owner.filter.yml`, and all changes have been manually tested. For future reference, it would be helpful to include more information about the changes made, their impact, and the reason for deleting the specified file. * Partial revert `Let dashboard name adhere to naming convention` ([#3794](#3794)). In this release, we have partially reverted a previous change to the migration progress dashboard, reintroducing the owner filter. This change was made in response to feedback from users who found the previous modification to the dashboard less intuitive. The new owner filter has been defined in a new file, '02_0_owner.filter.yml', which includes the title, column name, type, and width of the filter. To ensure proper functionality, this change requires the release of lsql after merging. The change has been thoroughly tested to guarantee its correct operation and to provide the best possible user experience. * Partial revert `Let dashboard name adhere to naming convention` ([#3795](#3795)). In this release, we have partially reversed a previous change that enforced a naming convention for dashboard names, allowing the use of special characters such as spaces and brackets again. The `_create_dashboard` method in the `install.py` file and the `_name` method in the `mixins.py` file have been updated to reflect this change, affecting the migration progress dashboard. The `display_name` attribute of the `metadata` object has been updated to use the original format, which may include special characters. The `reference` variable has also been updated accordingly. The functions `created_job_tasks` and `created_job` have been updated to use the new naming convention when retrieving installation jobs with specific names. These changes have been manually tested and the tests have been verified to work correctly after the reversion. This change is related to issues [#3799](#3799), [#3789](#3789), and reverts commit 048bc8f. * Put back dashboard names ([#3808](#3808)). In the lsql release v0.16.0, the naming convention for dashboards has been updated to support non-alphanumeric characters in the dashboard names. This change modifies the `_create_dashboard` function in `install.py` and the `_name` method in `mixins.py` to create dashboard names with a format like `[UCX] assessment (Main)`, which includes parent and child folder names. This update addresses issues reported in tickets [#3797](#3797) and [#3790](#3790), and partially reverses previous changes made in commits 4017a25 and 834ef14. The functionality of other methods remains unchanged. With this release, the `created_job_tasks` and `created_job` functions now accept dashboard names with non-alphanumeric characters as input. * Updated databricks-labs-lsql requirement from <0.15,>=0.14.0 to >=0.14.0,<0.17 ([#3801](#3801)). In this update, we have updated the required version of the `dat ab ricks-l abs-ls ql` package from a version greater than or equal to 0.15.0 and less than 0.16.0 to a version greater than or equal to 0.16.0 and less than 0.17.0. This change allows for the use of the latest version of the package, which includes various bug fixes and dependency updates. The package is utilized in the acceptance tests that are run as part of the CI/CD pipeline. With this update, the acceptance tests can now be executed using the most recent version of the package, resulting in enhanced functionality and reliability. * Updated databricks-sdk requirement from <0.42,>=0.40 to >=0.44,<0.45 ([#3686](#3686)). In this release, we have updated the version requirement for the `databricks-sdk` package to be greater than or equal to 0.44.0 and less than 0.45.0. This update allows for the use of the latest version of the `databricks-sdk`, which includes new methods, fields, and bug fixes. For instance, the `get_message_query_result_by_attachment` method has been added for the `w.genie.workspace_level_service`, and several fields such as `review_state`, `reviews`, and `runner_collaborators` have been removed for the `databricks.sdk.service.clean_rooms.CleanRoomAssetNotebook` object. Additionally, the `securable_kind` field has been removed for various objects such as `CatalogInfo` and `ConnectionInfo`. We recommend thoroughly testing this update to ensure compatibility with your project. The release notes for versions 0.44.0 and 0.43.0 can be found in the commit history. Please note that there are several backward-incompatible changes listed in the changelog for both versions. Dependency updates: * Updated databricks-labs-lsql requirement from <0.15,>=0.14.0 to >=0.14.0,<0.17 ([#3801](#3801)). * Updated databricks-sdk requirement from <0.42,>=0.40 to >=0.44,<0.45 ([#3686](#3686)).
Merged
gueniai
added a commit
that referenced
this pull request
Mar 5, 2025
* Convert UCX job ids to `int` before passing to `JobsCrawler` ([#3816](#3816)). In this release, we have addressed issue [#3722](#3722) and improved the robustness of the open-source library by modifying the `jobs_crawler` method to handle job IDs more effectively. Previously, job IDs were passed directly to the `exclude_job_ids` parameter, which could cause issues if they were not integers. To address this problem, we have updated the `jobs_crawler` method to convert all job IDs to integers using a list comprehension before passing them to the method. This change ensures that only valid integer job IDs are used, thereby enhancing the reliability of the method. The commit includes a manual test to confirm the correct behavior of this modification. In summary, this modification improves the robustness of the code by ensuring that integer job IDs are utilized correctly in the `JobsCrawler` method. * Exclude UCX jobs from crawling ([#3733](#3733)). In this release, we have made modifications to the `JobsCrawler` and the existing `assessment` workflow to exclude UCX jobs from crawling, avoiding confusion for users when they appear in assessment reports. This change addresses issues [#3656](#3656) and [#3722](#3722), and is a follow-up to previous issue [#3732](#3732). We have also incorporated updates from pull requests [#3767](#3767) and [#3759](#3759) to improve integration tests and linting. Additionally, a retry mechanism has been added to wait for grants to exist before crawling, addressing issue [#3758](#3758). The changes include the addition of unit and integration tests to ensure the correctness of the modifications. A new `exclude_job_ids` parameter has been added to the `JobsCrawler` constructor, which is initialized with the list of UCX job IDs, ensuring that UCX jobs are not included in the assessment report. The `_list_jobs` method now excludes jobs based on the provided `exclude_job_ids` and `include_job_ids` arguments. The `_crawl` method now uses the `_list_jobs` method to list the jobs to be crawled. The `_assess_jobs` method has been updated to take into account the exclusion of specific job IDs. The `test_grant_detail` file, an integration test for the Hive Metastore grants functionality, has been updated to include a retry mechanism to wait for grants to exist before crawling and to check if the SELECT permission on ANY FILE is present in the grants. * Let `WorkflowLinter.refresh_report` lint jobs from `JobsCrawler` ([#3732](#3732)). In this release, the `WorkflowLinter.refresh_report` method has been updated to lint jobs from the `JobsCrawler` class, ensuring that only jobs within the scope of the crawler are processed. This change resolves issue [#3662](#3662) and progresses issue [#3722](#3722). The workflow linting code, the `assessment` workflow, and the `JobsCrawler` class have been modified. The `JobsCrawler` class now includes a `snapshot` method, which is used in the `WorkflowLinter.refresh_report` method to retrieve necessary data about jobs. Unit and integration tests have been updated correspondingly, with the integration test for workflows now verifying that all rows returned from a query to the `workflow_problems` table have a valid `path` field. The `WorkflowLinter` constructor now includes an instance of `JobsCrawler`, allowing for more targeted linting of jobs. The introduction of the `JobsCrawler` class enables more efficient and precise linting of jobs, improving the overall accuracy of workflow assessment. * Let dashboard name adhere to naming convention ([#3789](#3789)). In this release, the naming convention for dashboard names in the `ucx` library has been enforced, restricting them to alphanumeric characters, hyphens, and underscores. This change replaces any non-conforming characters in existing dashboard names with hyphens or underscores, addressing several issues ([#3761](#3761) through [#3788](#3788)). A temporary fix has been added to the `_create_dashboard` method to ensure newly created dashboard names adhere to the new naming convention, indicated by a TODO comment. This release also resolves a test failure in a specific GitHub Actions run and addresses a total of 29 issues. The specifics of the modification made to the `databricks labs install ucx` command and the changes to existing functionality are not detailed, making it difficult to assess their scope. The commit includes the deletion of a file called `02_0_owner.filter.yml`, and all changes have been manually tested. For future reference, it would be helpful to include more information about the changes made, their impact, and the reason for deleting the specified file. * Partial revert `Let dashboard name adhere to naming convention` ([#3794](#3794)). In this release, we have partially reverted a previous change to the migration progress dashboard, reintroducing the owner filter. This change was made in response to feedback from users who found the previous modification to the dashboard less intuitive. The new owner filter has been defined in a new file, '02_0_owner.filter.yml', which includes the title, column name, type, and width of the filter. To ensure proper functionality, this change requires the release of lsql after merging. The change has been thoroughly tested to guarantee its correct operation and to provide the best possible user experience. * Partial revert `Let dashboard name adhere to naming convention` ([#3795](#3795)). In this release, we have partially reversed a previous change that enforced a naming convention for dashboard names, allowing the use of special characters such as spaces and brackets again. The `_create_dashboard` method in the `install.py` file and the `_name` method in the `mixins.py` file have been updated to reflect this change, affecting the migration progress dashboard. The `display_name` attribute of the `metadata` object has been updated to use the original format, which may include special characters. The `reference` variable has also been updated accordingly. The functions `created_job_tasks` and `created_job` have been updated to use the new naming convention when retrieving installation jobs with specific names. These changes have been manually tested and the tests have been verified to work correctly after the reversion. This change is related to issues [#3799](#3799), [#3789](#3789), and reverts commit 048bc8f. * Put back dashboard names ([#3808](#3808)). In the lsql release v0.16.0, the naming convention for dashboards has been updated to support non-alphanumeric characters in the dashboard names. This change modifies the `_create_dashboard` function in `install.py` and the `_name` method in `mixins.py` to create dashboard names with a format like `[UCX] assessment (Main)`, which includes parent and child folder names. This update addresses issues reported in tickets [#3797](#3797) and [#3790](#3790), and partially reverses previous changes made in commits 4017a25 and 834ef14. The functionality of other methods remains unchanged. With this release, the `created_job_tasks` and `created_job` functions now accept dashboard names with non-alphanumeric characters as input. * Updated databricks-labs-lsql requirement from <0.15,>=0.14.0 to >=0.14.0,<0.17 ([#3801](#3801)). In this update, we have updated the required version of the `dat ab ricks-l abs-ls ql` package from a version greater than or equal to 0.15.0 and less than 0.16.0 to a version greater than or equal to 0.16.0 and less than 0.17.0. This change allows for the use of the latest version of the package, which includes various bug fixes and dependency updates. The package is utilized in the acceptance tests that are run as part of the CI/CD pipeline. With this update, the acceptance tests can now be executed using the most recent version of the package, resulting in enhanced functionality and reliability. * Updated databricks-sdk requirement from <0.42,>=0.40 to >=0.44,<0.45 ([#3686](#3686)). In this release, we have updated the version requirement for the `databricks-sdk` package to be greater than or equal to 0.44.0 and less than 0.45.0. This update allows for the use of the latest version of the `databricks-sdk`, which includes new methods, fields, and bug fixes. For instance, the `get_message_query_result_by_attachment` method has been added for the `w.genie.workspace_level_service`, and several fields such as `review_state`, `reviews`, and `runner_collaborators` have been removed for the `databricks.sdk.service.clean_rooms.CleanRoomAssetNotebook` object. Additionally, the `securable_kind` field has been removed for various objects such as `CatalogInfo` and `ConnectionInfo`. We recommend thoroughly testing this update to ensure compatibility with your project. The release notes for versions 0.44.0 and 0.43.0 can be found in the commit history. Please note that there are several backward-incompatible changes listed in the changelog for both versions. Dependency updates: * Updated databricks-labs-lsql requirement from <0.15,>=0.14.0 to >=0.14.0,<0.17 ([#3801](#3801)). * Updated databricks-sdk requirement from <0.42,>=0.40 to >=0.44,<0.45 ([#3686](#3686)).
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.
Description
With the latest lsql release, the dashboard names can have non-alphanumeric characters. This PR creates dashboards with names like:
[UCX] assessment (Main)
.Resolves #3797
Resolves #3790
Requires #3801
Partially reverts #3795: 4017a25 and 834ef14