gouline / dbt-metabase

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

Read from manifest.json instead of parsing the YAML files #14

Closed fernandobrito closed 3 years ago

fernandobrito commented 3 years ago

I'm using the dbt alias feature in many of my models, which means that my model name doesn't match the respective object in my database and dbt-metabase won't be able to correctly match them when retrieving the schema from Metabase.

This is because dbt-metabase retrieves the model information/name directly from the YAML files written by the dbt users instead of using the dbt compiled artifacts which are now much more stable: https://docs.getdbt.com/reference/artifacts/dbt-artifacts. I also use multiple schemas in my project and the schemas can be defined in multiple places (config block in the model, dbt_project.yml, etc). The compiled manifest.json is the final source of truth of where my model will be materialized (schema, alias, etc).

I propose to expand the CLI and Python interfaces to, besides supporting the existing dbt_path (for backward compatibility), also support a dbt_manifest_path to a manifest.json file (mutually exclusive arguments).

I'm working on a PR for that, but I'm creating this issue here to gather early feedback and see what you think.

PS: Thanks a lot for this project! It will be very useful in my organization.

gouline commented 3 years ago

Agreed, this is a better approach. I believe this will also fix #3. Looking forward to reviewing your PR when ready.

fernandobrito commented 3 years ago

Hi @gouline! Sorry for taking so long to answer. I started working on this for my job, and then it got somehow deprioritized. But then it just became prioritized again, but I'm leaving for vacation by the end of this week, so I had to do some quick and dirty code to get it ready to be used internally before I leave.

There are a few things/extra features I would like to discuss with you to see if you would like them in the PR as well, or if they are too specific and I should rather keep it on my own fork.

Example:

I also opened a feature request on Metabase to enable Markdown formatting on the table and column descriptions: https://github.com/metabase/metabase/issues/15814.

In summary, I'm testing my fork in my org before I leave on holidays, but due to the tight deadline, I pushed some very ugly code that I would like to refactor. Once I'm back in some weeks, I can write here again to discuss which features I should keep out, then I can clean the code a bit, update the docs, add some tests and finally open a PR for review. What do you think? 😄

My WIP branch is here: https://github.com/fernandobrito/dbt-metabase/compare/master...fernandobrito:read-from-artifacts?expand=1

fernandobrito commented 3 years ago

Just documenting here that I'm back from vacation and I see that someone else already picked up my fork, did some improvements, and opened a PR: https://github.com/gouline/dbt-metabase/pull/19.

If/when that PR is merged, I can close this issue.

fernandobrito commented 3 years ago

19 has been merged so I'm closing this issue. 🎉