Skip to content

Commit e9c7f5d

Browse files
mwojtyczkanfx
andauthored
Improve error handling by not failing group rename step if a group was removed from account before reflecting it to workspace (#762)
Fixes #761 In rare cases, it can happen that a group is removed from the account before we manage to reflect it in the workspace. It's usually a couple of minutes between listing the groups and reflecting it. This PR will log such cases without failing the tool. --------- Co-authored-by: Serge Smertin <[email protected]>
1 parent db580a9 commit e9c7f5d

File tree

2 files changed

+32
-1
lines changed

2 files changed

+32
-1
lines changed

src/databricks/labs/ucx/workspace_access/groups.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,10 @@ def _reflect_account_group_to_workspace(self, account_group_id: str):
507507
except BadRequest:
508508
# already exists
509509
return True
510+
except NotFound:
511+
# the given group has been removed from the account after getting the group and before running this method
512+
logger.warning("Group with ID: %s does not exist anymore in the Databricks account.", account_group_id)
513+
return True
510514

511515
def _get_strategy(
512516
self, workspace_groups_in_workspace: dict[str, Group], account_groups_in_account: dict[str, Group]

tests/unit/workspace_access/test_groups.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1+
import json
12
from unittest.mock import MagicMock
23

34
import pytest
45
from _pytest.outcomes import fail
56
from databricks.labs.blueprint.parallel import ManyError
67
from databricks.labs.blueprint.tui import MockPrompts
7-
from databricks.sdk.errors import DatabricksError
8+
from databricks.sdk.errors import DatabricksError, ResourceDoesNotExist
89
from databricks.sdk.service import iam
910
from databricks.sdk.service.iam import ComplexValue, Group, ResourceMeta
1011

@@ -380,6 +381,32 @@ def do_side_effect(*args, **kwargs):
380381
gm.reflect_account_groups_on_workspace()
381382

382383

384+
def test_reflect_account_should_not_fail_if_group_not_in_the_account_anymore():
385+
backend = MockBackend(rows={"SELECT": [("1", "de", "de", "test-group-de", "", "", "", "")]})
386+
wsclient = MagicMock()
387+
account_group1 = Group(id="11", display_name="de")
388+
389+
def reflect_account_side_effect(method, *args, **kwargs):
390+
if method == "GET":
391+
return {
392+
"Resources": [g.as_dict() for g in [account_group1]],
393+
}
394+
if method == "PUT":
395+
raise ResourceDoesNotExist(
396+
"The group has been removed from the Databricks account after getting the group "
397+
"and before reflecting it to the workspace."
398+
)
399+
400+
wsclient.api_client.do.side_effect = reflect_account_side_effect
401+
GroupManager(backend, wsclient, inventory_database="inv").reflect_account_groups_on_workspace()
402+
403+
wsclient.api_client.do.assert_called_with(
404+
"PUT",
405+
f"/api/2.0/preview/permissionassignments/principals/{account_group1.id}",
406+
data=json.dumps({"permissions": ["USER"]}),
407+
)
408+
409+
383410
def test_delete_original_workspace_groups_should_delete_relected_acc_groups_in_workspace():
384411
account_id = "11"
385412
ws_id = "1"

0 commit comments

Comments
 (0)