gouline / dbt-metabase

dbt + Metabase integration
https://pypi.org/project/dbt-metabase/
MIT License
442 stars 63 forks source link

Deactiveted user in metabase causes an error during exposures sync #88

Closed arathunku closed 2 years ago

arathunku commented 2 years ago

Hello,

when a user is deactivated, metabase returns 404 for any API requests regarding this user, and this causes an exception during exposures sync:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/dbtmetabase/metabase.py", line 847, in api
    response.raise_for_status()
  File "/usr/local/lib/python3.8/site-packages/requests/models.py", line 940, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://metabase-internal.gatserver.com/api/user/15
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/usr/local/bin/dbt-metabase", line 5, in <module>
    dbtmetabase.main()
  File "/usr/local/lib/python3.8/site-packages/dbtmetabase/__init__.py", line 720, in main
    cli(max_content_width=600)  # pylint: disable=unexpected-keyword-arg
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/local/lib/python3.8/site-packages/dbtmetabase/__init__.py", line 113, in invoke
    return super().invoke(ctx)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/dbtmetabase/__init__.py", line 262, in wrapper
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/dbtmetabase/__init__.py", line 709, in exposures
    metabase.client.extract_exposures(
  File "/usr/local/lib/python3.8/site-packages/dbtmetabase/metabase.py", line 583, in extract_exposures
    creator = self.api("get", f"/api/user/{exposure['creator_id']}")
  File "/usr/local/lib/python3.8/site-packages/dbtmetabase/metabase.py", line 849, in api
    if "password" in kwargs["json"]:

There's missing error handler at: https://github.com/gouline/dbt-metabase/blob/275a72dac489916479ed1309c837206db329ec9d/dbtmetabase/metabase.py#L595

                    creator = self.api("get", f"/api/user/{exposure['creator_id']}")
                    creator_email = creator["email"]
                    creator_name = creator["common_name"]

I'm considering submitting a PR where 404 errors would be handled and if is happening, both creator email and name would be empty. I wanted to confirm if that would be an acceptable solution in this case?

z3z1ma commented 2 years ago

Yes that makes sense to me, I wonder if its worth using something like "Unknown" or "Inactive User" for the common name Either way we should handle this, yes.

KenxinKun commented 2 years ago

I'm getting this same issue with deactivated users https://github.com/gouline/dbt-metabase/issues/90 (sorry for the duplicate issue!)