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

[Bug] Calling a macro in a pre- or post-hook #7128

Closed
2 tasks done
seub opened this issue Mar 6, 2023 · 16 comments · Fixed by #10603
Closed
2 tasks done

[Bug] Calling a macro in a pre- or post-hook #7128

seub opened this issue Mar 6, 2023 · 16 comments · Fixed by #10603
Labels
bug Something isn't working help_wanted Trickier changes, with a clear starting point, good for previous/experienced contributors Medium Severity bug with minor impact that does not have resolution timeframe requirement paper_cut A small change that impacts lots of users in their day-to-day

Comments

@seub
Copy link
Contributor

seub commented Mar 6, 2023

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

There's 3 ways to define a pre-hook for a model:

  1. In dbt_project.yml
  2. In a property file (properties.yml)
  3. In a config block (my_model.sql)

Calling a macro in a pre-hook works for 1. and 3. but not 2:

# properties.yml
version: 2
models:
  - name: my_model
    config:
      pre-hook: "{{ my_macro() }}"

produces a compilation error

Could not render {{ my_macro() }}: 'my_macro' is undefined

Expected Behavior

The behavior should be consistent across 1. 2. 3. Ideally, calling a macro for a pre-hook should work in property files too.

Steps To Reproduce

See Current Behavior

Relevant log output

No response

Environment

- OS:
- Python:
- dbt: 1.4.3

Which database adapter are you using with dbt?

snowflake

Additional Context

https://getdbt.slack.com/archives/C50NEBJGG/p1677527674055049

@seub seub added bug Something isn't working triage labels Mar 6, 2023
@github-actions github-actions bot changed the title [Bug] macros in pre-hook configs [CT-2274] [Bug] macros in pre-hook configs Mar 6, 2023
@seub seub changed the title [CT-2274] [Bug] macros in pre-hook configs [Bug] Calling a macro in a pre-hook Mar 6, 2023
@dbeatty10 dbeatty10 self-assigned this Mar 6, 2023
@dbeatty10
Copy link
Contributor

Thanks for noticing this scenario, instigating a discussion in Slack, and opening this issue @seub 🏆 !

I agree with you and @jtcohen6 that it should align with the expected behavior and work across all three scenarios.

@dbeatty10 dbeatty10 removed their assignment Mar 6, 2023
@dbeatty10 dbeatty10 removed the triage label Mar 6, 2023
@dbeatty10
Copy link
Contributor

Acceptance criteria

  1. A new test is added that verifies macros within pre- and post-hooks are late-rendered when they are specified within a model properties YAML configuration file. Use YAML similar to that described in the "Current Behavior" above.
  2. This test fails prior to implementing the feature.
  3. This test succeeds after implementing this feature for pre-hook, post-hook, pre_hook, and post_hook.
  4. Ensure that late-rendering tests exist for dbt_project.yml and model config blocks also.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2023

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 Sep 3, 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 Sep 10, 2023
@d-musambi
Copy link

This would be very beneficial for us as we maintain a multi-tenant architecture and would allow us to have dbt docs generate for our models that run across tenants

@siljamardla
Copy link

