Skip to content

Commit

Permalink
apply make fmt changes
Browse files Browse the repository at this point in the history
  • Loading branch information
qziyuan committed Feb 21, 2024
1 parent 1d8e69c commit b55a812
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 28 deletions.
12 changes: 7 additions & 5 deletions src/databricks/labs/ucx/azure/azure_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,7 @@ def _fetch_client_secret(self, sp_list: list[StoragePermissionMapping]) -> list[
for sp in sp_list:
if sp.client_id in azure_sp_info_with_client_secret:
sp_list_with_secret.append(
ServicePrincipalMigrationInfo(
sp, azure_sp_info_with_client_secret[sp.client_id]
)
ServicePrincipalMigrationInfo(sp, azure_sp_info_with_client_secret[sp.client_id])
)
return sp_list_with_secret

Expand Down Expand Up @@ -231,10 +229,14 @@ def _create_storage_credential(self, sp_migration: ServicePrincipalMigrationInfo
name=name, azure_service_principal=azure_service_principal, comment=comment, read_only=read_only
)

validation_result = self._validate_storage_credential(storage_credential, sp_migration.service_principal.prefix, read_only)
validation_result = self._validate_storage_credential(
storage_credential, sp_migration.service_principal.prefix, read_only
)
return validation_result

def _validate_storage_credential(self, storage_credential, location: str, read_only: bool) -> StorageCredentialValidationResult:
def _validate_storage_credential(
self, storage_credential, location: str, read_only: bool
) -> StorageCredentialValidationResult:
# storage_credential validation creates a temp UC external location, which cannot overlap with
# existing UC external locations. So add a sub folder to the validation location just in case
try:
Expand Down
27 changes: 11 additions & 16 deletions tests/integration/azure/test_azure_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@
AzureServicePrincipalInfo,
StoragePermissionMapping,
)
from databricks.labs.ucx.assessment.crawlers import (
_SECRET_PATTERN,
)
from databricks.labs.ucx.assessment.crawlers import _SECRET_PATTERN
from databricks.labs.ucx.azure.azure_credentials import AzureServicePrincipalMigration


Expand All @@ -23,10 +21,13 @@ def inner(read_only=False) -> dict:
application_id = spark_conf.get("fs.azure.account.oauth2.client.id")

secret_matched = re.search(_SECRET_PATTERN, spark_conf.get("fs.azure.account.oauth2.client.secret"))
secret_scope, secret_key = (
secret_matched.group(1).split("/")[1],
secret_matched.group(1).split("/")[2],
)
if secret_matched:
secret_scope, secret_key = (
secret_matched.group(1).split("/")[1],
secret_matched.group(1).split("/")[2],
)
assert secret_scope is not None
assert secret_key is not None

secret_response = ws.secrets.get_secret(secret_scope, secret_key)
client_secret = base64.b64decode(secret_response.value).decode("utf-8")
Expand Down Expand Up @@ -128,21 +129,15 @@ def test_spn_migration(ws, execute_migration, prepare_spn_migration_test, read_o
# In real life, the READ validation for read only storage credential may fail if there is no file,
# but that is fine, as the storage credential is created, and we just cannot validate it until it's really used.
assert not any(
(res.operation is not None)
and ("WRITE" in res.operation.value)
for res in validation_result.results
(res.operation is not None) and ("WRITE" in res.operation.value) for res in validation_result.results
)
else:
assert any(
(res.operation is not None)
and ("WRITE" in res.operation.value)
and ("PASS" in res.result.value)
(res.operation is not None) and ("WRITE" in res.operation.value) and ("PASS" in res.result.value)
for res in validation_result.results
)
assert any(
(res.operation is not None)
and ("DELETE" in res.operation.value)
and ("PASS" in res.result.value)
(res.operation is not None) and ("DELETE" in res.operation.value) and ("PASS" in res.result.value)
for res in validation_result.results
)
finally:
Expand Down
12 changes: 5 additions & 7 deletions tests/unit/azure/test_azure_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,11 +420,9 @@ def test_execute_migration(caplog, capsys, mocker, ws):
# assert validation results
save_args = sp_migration._installation.save.call_args.args[0]
assert any("The validation is skipped" in arg.results[0].message for arg in save_args)
assert any(("READ" in arg.results[0].operation.value)
and ("PASS" in arg.results[0].result.value)
for arg in save_args
assert any(
("READ" in arg.results[0].operation.value) and ("PASS" in arg.results[0].result.value) for arg in save_args
)
assert any(
("WRITE" in arg.results[0].operation.value) and ("PASS" in arg.results[0].result.value) for arg in save_args
)
assert any(("WRITE" in arg.results[0].operation.value)
and ("PASS" in arg.results[0].result.value)
for arg in save_args
)

0 comments on commit b55a812

Please sign in to comment.