gouline / dbt-metabase

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

DbtFolder parser: collect schema info for source tables #27

Closed remigabillet closed 3 years ago

remigabillet commented 3 years ago

New take on #24 since the underlying code changed so much since #24 was open.

remigabillet commented 3 years ago

@z3z1ma most of #24 was implemented in #19 so this PR is much smaller.

z3z1ma commented 3 years ago

Looking good @remigabillet thanks for rebasing. I will review more deeply this later today with any code comments (its a minor change so it wont be too bad). One question that came to mind right away that we already know, makes the folder parser a less robust option in comparison to manifest parsing, is a block like this (I personally use this in a production config). Let me know thoughts on this.

version: 2
sources:
- name: modelling
  schema: '{{ target.schema }}'
  loader: python
  loaded_at_field: model_ran_at
  tables:
  - name: fitness_model_3
    identifier: model_gen_f_daily
    description: '# Modelling data for Gen E -> Gen G Hydropanels using a linear model based on TMY solar data'
remigabillet commented 3 years ago

@z3z1ma Good question. I feel that handing jinja variable substitutions is beyond the scope for DbtFolder. It's inherently limited. At that point, the manifest file should be used.

remigabillet commented 3 years ago

@z3z1ma thanks for the suggestions!

gouline commented 3 years ago

Looks good to me, if @z3z1ma is happy then I'll merge.

z3z1ma commented 3 years ago

@gouline

Yep, looks great. I will rebase my fork after this is implemented.

remigabillet commented 3 years ago

Thanks @gouline and @z3z1ma!