Display all model compatibility errors at once #11
Conversation
If there are multiple incompatibilities between the dbt docs and Metabase, displaying them all at once makes it much easier to fix. This allows the user to fix all errors at once, instead of fixing one, running `dbt-metabase`, fixing the next one, and so on.
vitorbaptista
force-pushed
the
display-all-errors
branch
from
d8b1931 to
0a33fce
Compare
| if column_name not in table_lookup: | ||
| logging.warn("Column %s not found in model %s", column_name, model_name) | ||
| return False | ||
| are_models_compatible = False |
There was a problem hiding this comment.
Once this becomes false, it never goes back to true so just return False in both instances and then return True at the end. The only change here is the else branch (which I'm happy with).
There was a problem hiding this comment.
My intent on this PR was to display all errors in all models and fields, so the user could fix all errors at once instead of fixing one error, running dbt-metabase, fixing another error, running it again, and so on. This helped a lot when running dbt-metabase for the first time -- it was a great way to find discrepancies between my dbt docs and the tables in the DB.
If I removed these are_models_compatible changes and just kept the else branch in this PR, the code's behaviour would be the same as it is now.
There was a problem hiding this comment.
Ah, of course, you're doing it for the logs. Makes sense.
|
Thanks for merging and releasing a new version so quickly (and, of course, the project itself -- it's super useful!) |
If there are multiple incompatibilities between the dbt docs and
Metabase, displaying them all at once makes it much easier to fix. This
allows the user to fix all errors at once, instead of fixing one,
running
dbt-metabase, fixing the next one, and so on.