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

Store the un-resolved form of config attributes in node configs #2713

Closed
beckjake opened this issue Aug 18, 2020 · 1 comment
Closed

Store the un-resolved form of config attributes in node configs #2713

beckjake opened this issue Aug 18, 2020 · 1 comment
Labels
enhancement New feature or request state Stateful selection (state:modified, defer)

Comments

@beckjake
Copy link
Contributor

Describe the feature

Currently, dbt resolves all node configurations before putting them into a node's config. For example, if you have this dbt_project.yml:

vars:
  normal_materialization: 'view'
  folder_materialization: 'table'

models:
  my_project:
    materialized: "{{ var('normal_materialization') }}"
   folder:
       materialized: "{{ var('folder_materialization') }}"

Then when you run with dbt run --var {normal_materialization the manifest is serialized, the config of a node in folder/will look have{"materialized": "table"}`. This is nice, but it'd be better if:

  • the model's behavior at runtime was the same as it currently is
  • the config's representation in the manifest was {"materialized": "{{ var('folder_materialization') }}"}

This is complicated in the in-model config case:

{{ config(materialized=var('folder_materialization')) }}

A future issue should address this problem, but for this issue it'll be sufficient to only store the resolved form here.

This will help dbt do a better job on state.modified checks to decide of a node has been modified. Currently we have a false-positive issue where changing a var like this will result in dbt detecting that the node is "modified", which isn't really true.

This will definitely be a breaking change to some degree! We're going to have to figure out a number of interesting edge cases:

  • what should dbt ls show? Should it be a flag to say "don't resolve config values"?
  • the config.xxx selector probably needs to resolve xxx!
  • Should we include var values in the manifest as part of this?

Describe alternatives you've considered

We can stick with the current behavior.
We can make the un-resolved form a new field and include both in the manifest.

Who will this benefit?

Users of state:modified selector.

@beckjake beckjake added enhancement New feature or request triage labels Aug 18, 2020
@jtcohen6 jtcohen6 removed the triage label Aug 18, 2020
@jtcohen6 jtcohen6 added this to the 0.19.0 milestone Aug 18, 2020
@jtcohen6
Copy link
Contributor

jtcohen6 commented Sep 9, 2020

Resolved by #2735

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request state Stateful selection (state:modified, defer)
Projects
None yet
Development

No branches or pull requests

2 participants