Feature: include unrendered configs

[beckjake]

resolves #2713

Description

Store the un-rendered form of configs on models, and use that as the basis for comparison.

This won't fix {{ config(schema=target.schema) }}, but it should fix this:

models:
  my_project:
    schema: "{{ target.schema }}"

Checklist

• 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 updated the CHANGELOG.md and added information about my change to the "dbt next" section.

[beckjake]

This logic used to live in the Project itself, but it's really part of rendering. This also avoids us having to make a second pass of deep_map through the config.

[jtcohen6]

This is wonderful. To me the biggest feat is in handling behavior like:

+materialized: "{{ 'table' if target.name == 'prod' else 'view' }}"

For a project that has a top-level, environment-aware config applied on every model (e.g. transient, post-hook), this change could bring the false positive count down from every model to zero.

Regarding model database representations in particular, I think we should still norm around folks using generate_schema_name and generate_database_name in lieu of arbitrary Jinja conditionals in dbt_project.yml... but it's cool to know it works!

Out of curiosity:

• What's the reason why this doesn't work for in-model {{config()}} calls? Just want to make sure I understand the critical difference
• How unrelated or extensible is the work here for capturing and comparing unrendered (re)source properties, e.g.

sources:
  - name: my_postgres_db
    schema: "{{ 'postgres_prod' if target.name == 'prod' else 'postgres_dev' }}"

[jtcohen6]

TIL this exists

[jtcohen6]

We released dbt-utils v0.6.0 at the end of last week, which is compatible with dbt v0.18.0 (i.e. uses adapter.dispatch). I'm not sure if you want to use that version instead throughout.

[beckjake]

Unfortunately, because dbt-utils is now using sensible version bounds, upgrading to that is just going to break integration tests as soon as we rev dev/kiyoshi-kuromiya to 0.19.0a1 (which I should probably do).

I think in the long run we're just going to want to make another package that's like dbt-integration-project, but depends upon dbt-integration-project. Tests shouldn't care about "real" packages.

[beckjake]

What's the reason why this doesn't work for in-model {{config()}} calls? Just want to make sure I understand the critical difference

For in-model config() calls, there's never a point where we know both what's been passed to config() and what it looks like un-rendered, because the config() call happens in the same jinja rendering phase as its arguments are rendered. So it's not feasible to know both:

• this is a config() call
• these are what its arguments look like before rendering

unless you're getting that information from "inside" the rendering phase - #2714 is how I'd hope to solve that, though even there it's slightly different - instead of comparing the textual representation, we'd probably have to compare some sort of AST-level representation. I think it'd be similar from a user perspective, if we got it right.

How unrelated or extensible is the work here for capturing and comparing unrendered (re)source properties

I think we could follow the same basic pattern of stashing the un-rendered source information on the source object and looking at that instead of the existing source object for comparisons.

[jtcohen6]

Thanks for the explainer on config() calls.

I think we could follow the same basic pattern of stashing the un-rendered source information on the source object and looking at that instead of the existing source object for comparisons.

Nice! This has been one of my bigger concerns about state:modified as it exists today. I may open an issue for this, perhaps for a future sprint when we can dive deeper into AST land, too.