Skip to content

Commit 65d30d3

Browse files
authored
refactor workspace_client_mock to use installation.load (#1056)
## Changes <!-- Summary of your changes that are easy to understand. Add screenshots when necessary --> This removes the need of `from_dict` in the underlying class when loading from `workspace_client_mock` ### Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [x] manually tested - [x] verified on staging environment
1 parent e9765dd commit 65d30d3

File tree

15 files changed

+51
-77
lines changed

15 files changed

+51
-77
lines changed

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

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
import re
33
from dataclasses import dataclass
44
from functools import partial
5-
from typing import Any
65

76
from databricks.labs.blueprint.installation import Installation
87
from databricks.labs.blueprint.parallel import Threads
@@ -28,18 +27,6 @@ class Rule:
2827
src_table: str
2928
dst_table: str
3029

31-
@classmethod
32-
def from_dict(cls, data: dict[str, Any]):
33-
"""Deserializes the Rule from a dictionary."""
34-
return cls(
35-
workspace_name=data["workspace_name"],
36-
catalog_name=data["catalog_name"],
37-
src_schema=data["src_schema"],
38-
dst_schema=data["dst_schema"],
39-
src_table=data["src_table"],
40-
dst_table=data["dst_table"],
41-
)
42-
4330
@classmethod
4431
def initial(cls, workspace_name: str, catalog_name: str, table: Table) -> "Rule":
4532
return cls(
@@ -65,11 +52,6 @@ class TableToMigrate:
6552
src: Table
6653
rule: Rule
6754

68-
@classmethod
69-
def from_dict(cls, data: dict[str, Any]):
70-
"""Deserializes the TableToMigrate from a dictionary."""
71-
return cls(Table.from_dict(data["table"]), Rule.from_dict(data["rule"]))
72-
7355

7456
class TableMapping:
7557
UCX_SKIP_PROPERTY = "databricks.labs.ucx.skip"

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,20 +56,6 @@ class Table:
5656

5757
UPGRADED_FROM_WS_PARAM: typing.ClassVar[str] = "upgraded_from_workspace_id"
5858

59-
@classmethod
60-
def from_dict(cls, data: dict[str, typing.Any]):
61-
return cls(
62-
catalog=data.get("catalog", "UNKNOWN"),
63-
database=data.get("database", "UNKNOWN"),
64-
name=data.get("name", "UNKNOWN"),
65-
object_type=data.get("object_type", "UNKNOWN"),
66-
table_format=data.get("table_format", "UNKNOWN"),
67-
location=data.get("location", None),
68-
view_text=data.get("view_text", None),
69-
upgraded_to=data.get("upgraded_to", None),
70-
storage_properties=data.get("storage_properties", None),
71-
)
72-
7359
@property
7460
def is_delta(self) -> bool:
7561
if self.table_format is None:

tests/unit/__init__.py

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import pathlib
55
from unittest.mock import create_autospec
66

7+
from databricks.labs.blueprint.installation import MockInstallation
78
from databricks.sdk import WorkspaceClient
89
from databricks.sdk.errors import NotFound
910
from databricks.sdk.service.compute import ClusterDetails, Policy
@@ -59,30 +60,32 @@ def _load_fixture(filename: str):
5960
PipelineStateInfo: 'assessment/pipelines',
6061
Policy: 'assessment/policies',
6162
TableToMigrate: 'hive_metastore/tables',
63+
EndpointConfPair: 'assessment/warehouses',
6264
}
6365

6466

65-
def _load_list(cls: type, filename: str, ids=None):
66-
if not ids: # TODO: remove
67-
return [cls.from_dict(_) for _ in _load_fixture(filename)] # type: ignore[attr-defined]
68-
return _id_list(cls, ids)
67+
def _load_list(cls: type, filename: str):
68+
fixtures = _load_fixture(f'{_FOLDERS[cls]}/{filename}.json')
69+
installation = MockInstallation(DEFAULT_CONFIG | {str(num): fixture for num, fixture in enumerate(fixtures)})
70+
return [installation.load(cls, filename=str(num)) for num in range(len(fixtures))]
6971

7072

7173
def _id_list(cls: type, ids=None):
7274
if not ids:
7375
return []
74-
return [cls.from_dict(_load_fixture(f'{_FOLDERS[cls]}/{_}.json')) for _ in ids] # type: ignore[attr-defined]
76+
installation = MockInstallation(DEFAULT_CONFIG | {_: _load_fixture(f'{_FOLDERS[cls]}/{_}.json') for _ in ids})
77+
return [installation.load(cls, filename=_) for _ in ids]
7578

7679

7780
def _cluster_policy(policy_id: str):
78-
fixture = _load_fixture(f"assessment/policies/{policy_id}.json")
81+
fixture = _load_fixture(f"{_FOLDERS[Policy]}/{policy_id}.json")
7982
definition = json.dumps(fixture["definition"])
8083
overrides = json.dumps(fixture["policy_family_definition_overrides"])
8184
return Policy(description=definition, policy_family_definition_overrides=overrides)
8285

8386

8487
def _pipeline(pipeline_id: str):
85-
fixture = _load_fixture(f"assessment/pipelines/{pipeline_id}.json")
88+
fixture = _load_fixture(f"{_FOLDERS[PipelineStateInfo]}/{pipeline_id}.json")
8689
return GetPipelineResponse.from_dict(fixture)
8790

8891

@@ -97,7 +100,7 @@ def workspace_client_mock(
97100
job_ids: list[str] | None = None,
98101
jobruns_ids: list[str] | None = None,
99102
policy_ids: list[str] | None = None,
100-
warehouse_config="single-config.json",
103+
warehouse_config="single-config",
101104
secret_exists=True,
102105
):
103106
ws = create_autospec(WorkspaceClient)
@@ -108,9 +111,7 @@ def workspace_client_mock(
108111
ws.pipelines.get = _pipeline
109112
ws.jobs.list.return_value = _id_list(BaseJob, job_ids)
110113
ws.jobs.list_runs.return_value = _id_list(BaseRun, jobruns_ids)
111-
ws.warehouses.get_workspace_warehouse_config().data_access_config = _load_list(
112-
EndpointConfPair, f"assessment/warehouses/{warehouse_config}"
113-
)
114+
ws.warehouses.get_workspace_warehouse_config().data_access_config = _load_list(EndpointConfPair, warehouse_config)
114115
ws.workspace.export = _workspace_export
115116
if secret_exists:
116117
ws.secrets.get_secret.return_value = GetSecretResponse(key="username", value="SGVsbG8sIFdvcmxkIQ==")

tests/unit/assessment/clusters/policy-single-user-with-empty-appid-spn.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"max_workers": 6,
44
"min_workers": 1
55
},
6-
"cluster_source": "ClusterSource.UI",
6+
"cluster_source": "UI",
77
"spark_context_id": 5134472582179565315,
88
"spark_version": "9.3.x-cpu-ml-scala2.12",
99
"cluster_id": "0810-225833-atlanta69",

tests/unit/assessment/clusters/policy-spn-in-policy-overrides.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"max_workers": 6,
44
"min_workers": 1
55
},
6-
"cluster_source": "ClusterSource.UI",
6+
"cluster_source": "UI",
77
"spark_context_id": 5134472582179565315,
88
"spark_version": "9.3.x-cpu-ml-scala2.12",
99
"cluster_id": "0810-225833-atlanta69",

