Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CT-1483] [Bug] Macros whose names contain "materialization" treated as materialization macros by compiler, fail with "no supported_languages..." #6231

Closed
2 tasks done
CraftyIvan opened this issue Nov 9, 2022 · 11 comments · Fixed by #8181
Assignees
Labels
bug Something isn't working

Comments

@CraftyIvan
Copy link

CraftyIvan commented Nov 9, 2022

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

After upgrading to dbt 1.3.0 which adds supported_languages for materialization macros (#5695), all standard macros whose name has "materlialization" as a substring prevent compilation.

A project I'm contributing to contains a macro named get_materialization (it defaults model materializations to view under certain conditions). The parser seems to treat it as a materialization macro for that, even though it's not (location of this in dbt-core code referenced below in "Additional Context").

So when running dbt run or dbt compile I get the following error:

No supported_languages found in materialization macro <project_prefix>__get_materialization

Expected Behavior

Only materialization macros are able to trigger this compilation error when not declaring supported languages.

A regular macro ({% macro_name([params]) %}) with the substring "materialization" in its name is not considered a materialization macro ({% materialization [materialization_name] ... %}) and does not trigger the compilation error.

Steps To Reproduce

  1. Create a new regular macro and have its name contain the substring "materialization", e.g. get_materialization or print_materialization.
  2. Run dbt compile.

Relevant log output

No response

Environment

- OS: macOS 12.5.1
- Python: 3.8.15
- dbt: 1.3.0

Which database adapter are you using with dbt?

snowflake

Additional Context

The parse_unparsed_macros method performs this "generalization":

if "materialization" in name:

@CraftyIvan CraftyIvan added bug Something isn't working triage labels Nov 9, 2022
@github-actions github-actions bot changed the title [Bug] Macros whose names contain "materialization" treated as materialization macros by compiler, fail with "no supported_languages..." [CT-1483] [Bug] Macros whose names contain "materialization" treated as materialization macros by compiler, fail with "no supported_languages..." Nov 9, 2022
@dbeatty10
Copy link
Contributor

Thanks for surfacing this @CraftyIvan !

Agreed that this is a surprising result when there is a non-materialization macro that happens to have "materialization" within the name.

Our next step will be to determine what our options are for changing the implementation.

@dbeatty10 dbeatty10 removed the triage label Nov 14, 2022
@dbeatty10
Copy link
Contributor

Goal

  • Only set the node.supported_languages configuration for materialization nodes
  • Determine the node type independently of the name
    • so that non-materialization nodes may have "materialization" in their name without failing during compilation

Idea for resolution

@stu-k I saw you worked on this area of the code -- do you know if we could do something like the following? If not, do you have any other suggestions?

Replace:

# get supported_languages for materialization macro
if "materialization" in name:
node.supported_languages = jinja.get_supported_languages(macro)
yield node

with something like:

 # get supported_languages for materialization macro 
 if block.block_type_name == "materialization": 
     node.supported_languages = jinja.get_supported_languages(macro) 
 yield node 

@jtcohen6
Copy link
Contributor

jtcohen6 commented Nov 14, 2022

@dbeatty10 Your suggested change makes sense to me! Much better than the idea I was halfway through typing.

I'm going to tag this one for Team:Language, since that's whose surface area this falls under. Stu was original implementer here, so still very happy to hear from him too.

@github-actions
Copy link
Contributor

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label May 14, 2023
@github-actions
Copy link
Contributor

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest. Just add a comment to notify the maintainers.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 21, 2023
@mjedrzejczak-wh
Copy link

please reopen this ticket, now we have version 1.5.2 and it is still not fixed :(

@jtcohen6 jtcohen6 removed the stale Issues that have gone stale label Jun 30, 2023
@jtcohen6 jtcohen6 reopened this Jun 30, 2023
@jtcohen6
Copy link
Contributor

Given how straightforward Doug's proposed fix is, I think this could be a community contribution.

@mjedrzejczak-wh Any interest in giving it a spin?

@mjedrzejczak-wh
Copy link

@jtcohen6 unfortunately we have in our projects such macros, which block us effectively from upgrade

and I would prefer not to fix this myself - I do not know dbt internals so well

@ArunT317
Copy link

ArunT317 commented Jul 6, 2023

@jtcohen6 @mjedrzejczak-wh @dbeatty10 @CraftyIvan @joevandyk

Just FYI, I too faced the same error while trying to upgrade dbt.

I did try the code change suggested by @dbeatty10 locally in my python virtual environment and it is working as expected. I am facing some other project specific errors but thought to share this here so that we are all aware that the idea proposed above is fixing the issue.

@fconrady
Copy link

fconrady commented Sep 11, 2023

I need to make materializations depend on a variable and this bug prevents me from giving the macro a correct name. Besides, the error is very confusing.

@mjedrzejczak-wh
Copy link

@fconrady it is already fixed and closed, update your dbt :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants