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

Deprecate custom config keys (all non dbt controlled configs should live in meta within config) #11337

Open
Tracked by #11335
QMalcolm opened this issue Feb 26, 2025 · 0 comments

Comments

@QMalcolm
Copy link
Contributor

QMalcolm commented Feb 26, 2025

Problem

In the config of nodes (whether in SQL or YAML), custom config keys. Consider the following

# my_model.yaml

models:
  - name: my_model
    config:
      materialization: table
      my_custom_config: value
      ailas: alias_name
      

Believe it or not there are three issues with the above.

Firstly, it means anytime a new config key is defined for a node in dbt-core, it is potentially breaking change. To guard against this, we have to ensure any new config key added is typed as Any. If we don't type it as Any then if the config key exists in a project already, with a value that doesn't match the type. Having to type all new config values as Any ends up require a bunch of defensive code and behavior flags, making the development of new features more cumbersome.

This leads into the second problem in that people might add configs thinking it will do something when it does nothing. For instance, the full_refresh has a purpose on seeds and models, but not other nodes. Despite this, you can define it on other nodes. What does full_refresh mean on a snapshot or source? It doesn't do anything during the operation of those nodes, but it might be expected that it should, given that you can define it.

Finally, the third issue is that it means it's nearly impossible to detect typos. You may not have noticed in the example YAML above, but I misspelled the "alias" key as "ailas". When this happens, again we think we're doing something (aliasing our model), but that information doesn't get propagated.

Solution

Deprecate the custom config keys, and let people know to move these key/values under config.meta (config.meta will henceforth be the safe custom user space). We want to make custom config keys an error, and at some time it will become an error, but we can't move straight to it being an error. As it being an error would be breaking to projects everywhere, we need to first deprecate it to give people time to fix the problem. Custom config keys still are useful though, and we don't want to take them away in total

Acceptance criteria

  • Deprecation warning is fired if a custom config key is present
  • A deprecation warning is not fired if a custom config key is not present
  • In non-verbose mode a count of the number of occurences is included in the deprecation warning
  • In verbose mode all the instances are listed in the deprecation warning

Suggested Tests

  • Deprecation warning is fired if a custom config key is present
  • A deprecation warning is not fired if a custom config key is not present
  • In non-verbose mode a count of the number of occurences is included in the deprecation warning
  • In verbose mode all the instances are listed in the deprecation warning

Backport?

N/A

@QMalcolm QMalcolm changed the title Deprecate custom config values (all non dbt controlled configs should live in meta within config) Deprecate custom config keys (all non dbt controlled configs should live in meta within config) Mar 4, 2025
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

1 participant