Add a skip and better logging for fatal dbt issues

[alanmcruickshank]

This is a draft PR to assist with debugging #3587.

We may actually want to consider this a permanent fixture (or a variation of it), to allow dbt projects to a) better understand failing files and b) still lint the rest of the project.

In short, if a dbt_compiler.compile_node() raises any exception - we skip the file and output to the user a stringified version of the dbt error and a reference to the file which caused it for later debugging.

[codecov]

Codecov Report

Merging #3603 (8581ab8) into main (32687ca) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main     #3603   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          174       174           
  Lines        13244     13244           
=========================================
  Hits         13244     13244           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32687ca...8581ab8. Read the comment docs.

[barrywhart]

What does the logging look like from this? Curious if we should do one or both of:

Logging where the exception was raised from:

logger.exception("<<MESSAGE>>")

Exception chaining:

raise SQLTemplaterSkipFile(...) from err

[alanmcruickshank]

Both are things I've never used before 🤔 .

On reading - I think we should definitely do the latter. I think it will still result in a skipped file. Doing the former seems like a good idea too. Perhaps with some more information attached.

Great ideas!

[barrywhart]

I'm a bit of a Python 🤓, I suppose. 😝

[alanmcruickshank]

Take a look at what I've done now. Used both 🤓

[alanmcruickshank]

@barrywhart - do you reckon this is a good idea to merge to main, or should I keep this is a debugging branch for now?

On one side, this makes the fatal compilation errors much more manageable for dbt users, and allows some files to be linted (hopefully) - but it also reduces the visibility of those bugs and unless people see the errors, they might no know they're there.

I think I'm leaning toward merge, but could be persuaded otherways.

Views?

@NiallRees - do you have views as a regular dbt user?

[NiallRees]

@barrywhart - do you reckon this is a good idea to merge to main, or should I keep this is a debugging branch for now?

On one side, this makes the fatal compilation errors much more manageable for dbt users, and allows some files to be linted (hopefully) - but it also reduces the visibility of those bugs and unless people see the errors, they might no know they're there.

I think I'm leaning toward merge, but could be persuaded otherways.

Views?

@NiallRees - do you have views as a regular dbt user?

I think it makes sense to merge given that the models were failing anyway, I've not encountered scenarios where the first time I find a model won't compile is from SQLFluff though!

[alanmcruickshank]

Given what @NiallRees says, I'm going to merge this later today unless anyone else has strong views.

[barrywhart]

I'm good with merging.

One more possibility -- change the logging output to display in red and/or add a similar print() that displays in red. This makes it less likely to be overlooked.

[alanmcruickshank]

One more possibility -- change the logging output to display in red and/or add a similar print() that displays in red. This makes it less likely to be overlooked.

The text from the templater skip error will already get printed in red 😄 👍

[noel]

would this resolve the issue where it appears everything is good when dbt compile actually failed?