Warn for dbt snapshot configurations containing target_schema or target_database

[dbeatty10]

Describe the feature

Warn whenever the target_schema or target_database is configured for a dbt snapshot.

This is because the best practice in dbt v1.9+ for dbt snapshots is to use schema and database configs rather than target_schema or target_database. See "Additional context" below for more details.

Describe alternatives you've considered

Leave things as-is with no warning.

Who will this benefit?

This will benefit folks that want to use the best practices as it relates to configuring dbt snapshots.

Additional context

The feature for schema and database configs on snapshots is in dbt-labs/dbt-core#10301, and this will be released as part of v1.9.

dbt-labs/docs.getdbt.com#5805 describes the following changes this introduces for dbt snapshots:

• target_schema is now optional for snapshots
• if you do not set target_schema, your snapshot will respect generate_schema_name macro
• if you do not set target_database, your snapshot will respect generate_schema_name macro
• old behavior continues to work for backwards compatibility: so you can still set target_schema and/or target_database and that will be where your snapshot is built, but these are "old" configs
• new configs for snapshots: you can now set database, schema, alias for your snapshots to be used by the generate_x_name macros

[b-per]

Is it really the remit of this package?

I feel like it should come as a warning from dbt Core directly, no?

I view this package more as a modelling best practice packages, not so much of a configuration best practice one.

[dbeatty10]

If it doesn't go in either Core or dbt-project-evaluator, is there any other place you think it could go?

When @Grace, Gerda, Michelle and myself discussed this, we decided not to raise a warning in dbt Core. We were hoping/thinking that project evaluator might be a good mechanism for this particular best practice (read below for more details).

More details

If we were going to deprecate target_schema and target_database, then we'd just raise a deprecation warning in dbt Core. But we're planning to continue to support these indefinitely, which would make them an optional "best practice" instead.

dbt Core has generally not raised warnings for best practices, and we've typically reserved WarnLevel warnings within dbt Core for things like:

• deprecations (ex. PackageMaterializationOverrideDeprecation)
• behavior changes (ex. SourceFreshnessProjectHooksNotRun)
• invalid values (ex. InvalidValueForField)
• configuration that won't have any affect and will be ignored (ex. UnusedResourceConfigPath)
• configuration with potentially surprising consequences (ex. DepsUnpinned)

[b-per]

I definitely don't want to have to define new rules every time a field is not a best practice anymore but could maybe buy creating one single rule where we would bundle all the config that used to be standard and is not recommended anymore.

But one of the problems with this approach is that I guess that most dbt users don't install this package in their project.

Alternatives I am thinking of (which also could be put in addition of providing the feature in this package):

• should we surface this in dbt Cloud somewhere? the same way that dbt Explorer shows recommendations
• should we create a new subcommand in dbt-core, dbt best-practice or dbt recommendation that would list such changes (and maybe the recommendation to move from test to data_test as well. The advantage is that every use of dbt would have access to this, without installing a third party package

[dbeatty10]

should we surface this in dbt Cloud somewhere? the same way that dbt Explorer shows recommendations

Good idea!

should we create a new subcommand in dbt-core

There's a really high threshold to add new subcommands to dbt-core, so we probably wouldn't for this. Instead, we're looking for linting of best practices to go into third party tooling.

[dave-connors-3]

I think this falls into the same category as the data_tests issue! we are not likely to pursue this warning in this package