Fixing run_workflow functionality for better error handling. #1159
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1159 +/- ##
==========================================
+ Coverage 89.77% 89.80% +0.03%
==========================================
Files 61 61
Lines 7246 7249 +3
Branches 1299 1300 +1
==========================================
+ Hits 6505 6510 +5
+ Misses 477 475 -2
Partials 264 264 ☔ View full report in Codecov by Sentry. |
FastLee
changed the title
|
❌ 131/133 passed, 3 flaky, 2 failed, 20 skipped, 1h49m25s total ❌ test_running_real_assessment_job: TimeoutError: timed out after 0:20:00: (21m36.544s)❌ test_partitioned_tables: TimeoutError: Timed out after 0:05:00 (21m24.247s)Flaky tests:
Running from acceptance #1861 |
FastLee
force-pushed
the
fix/fix_int_test_1143
branch
from
7b8cdea
to
e50fd82
Compare
dmoore247
suggested changes
Mar 29, 2024
| job_run = self._ws.jobs.get_run(job_initial_run.run_id) | ||
| raise self._infer_error_from_job_run(job_run) from err | ||
| return | ||
| raise NotFound(f"job run not found for {step}") |
There was a problem hiding this comment.
Please include {job_id} in the NotFound message as part of the context, needed to aid troubleshooting.
nfx
added a commit
that referenced
this pull request
Mar 29, 2024
* Ensure proper sequencing of view migrations ([#1157](#1157)). In this release, we have introduced a `views_migrator` module and corresponding test cases to ensure proper sequencing of view migrations, addressing issue [#1132](#1132). The module contains two main classes: `ViewToMigrate` and `ViewsMigrator`. The former is responsible for parsing a view's SQL text and identifying its dependencies, while the latter sequences views based on their dependencies. The commit also adds a new method, `__hash__`, to the Table class, which returns a hash value of the key of the table, improving the handling of Table objects. Additionally, we have added unit tests and verified the changes on a staging environment. We have also introduced a new file `tables_and_views.json` for unit testing and added a `views_migrator` module that takes a `TablesCrawler` object and returns a sequence of tables (views) that need to be migrated in the correct order. The commit addresses various scenarios such as no views, direct views, indirect views, deep indirect views, invalid SQL, invalid SQL tables, and circular view references. This release is focused on improving the sequencing of view migrations and is accompanied by appropriate tests. * Experimental support for scanning Delta Tables inside Mount Points ([#1095](#1095)). This commit introduces experimental support for scanning Delta Tables located inside mount points using a new `TablesInMounts` crawler. Users can now scan specific mount points using the `--include-mounts` flag and include Parquet files in the scan results with the `--include-parquet-files` flag. Additionally, the `--filter-paths` flag allows for filtering paths in a mount point and the `--max-depth` flag (currently unimplemented) will filter at a specific sub-folder depth in future development. The project dependencies have been updated to use `databricks-labs-lsql~=0.3.0`. This new feature provides a more granular and flexible way to scan Delta Tables, making the project more user-friendly and adaptable to various use cases. * Fixed `NULL` values in `ucx.views.table_format` to have `UNKNOWN` value instead ([#1156](#1156)). This commit includes a fix for handling NULL values in the `table_format` column of Views in the `ucx.views.table_format` module. Previously, NULL values were displayed as-is, but now they will be replaced with the string "UNKNOWN". This change is part of the fix for issue [#115](#115) * Fixing run_workflow functionality for better error handling ([#1159](#1159)). In this release, the `run_workflow` method in the `workflows.py` file has been updated to improve error handling by waiting for the job to terminate or skip before raising an error, allowing for a more detailed error message to be generated. A new method, `job_initial_run`, has been added to initiate a job run and return the run ID, raising a `NotFound` exception if the job run is not found. The `run_workflow` functionality in the `WorkflowsInstall` module has also been enhanced to handle unexpected error types and improve overall error handling during the installation of products. New test cases have been added and existing ones updated to check how the code handles errors when the run ID is not found or when an `OperationFailed` exception is raised during the installation process. These changes improve the robustness and stability of the system. * Use experimental Permissions Migration API also for Legacy Table ACLs ([#1161](#1161)). This release introduces several changes to the group permissions migration functionality and associated tests. The experimental Permissions Migration API is now being utilized for Legacy Table ACLs, which has led to the removal of the verification step from the experimental group migration job. The `TableAclSupport` import and class have been removed, as they are no longer needed. A new `apply_to_renamed_groups` method has been added for production usage, and a `apply_to_groups_with_different_names` method has been added for integration testing, both of which are part of the Permissions Migration API. Additionally, two tests have been added to support the experimental permissions migration for a group with the same name in the workspace and account. The `permission_manager` parameter has been removed from several test functions in the `test_generic.py` file and replaced with the `MigrationState` class, which is used directly with the `WorkspaceClient` object to apply permissions to groups with different names. The `test_some_entitlements` function in the `test_scim.py` file has also been updated to use the `MigratedGroup` class and the `MigrationState` class's `apply_to_groups_with_different_names` method. Finally, new tests for the Permissions Migration API have been added to the `test_tacl.py` file in the `tests/integration/workspace_access` directory to verify the behavior of the Permissions Migration API when migrating different grants.
Merged
nfx
added a commit
that referenced
this pull request
Mar 29, 2024
* Ensure proper sequencing of view migrations ([#1157](#1157)). In this release, we have introduced a `views_migrator` module and corresponding test cases to ensure proper sequencing of view migrations, addressing issue [#1132](#1132). The module contains two main classes: `ViewToMigrate` and `ViewsMigrator`. The former is responsible for parsing a view's SQL text and identifying its dependencies, while the latter sequences views based on their dependencies. The commit also adds a new method, `__hash__`, to the Table class, which returns a hash value of the key of the table, improving the handling of Table objects. Additionally, we have added unit tests and verified the changes on a staging environment. We have also introduced a new file `tables_and_views.json` for unit testing and added a `views_migrator` module that takes a `TablesCrawler` object and returns a sequence of tables (views) that need to be migrated in the correct order. The commit addresses various scenarios such as no views, direct views, indirect views, deep indirect views, invalid SQL, invalid SQL tables, and circular view references. This release is focused on improving the sequencing of view migrations and is accompanied by appropriate tests. * Experimental support for scanning Delta Tables inside Mount Points ([#1095](#1095)). This commit introduces experimental support for scanning Delta Tables located inside mount points using a new `TablesInMounts` crawler. Users can now scan specific mount points using the `--include-mounts` flag and include Parquet files in the scan results with the `--include-parquet-files` flag. Additionally, the `--filter-paths` flag allows for filtering paths in a mount point and the `--max-depth` flag (currently unimplemented) will filter at a specific sub-folder depth in future development. The project dependencies have been updated to use `databricks-labs-lsql~=0.3.0`. This new feature provides a more granular and flexible way to scan Delta Tables, making the project more user-friendly and adaptable to various use cases. * Fixed `NULL` values in `ucx.views.table_format` to have `UNKNOWN` value instead ([#1156](#1156)). This commit includes a fix for handling NULL values in the `table_format` column of Views in the `ucx.views.table_format` module. Previously, NULL values were displayed as-is, but now they will be replaced with the string "UNKNOWN". This change is part of the fix for issue [#115](#115) * Fixing run_workflow functionality for better error handling ([#1159](#1159)). In this release, the `run_workflow` method in the `workflows.py` file has been updated to improve error handling by waiting for the job to terminate or skip before raising an error, allowing for a more detailed error message to be generated. A new method, `job_initial_run`, has been added to initiate a job run and return the run ID, raising a `NotFound` exception if the job run is not found. The `run_workflow` functionality in the `WorkflowsInstall` module has also been enhanced to handle unexpected error types and improve overall error handling during the installation of products. New test cases have been added and existing ones updated to check how the code handles errors when the run ID is not found or when an `OperationFailed` exception is raised during the installation process. These changes improve the robustness and stability of the system. * Use experimental Permissions Migration API also for Legacy Table ACLs ([#1161](#1161)). This release introduces several changes to the group permissions migration functionality and associated tests. The experimental Permissions Migration API is now being utilized for Legacy Table ACLs, which has led to the removal of the verification step from the experimental group migration job. The `TableAclSupport` import and class have been removed, as they are no longer needed. A new `apply_to_renamed_groups` method has been added for production usage, and a `apply_to_groups_with_different_names` method has been added for integration testing, both of which are part of the Permissions Migration API. Additionally, two tests have been added to support the experimental permissions migration for a group with the same name in the workspace and account. The `permission_manager` parameter has been removed from several test functions in the `test_generic.py` file and replaced with the `MigrationState` class, which is used directly with the `WorkspaceClient` object to apply permissions to groups with different names. The `test_some_entitlements` function in the `test_scim.py` file has also been updated to use the `MigratedGroup` class and the `MigrationState` class's `apply_to_groups_with_different_names` method. Finally, new tests for the Permissions Migration API have been added to the `test_tacl.py` file in the `tests/integration/workspace_access` directory to verify the behavior of the Permissions Migration API when migrating different grants.
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 PR addressed issues with the integration test.
We improved handling of workflow runs.
Closes #1143
The 'run_workflow' method in 'workflows.py' has been updated to reliably handle job failures and provide better error information. Previously, the method used 'run_now' to start a job without waiting for it to finish, which could result in an incomplete or failed job. The updated code creates a 'job_initial_run' using 'run_now' and checks if a run ID is returned, indicating that the job has started successfully. If the job fails, an error is raised with more detailed information. Additionally, a new private method '_infer_error_from_job_run' is defined to infer the error from the job run object. In the
test_installation.pyfile, the error handling in the integration test for the installation process has been improved. Previously, when an error occurred, the functionwait_get_run_job_terminated_or_skippedwould raise anOperationFailedexception, but the error message would not be properly propagated. This commit ensures that the error message is properly propagated whenwait_get_run_job_terminated_or_skippedraises anOperationFailedexception. The test intest_install.pyhas also been updated to expect anUnknownerror instead of aBadRequesterror when running a specific workflow. These changes improve the reliability and robustness of the library by enhancing error handling and providing better error information.