From f9c7516f594dc2c2be773e0b15af7e4a8b0a6149 Mon Sep 17 00:00:00 2001 From: FastLee Date: Tue, 3 Oct 2023 09:42:53 -0400 Subject: [PATCH] Changed list manipulation. Added 2 tests (for system tables and for workspace only tables) --- .../labs/ucx/workspace_access/groups.py | 9 ++--- tests/unit/workspace_access/test_groups.py | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/databricks/labs/ucx/workspace_access/groups.py b/src/databricks/labs/ucx/workspace_access/groups.py index 16a09e699c..c941415919 100644 --- a/src/databricks/labs/ucx/workspace_access/groups.py +++ b/src/databricks/labs/ucx/workspace_access/groups.py @@ -161,25 +161,24 @@ def prepare_groups_in_environment(self): "exist and are of the correct type. If some temporary groups are missing, they'll be created" ) group_names = self.config.selected + valid_group_names = [] if group_names: logger.info("Using the provided group listing") for g in group_names: if g in self.SYSTEM_GROUPS: logger.info(f"Cannot migrate system group {g}. {g} will be skipped.") - group_names.remove(g) continue if not self._get_group(g, "workspace"): logger.info(f"Group {g} not found on the workspace level. {g} will be skipped.") - group_names.remove(g) continue if not self._get_group(g, "account"): logger.info( f"Group {g} not found on the account level. {g} will be skipped. You can add {g} " f"to the account and rerun the job." ) - group_names.remove(g) continue + valid_group_names.append(g) else: logger.info( "No group listing provided, all available workspace-level groups that have an account-level " @@ -187,9 +186,9 @@ def prepare_groups_in_environment(self): ) ws_group_names = {_.display_name for _ in self._workspace_groups} ac_group_names = {_.display_name for _ in self._account_groups} - group_names = list(ws_group_names.intersection(ac_group_names)) + valid_group_names = list(ws_group_names.intersection(ac_group_names)) - self._set_migration_groups(group_names) + self._set_migration_groups(valid_group_names) logger.info("Environment prepared successfully") @property diff --git a/tests/unit/workspace_access/test_groups.py b/tests/unit/workspace_access/test_groups.py index 760ec29b11..275b13b6d7 100644 --- a/tests/unit/workspace_access/test_groups.py +++ b/tests/unit/workspace_access/test_groups.py @@ -318,6 +318,44 @@ def test_replace_workspace_groups_with_account_groups_should_call_delete_and_do( ) +def test_system_groups(): + client = Mock() + test_ws_group = Group(display_name="admins", meta=ResourceMeta(resource_type="WorkspaceGroup")) + test_acc_group = Group(display_name="admins", meta=ResourceMeta(resource_type="Group")) + backup_group_id = "100" + client.groups.list.return_value = [test_ws_group] + client.groups.create.return_value = Group( + display_name="dbr_backup_de", meta=ResourceMeta(resource_type="WorkspaceGroup"), id=backup_group_id + ) + client.api_client.do.return_value = { + "Resources": [g.as_dict() for g in [test_acc_group]], + } + + group_conf = GroupsConfig(backup_group_prefix="dbr_backup_", selected=["admins"]) + manager = GroupManager(client, group_conf) + manager.prepare_groups_in_environment() + assert len(manager._migration_state.groups) == 0 + + +def test_workspace_only_groups(): + client = Mock() + test_ws_group = Group(display_name="ws_group", meta=ResourceMeta(resource_type="WorkspaceGroup")) + test_acc_group = Group(display_name="acc_group", meta=ResourceMeta(resource_type="Group")) + backup_group_id = "100" + client.groups.list.return_value = [test_ws_group, test_acc_group] + client.groups.create.return_value = Group( + display_name="dbr_backup_de", meta=ResourceMeta(resource_type="WorkspaceGroup"), id=backup_group_id + ) + client.api_client.do.return_value = { + "Resources": [g.as_dict() for g in [test_acc_group]], + } + + group_conf = GroupsConfig(backup_group_prefix="dbr_backup_", selected=["ws_group"]) + manager = GroupManager(client, group_conf) + manager.prepare_groups_in_environment() + assert len(manager._migration_state.groups) == 0 + + def test_delete_backup_groups(): client = Mock()