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

generate_schema_name compilation behavior has changed #1385

Closed
clrcrl opened this issue Apr 3, 2019 · 4 comments
Closed

generate_schema_name compilation behavior has changed #1385

clrcrl opened this issue Apr 3, 2019 · 4 comments

Comments

@clrcrl
Copy link
Contributor

clrcrl commented Apr 3, 2019

Issue

Issue description

I actually don't know what has changed here, just that something has.
This macro used to throw an error if a model or seed didn't have a custom schema (the desired behavior), but as of 0.12.2 it doesn't throw the error.

Steps to reproduce:

  1. Add this to your project:
{% macro generate_schema_name(custom_schema_name) -%}
    {%- if custom_schema_name == None and model.resource_type in ['seed', 'model'] -%}
     {%- set error_message -%}
       {{ model.resource_type | capitalize }} '{{ model.unique_id }}' does not have a schema configured.
     {%- endset -%}
    {{ exceptions.raise_compiler_error(error_message) }}
    {%- endif -%}

    {%- set default_schema = target.schema -%}
    {%- if target.name == 'prod' and custom_schema_name is not none -%}
        {{ custom_schema_name | trim }}
    {%- else -%}
        {{ default_schema }}_{{ custom_schema_name | trim }}
    {%- endif -%}
{%- endmacro %}

  1. Don't configure a custom schema in your dbt_project.yml file.

3.dbt compile

Expected results

On 0.12.1 this would return:

(dbt0.12.1) $ dbt compile
Encountered an error:
Compilation Error
  Model 'model.jaffle_shop.stg_customers' does not have a schema configured.

  > in macro generate_schema_name (macros/get_custom_schema.sql)
  > called by <Unknown>

Actual results

From 0.12.2 onwards this returns

(dbt0.12.2) $ dbt compile
Found 8 models, 20 tests, 0 archives, 0 analyses, 96 macros, 2 operations, 0 seed files

15:25:51 | Concurrency: 1 threads (target='dev')
15:25:51 |
15:25:52 | Done.

Notes

I'm very happy for you to slap a #wontfix on this, since when I wrote this code I was hacking at the edges of dbt. But if there's a better way to do this, that would also be good to know!

@beckjake
Copy link
Contributor

beckjake commented Apr 4, 2019

This was not what I expected it to be! It's another generate_schema_name macro wart, with a twist. As of 0.12.2, In generate_schema_name the model context object is always the macro object itself. So the check that's not passing anymore is model.resource_type in ['seed', 'model'] (because model.resource_type is 'macro').

Maybe generate_schema_name could take a model argument? That's easy for us to do but probably user unfriendly, and a bad compatibility break.

Or we could stash the context and defer the call to generator that's currently here until use-time, though that probably hurts performance since we have to call .generator() once per model that way.

Or maybe there's some way to override the environment after calling generator but before the actual macro call.

At a minimum, it does seem to me like we should probably not have model in the context for generate_schema_name if it's going to be this wrong!

@clrcrl
Copy link
Contributor Author

clrcrl commented Apr 4, 2019

Hm so the reason that the resource_type condition lives there is because otherwise it raises a compilation error since macros can't have schemas configured. I think this is a reasonable thing to want to do, but I don't know another way around it.

@drewbanin
Copy link
Contributor

In 0.14.0, the generate_schema_name macro will accept a node argument (docs to come here). You can use this argument to generate a custom schema (or implement other logic) depending on attributes of the node that is being configured.

via: #1483

@clrcrl
Copy link
Contributor Author

clrcrl commented Jun 19, 2019

For posterity, the code that works is:

{% macro generate_schema_name(custom_schema_name, node) -%}
    {%- if custom_schema_name == None and node.resource_type in ['seed', 'model'] -%}
     {%- set error_message -%}
       {{ node.resource_type | capitalize }} '{{ node.unique_id }}' does not have a schema configured.
     {%- endset -%}
    {{ exceptions.raise_compiler_error(error_message) }}
    {%- endif -%}

    {%- set default_schema = target.schema -%}
    {%- if target.name == 'prod' and custom_schema_name is not none -%}
        {{ custom_schema_name | trim }}
    {%- else -%}
        {{ default_schema }}_{{ custom_schema_name | trim }}
    {%- endif -%}
{%- endmacro %}


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants