Skip to content

Commit f4e9754

Browse files
authored
fix(import ownership): Support emails with uppercase characters (#348)
* Add logging + write to file * Adding more logging * Normalizing emails as lowercase * Improving logic * Adding test coverage * Improving test coverage
1 parent 7619fd7 commit f4e9754

File tree

4 files changed

+19
-7
lines changed

4 files changed

+19
-7
lines changed

src/preset_cli/api/clients/preset.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,14 +153,15 @@ def export_users(self, workspace_url: URL) -> Iterator[UserType]:
153153
validate_response(response)
154154
payload = response.json()
155155

156+
# Teams with SAML SSO might have emails with uppercase characters
156157
team_members: List[UserType] = [
157158
{
158159
"id": 0,
159160
"username": payload["user"]["username"],
160161
"role": [], # TODO (betodealmeida)
161162
"first_name": payload["user"]["first_name"],
162163
"last_name": payload["user"]["last_name"],
163-
"email": payload["user"]["email"],
164+
"email": payload["user"]["email"].lower(),
164165
}
165166
for payload in payload["payload"]
166167
]
@@ -189,8 +190,12 @@ def export_users(self, workspace_url: URL) -> Iterator[UserType]:
189190
if not payload["result"]:
190191
break
191192

193+
# Teams with SAML SSO might have emails with uppercase characters
192194
ids.update(
193-
{user["extra"]["email"]: user["value"] for user in payload["result"]},
195+
{
196+
user["extra"]["email"].lower(): user["value"]
197+
for user in payload["result"]
198+
},
194199
)
195200

196201
page += 1

src/preset_cli/api/clients/superset.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,7 @@ def update_resource(
492492
if query_args:
493493
url %= query_args
494494

495+
_logger.debug("PUT %s\n%s", url, json.dumps(kwargs, indent=4))
495496
response = self.session.put(url, json=kwargs)
496497
validate_response(response)
497498

@@ -1189,7 +1190,7 @@ def import_ownership(
11891190
"""
11901191
if ownership["uuid"] in resource_ids:
11911192
resource_id = resource_ids[ownership["uuid"]]
1192-
owner_ids = [user_ids[email] for email in ownership["owners"]]
1193+
owner_ids = [user_ids[email.lower()] for email in ownership["owners"]]
11931194
self.update_resource(resource_name, resource_id, owners=owner_ids)
11941195
else:
11951196
raise Exception(

src/preset_cli/cli/superset/import_.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,13 @@ def import_ownership( # pylint: disable=too-many-locals
116116
users,
117117
resource_ids,
118118
)
119-
except Exception: # pylint: disable=broad-except
119+
except Exception as exc: # pylint: disable=broad-except
120+
_logger.debug(
121+
"Failed to import ownership for %s %s: %s",
122+
resource_name,
123+
ownership["name"],
124+
str(exc),
125+
)
120126
if not continue_on_error:
121127
raise
122128
asset_log["status"] = "FAILED"

tests/api/clients/superset_test.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1897,7 +1897,7 @@ def test_export_users_preset(requests_mock: Mocker) -> None:
18971897
"username": "bdoe",
18981898
"first_name": "Bob",
18991899
"last_name": "Doe",
1900-
"email": "bdoe@example.com",
1900+
"email": "BDoe@example.com",
19011901
},
19021902
},
19031903
],
@@ -1950,7 +1950,7 @@ def test_export_users_preset(requests_mock: Mocker) -> None:
19501950
{
19511951
"extra": {
19521952
"active": True,
1953-
"email": "cdoe@example.com",
1953+
"email": "CDoe@example.com",
19541954
},
19551955
"text": "Clarisse Doe",
19561956
"value": 3,
@@ -3529,7 +3529,7 @@ def test_import_ownership(requests_mock: Mocker) -> None:
35293529
"dataset",
35303530
{
35313531
"name": "another_table",
3532-
"owners": ["[email protected]", "other_adoe@example.com"],
3532+
"owners": ["[email protected]", "Other_Adoe@example.com"],
35333533
"uuid": "1192072c-4bee-4535-b8ee-e9f5fc4eb6a2",
35343534
},
35353535
users,

0 commit comments

Comments
 (0)