From c32b14e782d8092da259d3c1a83c74bbb98d0005 Mon Sep 17 00:00:00 2001 From: Alex Koch Date: Tue, 19 Apr 2022 17:13:27 -0500 Subject: [PATCH 1/3] handle 404s on deactivated users when exporting exposures --- dbtmetabase/metabase.py | 16 ++++++++++++---- .../exposure/baseline_test_exposures.yml | 3 --- tests/fixtures/mock_api/api/card/3.json | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/dbtmetabase/metabase.py b/dbtmetabase/metabase.py index 3f55019d..1904f11c 100644 --- a/dbtmetabase/metabase.py +++ b/dbtmetabase/metabase.py @@ -593,9 +593,12 @@ def increase_indent(self, flow=False, indentless=False): creator_email = exposure["creator"]["email"] creator_name = exposure["creator"]["common_name"] elif "creator_id" in exposure: - creator = self.api("get", f"/api/user/{exposure['creator_id']}") - creator_email = creator["email"] - creator_name = creator["common_name"] + # If a metabase user is deactivated, the API returns a 404 + creator = self.api( + "get", f"/api/user/{exposure['creator_id']}", critical=False + ) + creator_email = creator.get("email") + creator_name = creator.get("common_name") # No spaces allowed in model names in dbt docs DAG / No duplicate model names exposure_name = exposure_name.replace(" ", "_") @@ -803,7 +806,8 @@ def _build_exposure( ) # Output exposure - return { + + exposure = { "name": name, "description": description, "type": "analysis" if exposure_type == "card" else "dashboard", @@ -819,6 +823,10 @@ def _build_exposure( if exposure.upper() in refable_models ], } + if creator_email is None: + del exposure["owner"] + + return exposure def api( self, diff --git a/tests/fixtures/exposure/baseline_test_exposures.yml b/tests/fixtures/exposure/baseline_test_exposures.yml index d2599ea6..ac602fe6 100644 --- a/tests/fixtures/exposure/baseline_test_exposures.yml +++ b/tests/fixtures/exposure/baseline_test_exposures.yml @@ -391,8 +391,5 @@ exposures: type: analysis url: http://localhost:3000/card/3 maturity: medium - owner: - name: Alexander Butler - email: user@example.com depends_on: - ref('customers') diff --git a/tests/fixtures/mock_api/api/card/3.json b/tests/fixtures/mock_api/api/card/3.json index 122c44e0..89c8a63a 100644 --- a/tests/fixtures/mock_api/api/card/3.json +++ b/tests/fixtures/mock_api/api/card/3.json @@ -1 +1 @@ -{"description": null, "archived": false, "collection_position": null, "table_id": 7, "result_metadata": [{"name": "count", "display_name": "Count", "base_type": "type/BigInteger", "semantic_type": "type/Quantity", "field_ref": ["aggregation", 0], "fingerprint": {"global": {"distinct-count": 1, "nil%": 0.0}, "type": {"type/Number": {"min": 100.0, "q1": 100.0, "q3": 100.0, "max": 100.0, "sd": null, "avg": 100.0}}}}], "creator": {"email": "user@example.com", "first_name": "Alexander", "last_login": "2021-07-21T19:25:28.6083Z", "is_qbnewb": false, "is_superuser": true, "id": 1, "last_name": "Butler", "date_joined": "2021-07-21T05:38:53.637091Z", "common_name": "Alexander Butler"}, "can_write": true, "database_id": 2, "enable_embedding": false, "collection_id": 3, "query_type": "query", "name": "Total customers", "dashboard_count": 1, "creator_id": 1, "updated_at": "2021-07-21T08:01:41.996Z", "made_public_by_id": null, "embedding_params": null, "cache_ttl": null, "dataset_query": {"type": "query", "database": 2, "query": {"source-table": 7, "aggregation": [["count"]]}}, "id": 3, "display": "scalar", "last-edit-info": {"id": 1, "email": "user@example.com", "first_name": "Alexander", "last_name": "Butler", "timestamp": "2021-07-21T08:01:37.449936Z"}, "visualization_settings": {"graph.series_labels": ["number"], "graph.metrics": ["count"], "graph.dimensions": []}, "collection": {"authority_level": null, "description": "Automatically generated cards.", "archived": false, "slug": "a_look_at_your_customers_table", "color": "#f9d45c", "name": "A look at your customers table", "personal_owner_id": null, "id": 3, "location": "/2/", "namespace": null}, "created_at": "2021-07-21T08:01:37.434243Z", "public_uuid": null} \ No newline at end of file +{"description": null, "archived": false, "collection_position": null, "table_id": 7, "result_metadata": [{"name": "count", "display_name": "Count", "base_type": "type/BigInteger", "semantic_type": "type/Quantity", "field_ref": ["aggregation", 0], "fingerprint": {"global": {"distinct-count": 1, "nil%": 0.0}, "type": {"type/Number": {"min": 100.0, "q1": 100.0, "q3": 100.0, "max": 100.0, "sd": null, "avg": 100.0}}}}], "can_write": true, "database_id": 2, "enable_embedding": false, "collection_id": 3, "query_type": "query", "name": "Total customers", "dashboard_count": 1, "creator_id": 2, "updated_at": "2021-07-21T08:01:41.996Z", "made_public_by_id": null, "embedding_params": null, "cache_ttl": null, "dataset_query": {"type": "query", "database": 2, "query": {"source-table": 7, "aggregation": [["count"]]}}, "id": 3, "display": "scalar", "last-edit-info": {"id": 1, "email": "user@example.com", "first_name": "Alexander", "last_name": "Butler", "timestamp": "2021-07-21T08:01:37.449936Z"}, "visualization_settings": {"graph.series_labels": ["number"], "graph.metrics": ["count"], "graph.dimensions": []}, "collection": {"authority_level": null, "description": "Automatically generated cards.", "archived": false, "slug": "a_look_at_your_customers_table", "color": "#f9d45c", "name": "A look at your customers table", "personal_owner_id": null, "id": 3, "location": "/2/", "namespace": null}, "created_at": "2021-07-21T08:01:37.434243Z", "public_uuid": null} From 7cf7eee5c318717404e73cf6306854cb5964dedd Mon Sep 17 00:00:00 2001 From: Alex Koch Date: Tue, 19 Apr 2022 18:29:57 -0500 Subject: [PATCH 2/3] owner email must be a string --- dbtmetabase/metabase.py | 8 ++------ tests/fixtures/exposure/baseline_test_exposures.yml | 3 +++ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/dbtmetabase/metabase.py b/dbtmetabase/metabase.py index 1904f11c..6caf6032 100644 --- a/dbtmetabase/metabase.py +++ b/dbtmetabase/metabase.py @@ -807,7 +807,7 @@ def _build_exposure( # Output exposure - exposure = { + return { "name": name, "description": description, "type": "analysis" if exposure_type == "card" else "dashboard", @@ -815,7 +815,7 @@ def _build_exposure( "maturity": "medium", "owner": { "name": creator_name, - "email": creator_email, + "email": creator_email or "", }, "depends_on": [ refable_models[exposure.upper()] @@ -823,10 +823,6 @@ def _build_exposure( if exposure.upper() in refable_models ], } - if creator_email is None: - del exposure["owner"] - - return exposure def api( self, diff --git a/tests/fixtures/exposure/baseline_test_exposures.yml b/tests/fixtures/exposure/baseline_test_exposures.yml index ac602fe6..7dd462b3 100644 --- a/tests/fixtures/exposure/baseline_test_exposures.yml +++ b/tests/fixtures/exposure/baseline_test_exposures.yml @@ -391,5 +391,8 @@ exposures: type: analysis url: http://localhost:3000/card/3 maturity: medium + owner: + name: null + email: '' depends_on: - ref('customers') From da912489d14b09a6fdbeb078d1be4a98b8a94135 Mon Sep 17 00:00:00 2001 From: Alex Koch Date: Wed, 20 Apr 2022 10:22:38 -0500 Subject: [PATCH 3/3] just handle 404s per PR feedback --- dbtmetabase/metabase.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/dbtmetabase/metabase.py b/dbtmetabase/metabase.py index 6caf6032..b2b7d892 100644 --- a/dbtmetabase/metabase.py +++ b/dbtmetabase/metabase.py @@ -594,9 +594,14 @@ def increase_indent(self, flow=False, indentless=False): creator_name = exposure["creator"]["common_name"] elif "creator_id" in exposure: # If a metabase user is deactivated, the API returns a 404 - creator = self.api( - "get", f"/api/user/{exposure['creator_id']}", critical=False - ) + try: + creator = self.api("get", f"/api/user/{exposure['creator_id']}") + except requests.exceptions.HTTPError as error: + if error.response.status_code == 404: + creator = {} + else: + raise + creator_email = creator.get("email") creator_name = creator.get("common_name") @@ -863,12 +868,12 @@ def api( try: response.raise_for_status() except requests.exceptions.HTTPError: - if "password" in kwargs["json"]: + if "json" in kwargs and "password" in kwargs["json"]: logger().error("HTTP request failed. Response: %s", response.text) else: logger().error( "HTTP request failed. Payload: %s. Response: %s", - kwargs["json"], + kwargs.get("json"), response.text, ) raise