tests/unit/assessment/jobruns/gitsource_task.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"start_time": 1704085200000,
55
"timeout_seconds": 86400,
66
"git_source": {
7+
"git_provider": "gitHub",
78
"git_url": "https://foo/bar"
89
},
910

tests/unit/assessment/jobruns/sql_tasks.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"task_key": "path",
99
"existing_cluster_id": "outdated-autoscale",
1010
"sql_task": {
11+
"warehouse_id": "outdated-autoscale",
1112
"file": {
1213
"path": "/foo/bar"
1314
}
@@ -17,6 +18,7 @@
1718
"task_key": "alert",
1819
"existing_cluster_id": "outdated-autoscale",
1920
"sql_task": {
21+
"warehouse_id": "outdated-autoscale",
2022
"alert": {
2123
"alert_id": "a123"
2224
}
@@ -26,6 +28,7 @@
2628
"task_key": "dashboard",
2729
"existing_cluster_id": "outdated-autoscale",
2830
"sql_task": {
31+
"warehouse_id": "outdated-autoscale",
2932
"dashboard": {
3033
"dashboard_id": "d123"
3134
}
@@ -35,6 +38,7 @@
3538
"task_key": "query",
3639
"existing_cluster_id": "outdated-autoscale",
3740
"sql_task": {
41+
"warehouse_id": "outdated-autoscale",
3842
"dashboard": {
3943
"dashboard_id": "q123"
4044
}

tests/unit/assessment/test_azure.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ def test_azure_service_principal_info_crawl():
1010
cluster_ids=['azure-spn-secret', 'simplest-autoscale'],
1111
pipeline_ids=['spec-with-spn'],
1212
job_ids=['some-spn'],
13-
warehouse_config="spn-config.json",
13+
warehouse_config="spn-config",
1414
secret_exists=True,
1515
)
1616
spn_crawler = AzureServicePrincipalCrawler(ws, MockBackend(), "ucx").snapshot()
@@ -23,7 +23,7 @@ def test_azure_service_principal_info_spark_conf_crawl():
2323
cluster_ids=['simplest-autoscale'],
2424
pipeline_ids=['empty-spec'],
2525
job_ids=['some-spn'],
26-
warehouse_config="spn-config.json",
26+
warehouse_config="spn-config",
2727
)
2828

2929
spn_crawler = AzureServicePrincipalCrawler(ws, MockBackend(), "ucx").snapshot()
@@ -36,7 +36,7 @@ def test_azure_service_principal_info_no_spark_conf_crawl():
3636
cluster_ids=['simplest-autoscale'],
3737
pipeline_ids=['empty-spec'],
3838
job_ids=['single-job'],
39-
warehouse_config="single-config.json",
39+
warehouse_config="single-config",
4040
)
4141

4242
spn_crawler = AzureServicePrincipalCrawler(ws, MockBackend(), "ucx").snapshot()
@@ -70,7 +70,7 @@ def test_list_all_cluster_with_spn_in_spark_conf_with_secret():
7070

7171
def test_list_all_wh_config_with_spn_no_secret():
7272
ws = workspace_client_mock(
73-
cluster_ids=['simplest-autoscale'], pipeline_ids=['empty-spec'], warehouse_config="spn-config.json"
73+
cluster_ids=['simplest-autoscale'], pipeline_ids=['empty-spec'], warehouse_config="spn-config"
7474
)
7575
result_set = AzureServicePrincipalCrawler(ws, MockBackend(), "ucx").snapshot()
7676

@@ -84,7 +84,7 @@ def test_list_all_wh_config_with_spn_and_secret():
8484
ws = workspace_client_mock(
8585
cluster_ids=['simplest-autoscale'],
8686
pipeline_ids=['empty-spec'],
87-
warehouse_config="spn-secret-config.json",
87+
warehouse_config="spn-secret-config",
8888
secret_exists=True,
8989
)
9090
result_set = AzureServicePrincipalCrawler(ws, MockBackend(), "ucx").snapshot()
@@ -107,7 +107,7 @@ def test_azure_service_principal_info_policy_conf():
107107
cluster_ids=['policy-single-user-with-spn', 'policy-azure-oauth'],
108108
pipeline_ids=['spec-with-spn'],
109109
job_ids=['policy-single-job-with-spn'],
110-
warehouse_config="spn-config.json",
110+
warehouse_config="spn-config",
111111
secret_exists=True,
112112
)
113113
spn_crawler = AzureServicePrincipalCrawler(ws, MockBackend(), "ucx").snapshot()
@@ -120,7 +120,7 @@ def test_azure_service_principal_info_dedupe():
120120
cluster_ids=['policy-single-user-with-spn'],
121121
pipeline_ids=['spec-with-spn'],
122122
job_ids=['policy-single-job-with-spn'],
123-
warehouse_config="dupe-spn-config.json",
123+
warehouse_config="dupe-spn-config",
124124
secret_exists=True,
125125
)
126126
spn_crawler = AzureServicePrincipalCrawler(ws, MockBackend(), "ucx").snapshot()