The issue is still relevant, still experiencing inconsistent behaviour.
We have just recently decided that, for clarity, we will keep all of our configurations in the configuration files so they would not be scattered between configuration files and model files. This issue is making it impossible to keep all the configurations in configuration files :(

@occulkot
Copy link

Ive just bump into this issue myself when we started using post-hook in our dbt project

@adrivn
Copy link

adrivn commented Dec 1, 2023

Same. I refactored all the individual schema.yml files along with the SQL models, removing the individual hooks, to unify them into a single properties.yml file, but seems like the pre-post-hooks aren't allowed to run. Is anyone looking into this inconsistency?

@dbeatty10 dbeatty10 removed the stale Issues that have gone stale label Jan 30, 2024
@dbeatty10 dbeatty10 reopened this Jan 30, 2024
@dbeatty10
Copy link
Contributor

Multiple people have reported the desire to put all configs within properties.yml YAML files (rather than having some configs within individual model files).

Resolving this issue will provide that capability. In the meantime, a workaround is provided below along with a reproducible example (reprex).

Reprex

This works when running dbt compile, dbt run, dbt build, etc:

models/my_model.sql

{{ config(
    pre_hook="{{ some_macro() }}"
) }}

select 1 as id

And so does this:

dbt_project.yml

name: my_project
profile: my_profile

models:
  my_project:
    +pre-hook: "{{ some_macro() }}"

But this doesn't:

models/_properties.yml

models:
  - name: my_model
    config:
      pre_hook: "{{ some_macro() }}"

And it gives this error message:

14:31:46  Encountered an error:
Compilation Error
  Could not render {{ some_macro() }}: 'some_macro' is undefined

Workaround

In the meantime, the workaround is to put hook configs within model files instead of properties.yml / schema.yml:

{{ config(
    pre_hook="{{ some_macro() }}"
) }}

...

@rbs392
Copy link

rbs392 commented Feb 9, 2024

@dbeatty10 What would you suggest for python models?

@dbeatty10
Copy link
Contributor

@rbs392 See below for something you could try out for dbt python models. I didn't try it out personally, so let me know if it works or not 🙏

Let's suppose you have a python model at the path models/subfolder_1/subfolder_2/my_python_model.sql. Then you could try this within dbt_project.yml:

name: my_project
profile: my_profile

models:
  my_project:
    subfolder_1:
      subfolder_2:
        my_python_model:
          +pre-hook: "{{ some_macro() }}"

@rbs392
Copy link

rbs392 commented Feb 9, 2024

you could try out for dbt python models. I didn't try it out personally, so let me know if it works or not 🙏

Yeah this seems to work 😄
Would be nice to get it working with schema.yml as well

@graciegoheen graciegoheen added the Medium Severity bug with minor impact that does not have resolution timeframe requirement label Feb 9, 2024
@dbeatty10 dbeatty10 added the paper_cut A small change that impacts lots of users in their day-to-day label Feb 9, 2024
@dbeatty10
Copy link
Contributor

We're not able to prioritize this ourselves right now, so labeling this as "help wanted".

@dbeatty10 dbeatty10 added the help_wanted Trickier changes, with a clear starting point, good for previous/experienced contributors label Feb 9, 2024
@toandm
Copy link

toandm commented Apr 22, 2024

This is also an issue when I try to apply macro to post-hook within seed property file. Since you cannot add config directly to seed file, the only workaround is to use dbt_project.yml. Would love to have this fixed so it would help trim down the already bloated dbt_project.yml file.

@dbeatty10 dbeatty10 changed the title [Bug] Calling a macro in a pre-hook [Bug] Calling a macro in a pre- or post-hook Apr 22, 2024
@dbeatty10
Copy link
Contributor

dbeatty10 commented Jul 21, 2024

I ran into this myself this week.

The solution would be late-rendering hooks that appear within properties.yml files (like #6435 added late-rendering for pre_hook and post_hook in dbt_project.yml).

The relevant code appears to be should_render_keypath / _is_norender_key within the SchemaYamlRenderer class.

The analogous code for dbt_project.yml is should_render_keypath within the DbtProjectYamlRenderer class.

@dbeatty10
Copy link
Contributor

To expand on the use-case of adding a post hook to a seed in a properties YAML file, here's an example:

seeds:
  - name: seed_name
    config:
      post_hook: "alter table {{ this }} alter column id set not null"
    columns:
      - name: id

But when running dbt seed, I got an error like this because {{ this }} wasn't late-rendered:

13:43:24    Database Error in seed seed_name (seeds/seed_name.csv)
  syntax error at or near "COLUMN"
  LINE 3:         alter table  alter column id set not null

If instead this worked, then this would be a useful alternative to add to #10551 for adding database constraints to a seed (or snapshot).

mirnawong1 added a commit to dbt-labs/docs.getdbt.com that referenced this issue Sep 5, 2024
[Preview: Calling a macro in a
hook](https://docs-getdbt-com-git-dbeatty10-patch-2-dbt-labs.vercel.app/docs/build/hooks-operations#calling-a-macro-in-a-hook)

[Preview: pre-hook & post-hook

](https://docs-getdbt-com-git-dbeatty10-patch-2-dbt-labs.vercel.app/reference/resource-configs/pre-hook-post-hook)

## What are you changing in this pull request and why?

We have a couple concrete examples of calling a macro in a hook (like
[here](https://docs.getdbt.com/reference/project-configs/on-run-start-on-run-end#call-a-macro-to-grant-privileges)
and
[here](https://docs.getdbt.com/reference/resource-configs/pre-hook-post-hook#apache-spark-analyze-tables-after-creation)).
But we don't currently have any co-located canonical examples of the
different locations where hooks can be configured:
1. SQL model config
1. `properties.yml` \*
1. `dbt_project.yml`

\* Calling a macro in a hook within a properties YAML file was fixed in
v1.8+ by dbt-labs/dbt-core#10603.

## Other pages to review

Once dbt-labs/dbt-core#7128 is resolved, we
should also review these pages to ensure they include examples of
configs across `dbt_project` YAML files, properties YAML files, and
node-level config in SQL files:
- https://docs.getdbt.com/reference/resource-configs/pre-hook-post-hook
-
https://docs.getdbt.com/reference/model-configs#apply-configurations-to-one-model-only
-
https://docs.getdbt.com/reference/seed-configs#apply-the-schema-configuration-to-one-seed-only

## Additional context

Inspired by dbt-labs/dbt-core#10522

## Checklist
- [x] Review the [Content style
guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md)
so my content adheres to these guidelines.

---------

Co-authored-by: Mirna Wong <89008547+mirnawong1@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help_wanted Trickier changes, with a clear starting point, good for previous/experienced contributors Medium Severity bug with minor impact that does not have resolution timeframe requirement paper_cut A small change that impacts lots of users in their day-to-day
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants