Let WorkflowLinter.refresh_report lint jobs from JobsCrawler
#3732
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
JCZuurmond
added
step/assessment
JCZuurmond
had a problem deploying
to
account-admin
GitHub Actions
Failure
|
✅ 85/85 passed, 1 flaky, 7 skipped, 51m55s total Flaky tests:
Running from acceptance #8374 |
5 tasks
FastLee
requested changes
Feb 21, 2025
|
|
||
| def refresh_report(self, sql_backend: SqlBackend, inventory_database: str) -> None: | ||
| tasks = [] | ||
| items_listed = 0 | ||
| for job in self._ws.jobs.list(): | ||
| if self._include_job_ids is not None and job.job_id not in self._include_job_ids: |
There was a problem hiding this comment.
Where do we replicate the filtering capability with the Job Crawler?
There was a problem hiding this comment.
JCZuurmond
temporarily deployed
to
account-admin
GitHub Actions
Inactive
gueniai
pushed a commit
that referenced
this pull request
Feb 27, 2025
## Changes Exclude UCX jobs from crawling to avoid confusing for users when they see UCX jobs in their assessment report. ### Linked issues Fixes #3656 Resolves #3722 Follow up on #3732 Relates to #3731 ### Functionality - [x] modified `JobsCrawler` - [x] modified existing workflow: `assessment` ### Tests - [x] added unit tests - [x] added integration tests ### PRs merged into this branch > Merged the following PRs into this branch in an attempt to let the CI pass. Those PRs contain fixes for integration tests From #3767: Scope linted dashboards on mock runtime context. We should use `make_dashboard` instead of the dashboard fixture directly `_make_dashboard`. Also changed one dashboard to a `LakeviewDashboard` so that we lint that too From #3759 Add retry mechanism to wait for the grants to exists before crawling Resolves #3758 - [x] modified integration tests: `test_all_grant_types`
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.
Changes
Let
WorkflowLinter.refresh_reportlint jobs fromJobsCrawlerso that we only lint what is within scopeLinked issues
Resolves #3662
Progresses #3722
Functionality
assessmentTests