tests/unit/assessment/test_jobs.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,38 +64,38 @@ def test_job_crawler_with_no_owner_should_have_empty_creator_name():
6464
(
6565
['notebook_task'],
6666
['outdated-autoscale'],
67-
'["123"]',
67+
'[123]',
6868
'["not supported DBR: 9.3.x-cpu-ml-scala2.12"]',
6969
),
7070
(
7171
['notebook_task', 'notebook_dupe_task'],
7272
['outdated-autoscale'],
73-
'["123", "124"]',
73+
'[123, 124]',
7474
'["not supported DBR: 9.3.x-cpu-ml-scala2.12"]',
7575
),
7676
(
7777
['sql_tasks'],
7878
['outdated-autoscale'],
79-
'["123"]',
79+
'[123]',
8080
'["not supported DBR: 9.3.x-cpu-ml-scala2.12"]',
8181
),
82-
(['gitsource_task'], ['outdated-autoscale'], '["123"]', '["not supported DBR: 9.3.x-cpu-ml-scala2.12"]'),
83-
(['dbt_task'], ['outdated-autoscale'], '["123"]', '["not supported DBR: 9.3.x-cpu-ml-scala2.12"]'),
84-
(['jar_task'], ['outdated-autoscale'], '["123"]', '["not supported DBR: 9.3.x-cpu-ml-scala2.12"]'),
85-
(['python_wheel_task'], ['outdated-autoscale'], '["123"]', '["not supported DBR: 9.3.x-cpu-ml-scala2.12"]'),
86-
(['run_condition_task'], ['outdated-autoscale'], '["123"]', '["not supported DBR: 9.3.x-cpu-ml-scala2.12"]'),
87-
(['notebook_no_failure_task'], ['simplest-autoscale'], '["123"]', '[]'),
82+
(['gitsource_task'], ['outdated-autoscale'], '[123]', '["not supported DBR: 9.3.x-cpu-ml-scala2.12"]'),
83+
(['dbt_task'], ['outdated-autoscale'], '[123]', '["not supported DBR: 9.3.x-cpu-ml-scala2.12"]'),
84+
(['jar_task'], ['outdated-autoscale'], '[123]', '["not supported DBR: 9.3.x-cpu-ml-scala2.12"]'),
85+
(['python_wheel_task'], ['outdated-autoscale'], '[123]', '["not supported DBR: 9.3.x-cpu-ml-scala2.12"]'),
86+
(['run_condition_task'], ['outdated-autoscale'], '[123]', '["not supported DBR: 9.3.x-cpu-ml-scala2.12"]'),
87+
(['notebook_no_failure_task'], ['simplest-autoscale'], '[123]', '[]'),
8888
(
8989
['notebook_no_sec_no_comp_task'],
9090
['simplest-autoscale'],
91-
'["123"]',
91+
'[123]',
9292
'["not supported DBR: 9.3.x-cpu-ml-scala2.12"]',
9393
),
94-
(['notebook_no_sec_comp_task'], ['simplest-autoscale'], '["123"]', '["no data security mode specified"]'),
94+
(['notebook_no_sec_comp_task'], ['simplest-autoscale'], '[123]', '["no data security mode specified"]'),
9595
(
9696
['notebook_spark_conf_task'],
9797
['simplest-autoscale'],
98-
'["123"]',
98+
'[123]',
9999
'["unsupported config: spark.databricks.passthrough.enabled"]',
100100
),
101101
],

tests/unit/hive_metastore/tables/external_src.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
{
2-
"table": {
2+
"src": {
33
"catalog": "hive_metastore",
44
"database": "db1_src",
55
"name": "external_src",
6-
"type": "EXTERNAL",
6+
"object_type": "EXTERNAL",
77
"table_format": "DELTA"
88
},
99
"rule": {

0 commit comments

Comments
 (0)