gouline / dbt-metabase

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

Exposure Extraction Error | In Case of Missing Cards in collection #237

Closed dosnep closed 3 months ago

dosnep commented 4 months ago

Context:

When I use Metabase API to delete archived cards, it creates inconsistency because card are still referenced in collections. So at this step, it's possible to search a deleted card/dashboard causing an HTTP code 400 error and makes application crash.

Problem

In case of inconsistency between collections and cards/dashboards, dbt-metabase will crash because it tries to query a missing card/dashboard.

Request

Skip a card/dashboard which is referenced in a collection but has been deleted.

Technical Solution

Test for each items in a Collection if the associated resource is reachable otherwise continue to the next item. We could add a try&catch statement for get_dashboard and get_card as we did here

Note

Event if it's not the purpose of that tool, it could be interresting to highlight inconsistency with the logger in case of unreachable item resource.

gouline commented 4 months ago

I'm surprised Metabase still lists deleted cards and dashboards. Does this happen on the latest version? If so, should this be reported to Metabase as a bug too?

dosnep commented 3 months ago

I'm working with an old version (0.45.4.3). Certainly, we should consider upgrading to the latest version. I'll make an effort to work on that and keep you updated. Additionally, I attempted to directly search the database for a table storing the mapping between collection_id and card_id (with a delete cascade configuration on the card table for the card_id primary key), but I couldn't find such a table. It seems that this logic is directly managed by the application for consistency as well.

gouline commented 3 months ago

Assuming you're deleting cards with DELETE /api/card/:id:

DEPRECATED – don’t delete a Card anymore – archive it instead.

You can finish #238 for extra safety, but I'm closing the issue as invalid, because this is unsupported by Metabase.