Skip to content

Commit b6dc22b

Browse files
authored
Update for Databricks SDK 0.56+ (#4178)
## Changes This PR updates the project to work with Databricks SDK 0.56+ which included breaking changes. Incidental changes included adding type-hints to ensure that all the updated functions are linted; some were not, which prevented some of the breaking changes from being found during linting. ### Linked issues Supersedes #4144. This PR is also required for the upstream blueprint, whose CI/CD always tests UCX with the latest available SDK (even if UCX specifies an earlier one): - databrickslabs/blueprint#243 - databrickslabs/blueprint#244 ### Tests - existing unit tests - existing integration tests
1 parent 931cead commit b6dc22b

File tree

12 files changed

+118
-89
lines changed

12 files changed

+118
-89
lines changed

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ classifiers = [
4545
]
4646

4747

48-
dependencies = ["databricks-sdk>=0.44.0,<0.54.0",
48+
dependencies = ["databricks-sdk>=0.56.0,<0.58.0",
4949
"databricks-labs-lsql>=0.16.0,<0.17.0",
5050
"databricks-labs-blueprint>=0.11.0,<0.12.0",
5151
"PyYAML>=6.0.0,<6.1.0",

src/databricks/labs/ucx/hive_metastore/federation.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,11 +299,11 @@ def _add_missing_permissions_if_needed(self, location_name: str, current_user: s
299299
grants = self._location_grants(location_name)
300300
if Privilege.CREATE_FOREIGN_SECURABLE not in grants[current_user]:
301301
change = PermissionsChange(principal=current_user, add=[Privilege.CREATE_FOREIGN_SECURABLE])
302-
self._ws.grants.update(SecurableType.EXTERNAL_LOCATION, location_name, changes=[change])
302+
self._ws.grants.update(SecurableType.EXTERNAL_LOCATION.value, location_name, changes=[change])
303303

304304
def _location_grants(self, location_name: str) -> dict[str, set[Privilege]]:
305305
grants: dict[str, set[Privilege]] = collections.defaultdict(set)
306-
result = self._ws.grants.get(SecurableType.EXTERNAL_LOCATION, location_name)
306+
result = self._ws.grants.get(SecurableType.EXTERNAL_LOCATION.value, location_name)
307307
if not result.privilege_assignments:
308308
return grants
309309
for assignment in result.privilege_assignments:

src/databricks/labs/ucx/hive_metastore/grants.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -737,11 +737,11 @@ def apply_location_acl(self):
737737
self._update_location_permissions(location_name, principals)
738738
logger.info("Applied all the permission on external location")
739739

740-
def _update_location_permissions(self, location_name: str, principals: list[str]):
740+
def _update_location_permissions(self, location_name: str, principals: list[str]) -> None:
741741
permissions = [Privilege.CREATE_EXTERNAL_TABLE, Privilege.CREATE_EXTERNAL_VOLUME, Privilege.READ_FILES]
742742
changes = [PermissionsChange(add=permissions, principal=principal) for principal in principals]
743743
self._ws.grants.update(
744-
SecurableType.EXTERNAL_LOCATION,
744+
SecurableType.EXTERNAL_LOCATION.value,
745745
location_name,
746746
changes=changes,
747747
)

src/databricks/labs/ucx/hive_metastore/table_move.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,9 +192,9 @@ def _alias_table(
192192
logger.error(f"Failed to alias table {from_table_name}: {err!s}", exc_info=True)
193193
return False
194194

195-
def _reapply_grants(self, from_table_name, to_table_name, *, target_view: bool = False):
195+
def _reapply_grants(self, from_table_name: str, to_table_name: str, *, target_view: bool = False) -> None:
196196
try:
197-
grants = self._ws.grants.get(SecurableType.TABLE, from_table_name)
197+
grants = self._ws.grants.get(SecurableType.TABLE.value, from_table_name)
198198
except NotFound:
199199
logger.warning(f"removed on the backend {from_table_name}")
200200
return
@@ -215,7 +215,7 @@ def _reapply_grants(self, from_table_name, to_table_name, *, target_view: bool =
215215
if privileges:
216216
grants_changes.append(PermissionsChange(list(privileges), permission.principal))
217217

218-
self._ws.grants.update(SecurableType.TABLE, to_table_name, changes=grants_changes)
218+
self._ws.grants.update(SecurableType.TABLE.value, to_table_name, changes=grants_changes)
219219

220220
def _recreate_table(self, from_table_name, to_table_name, *, is_managed: bool, del_table: bool):
221221
drop_table = f"DROP TABLE {escape_sql_identifier(from_table_name)}"

tests/integration/aws/test_access.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from pathlib import Path
44

55
from databricks.labs.blueprint.tui import MockPrompts
6+
from databricks.sdk import WorkspaceClient
67
from databricks.sdk.service.catalog import AwsIamRoleRequest
78
from databricks.sdk.service.compute import DataSecurityMode, AwsAttributes
89
from databricks.sdk.service.iam import PermissionLevel
@@ -104,13 +105,13 @@ def test_create_uber_instance_profile(ws, env_or_skip, make_random, make_cluster
104105

105106
def test_create_external_location_validate_acl(
106107
make_cluster_permissions,
107-
ws,
108+
ws: WorkspaceClient,
108109
make_user,
109110
make_cluster,
110111
aws_cli_ctx,
111112
env_or_skip,
112113
inventory_schema,
113-
):
114+
) -> None:
114115
aws_cli_ctx.workspace_installation.run()
115116
aws_cli_ctx.with_dummy_resource_permission()
116117
aws_cli_ctx.sql_backend.save_table(
@@ -149,13 +150,13 @@ def test_create_external_location_validate_acl(
149150
try:
150151
external_location_migration.run()
151152
permissions = ws.grants.get(
152-
SecurableType.EXTERNAL_LOCATION, external_location_name, principal=cluster_user.user_name
153+
SecurableType.EXTERNAL_LOCATION.value, external_location_name, principal=cluster_user.user_name
153154
)
154155
expected_aws_permission = PrivilegeAssignment(
155156
principal=cluster_user.user_name,
156157
privileges=[Privilege.CREATE_EXTERNAL_TABLE, Privilege.CREATE_EXTERNAL_VOLUME, Privilege.READ_FILES],
157158
)
158-
assert expected_aws_permission in permissions.privilege_assignments
159+
assert permissions.privilege_assignments and expected_aws_permission in permissions.privilege_assignments
159160
finally:
160161
remove_aws_permissions = [
161162
Privilege.CREATE_EXTERNAL_TABLE,
@@ -164,7 +165,7 @@ def test_create_external_location_validate_acl(
164165
]
165166
ws.external_locations.delete(external_location_name)
166167
ws.grants.update(
167-
SecurableType.EXTERNAL_LOCATION,
168+
SecurableType.EXTERNAL_LOCATION.value,
168169
env_or_skip("TEST_A_LOCATION"),
169170
changes=[PermissionsChange(remove=remove_aws_permissions, principal=cluster_user.user_name)],
170171
)

tests/integration/azure/test_locations.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import pytest
55
from databricks.labs.blueprint.installation import MockInstallation
66
from databricks.labs.blueprint.tui import MockPrompts
7+
from databricks.sdk import WorkspaceClient
78
from databricks.sdk.errors.platform import NotFound
89
from databricks.sdk.service.iam import PermissionLevel
910
from databricks.sdk.service.compute import DataSecurityMode
@@ -217,7 +218,9 @@ def test_overlapping_location(caplog, ws, sql_backend, inventory_schema, az_cli_
217218
save_delete_location(ws, "uctest_ziyuanqintest_overlap")
218219

219220

220-
def test_run_validate_acl(make_cluster_permissions, ws, make_user, make_cluster, az_cli_ctx, env_or_skip):
221+
def test_run_validate_acl(
222+
make_cluster_permissions, ws: WorkspaceClient, make_user, make_cluster, az_cli_ctx, env_or_skip
223+
) -> None:
221224
az_cli_ctx.with_dummy_resource_permission()
222225
az_cli_ctx.save_locations()
223226
cluster = make_cluster(single_node=True, spark_conf=_SPARK_CONF, data_security_mode=DataSecurityMode.NONE)
@@ -232,21 +235,21 @@ def test_run_validate_acl(make_cluster_permissions, ws, make_user, make_cluster,
232235
try:
233236
location_migration.run()
234237
permissions = ws.grants.get(
235-
SecurableType.EXTERNAL_LOCATION, env_or_skip("TEST_A_LOCATION"), principal=user.user_name
238+
SecurableType.EXTERNAL_LOCATION.value, env_or_skip("TEST_A_LOCATION"), principal=user.user_name
236239
)
237240
expected_azure_permission = PrivilegeAssignment(
238241
principal=user.user_name,
239242
privileges=[Privilege.CREATE_EXTERNAL_TABLE, Privilege.CREATE_EXTERNAL_VOLUME, Privilege.READ_FILES],
240243
)
241-
assert expected_azure_permission in permissions.privilege_assignments
244+
assert permissions.privilege_assignments and expected_azure_permission in permissions.privilege_assignments
242245
finally:
243246
remove_azure_permissions = [
244247
Privilege.CREATE_EXTERNAL_TABLE,
245248
Privilege.CREATE_EXTERNAL_VOLUME,
246249
Privilege.READ_FILES,
247250
]
248251
ws.grants.update(
249-
SecurableType.EXTERNAL_LOCATION,
252+
SecurableType.EXTERNAL_LOCATION.value,
250253
env_or_skip("TEST_A_LOCATION"),
251254
changes=[PermissionsChange(remove=remove_azure_permissions, principal=user.user_name)],
252255
)

tests/integration/hive_metastore/test_catalog_schema.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@
66
from databricks.sdk import WorkspaceClient
77
from databricks.sdk.errors import NotFound
88
from databricks.sdk.retries import retried
9-
from databricks.sdk.service.catalog import PermissionsList
109
from databricks.sdk.service.compute import DataSecurityMode, AwsAttributes
11-
from databricks.sdk.service.catalog import Privilege, SecurableType, PrivilegeAssignment
10+
from databricks.sdk.service.catalog import GetPermissionsResponse, Privilege, SecurableType, PrivilegeAssignment
1211
from databricks.sdk.service.iam import PermissionLevel
1312

1413
from databricks.labs.ucx.hive_metastore.grants import Grant
@@ -94,8 +93,8 @@ def test_create_catalog_schema_with_principal_acl_azure(
9493
mock_prompts = MockPrompts({"Please provide storage location url for catalog: *": ""})
9594
catalog_schema.create_all_catalogs_schemas(mock_prompts)
9695

97-
schema_grants = ws.grants.get(SecurableType.SCHEMA, schema_name)
98-
catalog_grants = ws.grants.get(SecurableType.CATALOG, catalog_name)
96+
schema_grants = ws.grants.get(SecurableType.SCHEMA.value, schema_name)
97+
catalog_grants = ws.grants.get(SecurableType.CATALOG.value, catalog_name)
9998
schema_grant = PrivilegeAssignment(user.user_name, [Privilege.USE_SCHEMA])
10099
catalog_grant = PrivilegeAssignment(user.user_name, [Privilege.USE_CATALOG])
101100
assert schema_grant in schema_grants.privilege_assignments
@@ -128,8 +127,8 @@ def test_create_catalog_schema_with_principal_acl_aws(
128127
mock_prompts = MockPrompts({"Please provide storage location url for catalog: *": ""})
129128
catalog_schema.create_all_catalogs_schemas(mock_prompts)
130129

131-
schema_grants = ws.grants.get(SecurableType.SCHEMA, schema_name)
132-
catalog_grants = ws.grants.get(SecurableType.CATALOG, catalog_name)
130+
schema_grants = ws.grants.get(SecurableType.SCHEMA.value, schema_name)
131+
catalog_grants = ws.grants.get(SecurableType.CATALOG.value, catalog_name)
133132
schema_grant = PrivilegeAssignment(user.user_name, [Privilege.USE_SCHEMA])
134133
catalog_grant = PrivilegeAssignment(user.user_name, [Privilege.USE_CATALOG])
135134
assert schema_grant in schema_grants.privilege_assignments
@@ -167,8 +166,8 @@ def test_create_catalog_schema_with_legacy_hive_metastore_privileges(
167166
runtime_ctx.catalog_schema.create_all_catalogs_schemas(mock_prompts, properties=properties)
168167

169168
@retried(on=[NotFound], timeout=timedelta(seconds=20))
170-
def get_schema_permissions_list(full_name: str) -> PermissionsList:
171-
return ws.grants.get(SecurableType.SCHEMA, full_name)
169+
def get_schema_permissions_list(full_name: str) -> GetPermissionsResponse:
170+
return ws.grants.get(SecurableType.SCHEMA.value, full_name)
172171

173172
assert (
174173
ws.schemas.get(f"{dst_catalog_name}.{dst_schema_name}").owner

tests/integration/hive_metastore/test_migrate.py

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22
from datetime import timedelta
33

44
import pytest
5-
from databricks.sdk import AccountClient
5+
from databricks.sdk import AccountClient, WorkspaceClient
66
from databricks.sdk.errors import NotFound
77
from databricks.sdk.retries import retried
88
from databricks.sdk.service.compute import DataSecurityMode, AwsAttributes
99
from databricks.sdk.service.catalog import Privilege, SecurableType, TableInfo, TableType
1010
from databricks.sdk.service.iam import PermissionLevel
11+
1112
from databricks.labs.ucx.__about__ import __version__
1213

1314
from databricks.labs.ucx.config import WorkspaceConfig
@@ -650,7 +651,9 @@ def test_mapping_reverts_table(ws, sql_backend, runtime_ctx, make_catalog):
650651

651652

652653
# @retried(on=[NotFound], timeout=timedelta(minutes=3))
653-
def test_migrate_managed_tables_with_acl(ws, sql_backend, runtime_ctx, make_catalog, make_user, env_or_skip):
654+
def test_migrate_managed_tables_with_acl(
655+
ws: WorkspaceClient, sql_backend, runtime_ctx, make_catalog, make_user, env_or_skip
656+
) -> None:
654657
src_schema = runtime_ctx.make_schema(catalog_name="hive_metastore")
655658
src_managed_table = runtime_ctx.make_table(catalog_name=src_schema.catalog_name, schema_name=src_schema.name)
656659
user = make_user()
@@ -687,10 +690,11 @@ def test_migrate_managed_tables_with_acl(ws, sql_backend, runtime_ctx, make_cata
687690
assert len(target_tables) == 1
688691

689692
target_table_properties = ws.tables.get(f"{dst_schema.full_name}.{src_managed_table.name}").properties
693+
assert target_table_properties
690694
assert target_table_properties["upgraded_from"] == src_managed_table.full_name
691695
assert target_table_properties[Table.UPGRADED_FROM_WS_PARAM] == str(ws.get_workspace_id())
692696

693-
target_table_grants = ws.grants.get(SecurableType.TABLE, f"{dst_schema.full_name}.{src_managed_table.name}")
697+
target_table_grants = ws.grants.get(SecurableType.TABLE.value, f"{dst_schema.full_name}.{src_managed_table.name}")
694698
target_principals = [pa for pa in target_table_grants.privilege_assignments or [] if pa.principal == user.user_name]
695699
assert len(target_principals) == 1, f"Missing grant for user {user.user_name}"
696700
assert target_principals[0].privileges == [Privilege.MODIFY, Privilege.SELECT]
@@ -721,8 +725,8 @@ def test_migrate_managed_tables_with_acl(ws, sql_backend, runtime_ctx, make_cata
721725

722726
@retried(on=[NotFound], timeout=timedelta(minutes=3))
723727
def test_migrate_external_tables_with_principal_acl_azure(
724-
ws, make_user, prepared_principal_acl, make_cluster_permissions, make_cluster, make_ucx_group
725-
):
728+
ws: WorkspaceClient, make_user, prepared_principal_acl, make_cluster_permissions, make_cluster, make_ucx_group
729+
) -> None:
726730
if not ws.config.is_azure:
727731
pytest.skip("only works in azure test env")
728732
ctx, table_full_name, _, _ = prepared_principal_acl
@@ -741,7 +745,8 @@ def test_migrate_external_tables_with_principal_acl_azure(
741745
)
742746
table_migrate.migrate_tables(what=What.EXTERNAL_SYNC)
743747

744-
target_table_grants = ws.grants.get(SecurableType.TABLE, table_full_name)
748+
target_table_grants = ws.grants.get(SecurableType.TABLE.value, table_full_name)
749+
assert target_table_grants.privilege_assignments is not None
745750
match = False
746751
for _ in target_table_grants.privilege_assignments:
747752
if _.principal == user_with_cluster_access.user_name and _.privileges == [Privilege.ALL_PRIVILEGES]:
@@ -764,8 +769,8 @@ def test_migrate_external_tables_with_principal_acl_azure(
764769

765770
@retried(on=[NotFound], timeout=timedelta(minutes=3))
766771
def test_migrate_external_tables_with_principal_acl_aws(
767-
ws, make_user, prepared_principal_acl, make_cluster_permissions, make_cluster, env_or_skip
768-
):
772+
ws: WorkspaceClient, make_user, prepared_principal_acl, make_cluster_permissions, make_cluster, env_or_skip
773+
) -> None:
769774
ctx, table_full_name, _, _ = prepared_principal_acl
770775
ctx.with_dummy_resource_permission()
771776
cluster = make_cluster(
@@ -782,7 +787,8 @@ def test_migrate_external_tables_with_principal_acl_aws(
782787
)
783788
table_migrate.migrate_tables(what=What.EXTERNAL_SYNC)
784789

785-
target_table_grants = ws.grants.get(SecurableType.TABLE, table_full_name)
790+
target_table_grants = ws.grants.get(SecurableType.TABLE.value, table_full_name)
791+
assert target_table_grants.privilege_assignments is not None
786792
match = False
787793
for _ in target_table_grants.privilege_assignments:
788794
if _.principal == user.user_name and _.privileges == [Privilege.ALL_PRIVILEGES]:
@@ -793,8 +799,12 @@ def test_migrate_external_tables_with_principal_acl_aws(
793799

794800
@retried(on=[NotFound], timeout=timedelta(minutes=3))
795801
def test_migrate_external_tables_with_principal_acl_aws_warehouse(
796-
ws, make_user, prepared_principal_acl, make_warehouse_permissions, make_warehouse, env_or_skip
797-
):
802+
ws: WorkspaceClient,
803+
make_user,
804+
prepared_principal_acl,
805+
make_warehouse_permissions,
806+
make_warehouse,
807+
) -> None:
798808
if not ws.config.is_aws:
799809
pytest.skip("temporary: only works in aws test env")
800810
ctx, table_full_name, _, _ = prepared_principal_acl
@@ -809,7 +819,8 @@ def test_migrate_external_tables_with_principal_acl_aws_warehouse(
809819
)
810820
table_migrate.migrate_tables(what=What.EXTERNAL_SYNC)
811821

812-
target_table_grants = ws.grants.get(SecurableType.TABLE, table_full_name)
822+
target_table_grants = ws.grants.get(SecurableType.TABLE.value, table_full_name)
823+
assert target_table_grants.privilege_assignments is not None
813824
match = False
814825
for _ in target_table_grants.privilege_assignments:
815826
if _.principal == user.user_name and _.privileges == [Privilege.ALL_PRIVILEGES]:
@@ -879,8 +890,8 @@ def test_migrate_table_in_mount(
879890

880891

881892
def test_migrate_external_tables_with_spn_azure(
882-
ws, make_user, prepared_principal_acl, make_cluster_permissions, make_cluster
883-
):
893+
ws: WorkspaceClient, prepared_principal_acl, make_cluster_permissions, make_cluster
894+
) -> None:
884895
if not ws.config.is_azure:
885896
pytest.skip("temporary: only works in azure test env")
886897
ctx, table_full_name, _, _ = prepared_principal_acl
@@ -897,7 +908,8 @@ def test_migrate_external_tables_with_spn_azure(
897908
)
898909
table_migrate.migrate_tables(what=What.EXTERNAL_SYNC)
899910

900-
target_table_grants = ws.grants.get(SecurableType.TABLE, table_full_name)
911+
target_table_grants = ws.grants.get(SecurableType.TABLE.value, table_full_name)
912+
assert target_table_grants.privilege_assignments is not None
901913
match = False
902914
for _ in target_table_grants.privilege_assignments:
903915
if _.principal == spn_with_mount_access and _.privileges == [Privilege.ALL_PRIVILEGES]:

0 commit comments

Comments
 (0)