Improve CompilationException messaging for generic test Jinja rendering #5393

nicholasyager · 2022-06-17T19:07:32Z

Resolves #5294

Description

In issue #5294, the reporter was having difficulties identifying the source of a CompliationException during a migration from 0.19.1 to 1.0.1. The original error message is as follows:

Compilation Error
  'dbt_utils' is undefined. This can happen when calling a macro that does not exist. Check for typos and/or install package dependencies with "dbt deps".

The message above does not provide any context as to where the issue was originating from.

This PR adds an exception handler for UndefinedMacroExceptions to the TestBuilder class so that additional context can be added to the exception raised in the event that a custom macro (or otherwise undefined macro) is reference in a generic test's configuration values. Additionally, a CompilationException exception handler was added to the _parse_generic_test method of SchemaParser so that the schema file that raised the CompilationException is also reported.

The new exception message is as follows:

Compilation Error
  Invalid generic test configuration given in models/example/schema.yml:
  The {target.name}.{column_name} column's "{test_name}" test references an undefined macro in its where {config_name} argument. The macro '{macro_name}' is undefined.
  Please note that the generic test configuration parser currently does not support using custom macros to populate configuration values
  	@: UnparsedNodeUpdate(original_file_path='model...ne)

A test has been added to confirm that a CompilationException is raised as expected when a custom macro is referenced in a generic test's config values, and to confirm that the appropriate exception message is returned.

Checklist

I have read the contributing guide and understand what's expected of me
I have signed the CLA
I have run this code in development and it appears to resolve the stated issue
This PR includes tests, or tests are not required/relevant for this PR
I have opened an issue to add/update docs, or docs changes are not required/relevant for this PR
I have run changie new to create a changelog entry

The error message rendered from the `UndefinedMacroException` when raised by a TestBuilder is very vague as to where the problem is and how to resolve it. This commit adds a basic amount of information about the specific model and column that is referencing an undefined macro. Note: All custom macros referenced in a generic test config will raise an UndefinedMacroException as of v0.20.0.

I realized that this exception information would be better if CompilationExceptions inclulded the file that raised the exception. To that end, I created a new exception handler in `_parse_generic_test` to report on CompilationExceptions raised during the parsing of generic tests. Along the way I reformatted the message returned from TestBuilder to play nicely with the the existing formatting of `_parse_generic_test`'s exception handling code.

I've added a basic test to confirm that the approriate CompilationException when a custom macro is referenced in a generic test config.

github-actions · 2022-06-17T19:07:50Z

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

emmyoop

The messages need a bit more work but this is on the right track!

.changes/unreleased/Under the Hood-20220617-150744.yaml

core/dbt/parser/generic_test_builders.py

core/dbt/parser/schemas.py

@emmyoop

Thanks to @emmyoop for the recommendation that this be listed as a Fix change instead of an "Under the Hood" change! Co-authored-by: Emily Rockman <[email protected]>

I've simplified the error message raised during a Compilation Error sourced from a test config. Mainly by way of removing tabs and newlines where not required.

This commit moves a format call to a multiline fstring in the schemas.py file for CompilationExceptions.

nicholasyager · 2022-07-21T20:35:28Z

Thank you for the thoughtful feedback @emmyoop! I believe I've made all of the requested changes. Please let me know if I'm missing anything! 😁

emmyoop · 2022-07-25T18:36:39Z

core/dbt/parser/generic_test_builders.py

+                    raise_compiler_error(
+                        f"The {self.target.name}.{column_name} column's "
+                        f'"{self.name}" test references an undefined '
+                        f"macro in its {key} configuration argument. "
+                        f"The macro {e.msg}.\n"
+                        "Please note that the generic test configuration parser "
+                        "currently does not support using custom macros to "
+                        "populate configuration values"


@jtcohen6 Do you have any thoughts in the wording of this exception?

This new error message format looks great to me 👍

The thing that will be super helpful is having all the additional context to help troubleshoot 💪

emmyoop

Looks great! This is a nice improvement to messaging @nicholasyager!

@emmyoop

…ng (dbt-labs#5393) * feat: Improve generic test UndefinedMacroException message The error message rendered from the `UndefinedMacroException` when raised by a TestBuilder is very vague as to where the problem is and how to resolve it. This commit adds a basic amount of information about the specific model and column that is referencing an undefined macro. Note: All custom macros referenced in a generic test config will raise an UndefinedMacroException as of v0.20.0. * feat: Bubble CompilationException into schemas.py I realized that this exception information would be better if CompilationExceptions inclulded the file that raised the exception. To that end, I created a new exception handler in `_parse_generic_test` to report on CompilationExceptions raised during the parsing of generic tests. Along the way I reformatted the message returned from TestBuilder to play nicely with the the existing formatting of `_parse_generic_test`'s exception handling code. * feat: Add tests to confirm CompileException I've added a basic test to confirm that the approriate CompilationException when a custom macro is referenced in a generic test config. * feat: Add changie entry and tweak error msg * Update .changes/unreleased/Under the Hood-20220617-150744.yaml Thanks to @emmyoop for the recommendation that this be listed as a Fix change instead of an "Under the Hood" change! Co-authored-by: Emily Rockman <[email protected]> * fix: Simplified Compliation Error message I've simplified the error message raised during a Compilation Error sourced from a test config. Mainly by way of removing tabs and newlines where not required. * fix: Convert format to fstring in schemas This commit moves a format call to a multiline fstring in the schemas.py file for CompilationExceptions. Co-authored-by: Emily Rockman <[email protected]>

nicholasyager added 3 commits June 17, 2022 13:09

feat: Add tests to confirm CompileException

d45875d

I've added a basic test to confirm that the approriate CompilationException when a custom macro is referenced in a generic test config.

cla-bot bot added the cla:yes label Jun 17, 2022

feat: Add changie entry and tweak error msg

72b1d3e

nicholasyager marked this pull request as ready for review June 17, 2022 19:33

nicholasyager requested review from a team as code owners June 17, 2022 19:33

nicholasyager requested review from nathaniel-may and VersusFacit June 17, 2022 19:33

jtcohen6 added Team:Language ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering labels Jun 17, 2022

emmyoop requested changes Jul 20, 2022

View reviewed changes

.changes/unreleased/Under the Hood-20220617-150744.yaml Outdated Show resolved Hide resolved

core/dbt/parser/generic_test_builders.py Outdated Show resolved Hide resolved

core/dbt/parser/schemas.py Outdated Show resolved Hide resolved

nicholasyager and others added 3 commits July 21, 2022 15:56

Update .changes/unreleased/Under the Hood-20220617-150744.yaml

ebf6ee3

Thanks to @emmyoop for the recommendation that this be listed as a Fix change instead of an "Under the Hood" change! Co-authored-by: Emily Rockman <[email protected]>

fix: Simplified Compliation Error message

2d3ce8e

I've simplified the error message raised during a Compilation Error sourced from a test config. Mainly by way of removing tabs and newlines where not required.

fix: Convert format to fstring in schemas

e82ceff

This commit moves a format call to a multiline fstring in the schemas.py file for CompilationExceptions.

nicholasyager requested a review from emmyoop July 21, 2022 20:35

emmyoop reviewed Jul 25, 2022

View reviewed changes

emmyoop approved these changes Jul 27, 2022

View reviewed changes

emmyoop merged commit 69ce677 into main Jul 27, 2022

emmyoop deleted the ct-683-improve_test_config_macro_exception branch July 27, 2022 14:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve CompilationException messaging for generic test Jinja rendering #5393

Improve CompilationException messaging for generic test Jinja rendering #5393

nicholasyager commented Jun 17, 2022 •

edited

Loading

github-actions bot commented Jun 17, 2022

emmyoop left a comment

nicholasyager commented Jul 21, 2022

emmyoop Jul 25, 2022

dbeatty10 Jul 26, 2022

emmyoop left a comment

Improve CompilationException messaging for generic test Jinja rendering #5393

Improve CompilationException messaging for generic test Jinja rendering #5393

Conversation

nicholasyager commented Jun 17, 2022 • edited Loading

Description

Checklist

github-actions bot commented Jun 17, 2022

emmyoop left a comment

Choose a reason for hiding this comment

nicholasyager commented Jul 21, 2022

emmyoop Jul 25, 2022

Choose a reason for hiding this comment

dbeatty10 Jul 26, 2022

Choose a reason for hiding this comment

emmyoop left a comment

Choose a reason for hiding this comment

nicholasyager commented Jun 17, 2022 •

edited

Loading