Skip to content

Commit

Permalink
Fix test failing due to groups not being created yet (#1669)
Browse files Browse the repository at this point in the history
## Changes
(We think that) sometimes groups are not created before the test run,
e.g.
[this](https://github.com/databrickslabs/ucx/actions/runs/9002466231?pr=1532)
and [that](#1616). This PR
extends the `make_group` with an optional flag to wait for the groups to
be created before continuing with the test. To limit impact on
performance, we don't wait for the groups to be created by default.

### Linked issues
Hopefully esolves #1616

### Functionality 

- [ ] added relevant user documentation
- [ ] added new CLI command
- [ ] modified existing command: `databricks labs ucx ...`
- [ ] added a new workflow
- [ ] modified existing workflow: `...`
- [ ] added a new table
- [ ] modified existing table: `...`

### Tests
<!-- How is this tested? Please see the checklist below and also
describe any other relevant tests -->

- [x] manually tested
- [ ] added unit tests
- [ ] added integration tests
- [ ] verified on staging environment (screenshot attached)
  • Loading branch information
JCZuurmond authored May 9, 2024
1 parent 7018156 commit 7997179
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 3 deletions.
9 changes: 9 additions & 0 deletions src/databricks/labs/ucx/mixins/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ def create(
roles: list[str] | None = None,
entitlements: list[str] | None = None,
display_name: str | None = None,
wait_for_provisioning: bool = False,
**kwargs,
):
kwargs["display_name"] = f"sdk-{make_random(4)}" if display_name is None else display_name
Expand All @@ -640,6 +641,14 @@ def create(
logger.info(f"Account group {group.display_name}: {cfg.host}/users/groups/{group.id}/members")
else:
logger.info(f"Workspace group {group.display_name}: {cfg.host}#setting/accounts/groups/{group.id}")

@retried(on=[NotFound], timeout=timedelta(minutes=2))
def _wait_for_provisioning() -> None:
interface.get(group.id)

if wait_for_provisioning:
_wait_for_provisioning()

return group

yield from factory(name, create, lambda item: interface.delete(item.id))
Expand Down
7 changes: 5 additions & 2 deletions tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ def __init__( # pylint: disable=too-many-arguments
self._make_acc_group = make_acc_group_fixture
self._make_user = make_user_fixture

def make_ucx_group(self, workspace_group_name=None, account_group_name=None):
def make_ucx_group(self, workspace_group_name=None, account_group_name=None, wait_for_provisioning=False):
if not workspace_group_name:
workspace_group_name = f"ucx_G{self._make_random(4)}"
if not account_group_name:
Expand All @@ -580,8 +580,11 @@ def make_ucx_group(self, workspace_group_name=None, account_group_name=None):
display_name=workspace_group_name,
members=members,
entitlements=["allow-cluster-create"],
wait_for_provisioning=wait_for_provisioning,
)
acc_group = self._make_acc_group(
display_name=account_group_name, members=members, wait_for_provisioning=wait_for_provisioning
)
acc_group = self._make_acc_group(display_name=account_group_name, members=members)
return ws_group, acc_group

@cached_property
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test_ext_hms.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def test_running_real_assessment_job_ext_hms(
r"Choose a cluster policy": "0",
},
)
ws_group_a, _ = ext_hms_ctx.make_ucx_group()
ws_group_a, _ = ext_hms_ctx.make_ucx_group(wait_for_provisioning=True)

cluster_policy = make_cluster_policy()
make_cluster_policy_permissions(
Expand Down

0 comments on commit 7997179

Please sign in to comment.