Use experimental Permissions Migration API also for Legacy Table ACLs #1161
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1161 +/- ##
==========================================
+ Coverage 89.94% 90.01% +0.06%
==========================================
Files 62 62
Lines 7441 7432 -9
Branches 1335 1335
==========================================
- Hits 6693 6690 -3
+ Misses 480 472 -8
- Partials 268 270 +2 ☔ View full report in Codecov by Sentry. |
|
❌ 135/137 passed, 2 flaky, 2 failed, 20 skipped, 1h23m42s total ❌ test_check_inventory_database_exists: Failed: DID NOT RAISE (1m51.989s)❌ test_compare_remote_local_install_versions: Failed: DID NOT RAISE (1m31.191s)Flaky tests:
Running from acceptance #1900 |
This PR removes verification step from the experimental group migration job, as platform already replaces permissions from workspace group to account group and old validation method is not required anymore.
nfx
force-pushed
the
fix/permissions-migration
branch
from
5e8b97e to
59e48d9
Compare
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 removes verification step from the experimental group migration job, as platform already replaces permissions from workspace group to account group and old validation method is not required anymore.