simplify logic and add documentation

nfx · nfx · commit d221a2095b14 · 2023-10-11T16:41:53.000+02:00
diff --git a/docs/local-group-migration.md b/docs/local-group-migration.md
@@ -238,4 +238,14 @@ ws_group_names = {_.display_name for _ in workspace_groups}
 ac_group_names = {_.display_name for _ in account_groups}
 group_names = list(ws_group_names.intersection(ac_group_names))
 print(f"Found {len(group_names)} groups to migrate")
+```
+
+2. Recover workspace-local groups from backup groups from within a debug notebook:
+
+```python
+from databricks.labs.ucx.workspace_access.groups import  GroupManager
+from databricks.labs.ucx.config import GroupsConfig
+
+group_manager = GroupManager(ws, GroupsConfig(auto=True))
+group_manager.ws_local_group_deletion_recovery()
 ```
diff --git a/src/databricks/labs/ucx/workspace_access/groups.py b/src/databricks/labs/ucx/workspace_access/groups.py
@@ -274,7 +274,6 @@ def delete_backup_groups(self):
         logger.info("Backup groups were successfully deleted")
 
     def ws_local_group_deletion_recovery(self):
-        self._workspace_groups = self._list_workspace_groups()
         workspace_groups = {_.display_name for _ in self._workspace_groups}
         source_groups = [
             g
@@ -297,9 +296,6 @@ def ws_local_group_deletion_recovery(self):
             )
             self._workspace_groups.append(ws_local_group)
             self._migration_state.add(ws_local_group)
-            logger.info(f"Workspace-local group {ws_local_group} successfully created")
-
-            self._ws.groups.delete(source_group.id)
-            logger.info(f"Temporary workspace-local group {ws_local_group} successfully deleted")
+            logger.info(f"Workspace-local group {ws_local_group} successfully recovered")
 
         logger.info("Workspace-local group deletion recovery completed")
diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
@@ -77,18 +77,3 @@ def inner():
         return ws_group, acc_group
 
     return inner
-
-
-@pytest.fixture
-def make_ucx_recovery_group(make_random, make_group, make_acc_group, user_pool):
-    def inner(prefix="db-temp-"):
-        display_name = f"ucx_{make_random(4)}"
-        members = [_.id for _ in random.choices(user_pool, k=random.randint(1, 40))]
-        ws_group = make_group(display_name=display_name, members=members, entitlements=["allow-cluster-create"])
-        backup_group = make_group(
-            display_name=prefix + display_name, members=members, entitlements=["allow-cluster-create"]
-        )
-        make_acc_group(display_name=display_name, members=members)
-        return ws_group, backup_group
-
-    return inner
diff --git a/tests/integration/workspace_access/test_groups.py b/tests/integration/workspace_access/test_groups.py
@@ -54,18 +54,22 @@ def test_id_validity(ws: WorkspaceClient, make_ucx_group):
     assert acc_group.id == manager._get_group(acc_group.display_name, "account").id
 
 
-def test_recover_from_ws_local_deletion(ws, make_ucx_recovery_group):
-    ws_group, backup_group = make_ucx_recovery_group()
-    ws_group_two, backup_group_two = make_ucx_recovery_group()
+def test_recover_from_ws_local_deletion(ws, make_ucx_group):
+    ws_group, _ = make_ucx_group()
+    ws_group_two, _ = make_ucx_group()
 
     group_manager = GroupManager(ws, GroupsConfig(auto=True))
     group_manager.prepare_groups_in_environment()
 
+    # simulate disaster
     ws.groups.delete(ws_group.id)
     ws.groups.delete(ws_group_two.id)
 
+    # recovery run from a debug notebook
+    group_manager = GroupManager(ws, GroupsConfig(auto=True))
     group_manager.ws_local_group_deletion_recovery()
 
+    # normal run after from a job
     group_manager = GroupManager(ws, GroupsConfig(auto=True))
     group_manager.prepare_groups_in_environment()
 
