Enact deprecations

[jtcohen6]

Describe the feature

• As of dbt v0.18.0, we collect anonymous usage stats for deprecation warnings.
• We were already planning to end support for config-version: 1 of dbt_project.yml in dbt v0.19.0 (per implementation of Feature: include unrendered configs #2735)
• We should officially deprecate:
  • dbt-project-yaml-1: this is far & away the most common deprecation warning, and we still see it popping up in ~40% of projects running v0.18.0. Users may have upgraded their own projects but still need to upgrade installed packages. In any case, this is definitely the most controversial of our proposed deprecations.
  • column-quoting-unset: 11% of projects
  • models-key-mismatch: 2% of projects
  • materialization-return: <1% of projects
  • not-a-dictionary: 0% of projects
• We should not deprecate:
  • adapter-macro: this is the second-most-common deprecation warning, and we only introduced it in v0.18 with the switch to adapter.dispatch. We should give users at least one more minor version to upgrade.

Describe alternatives you've considered

• Leaving these as warnings until v1.0.0, or forever

Who will this benefit?

• Us as dbt maintainers in the short term, as we can reduce the surface area of code that patched older syntaxes
• Users, in the long run, as we continue to crystallize the UX of dbt project development

[jtcohen6]

Note: dbt v0.19.0 did deprecate dbt-project-yaml-1, in exchange for improved state:modified behavior around env-based configs (#2735).

It's one of the few times that we've entirely removed a piece of functionality—definitions of vars that are different for different model subdirectories—without replacing it, or rearchitecting it, in a way that achieved functional parity. The original behavior was an accident of the initial vars implementation, and the current implementation (project-level vars) is much easier to manage and rationalize. Despite not being a feature we had really documented or encouraged, folks did leverage it for certain use cases, and now they can't.

This is something we'll want to keep thinking about, and develop rigorous communication plans, ahead of releasing v1.0.

[jtcohen6]

There's at least one new deprecation since we opened this ticket:

• dispatch-packages

We should establish a consistent habit around deprecations, with a few options:

• never removing functionality / always maintaining backwards compatibility
• removing deprecated functionality only in major versions

[leahwicz]

@jtcohen6 would the goal be to add these deprecation warnings to 0.21.0 and then actually remove the functionality in 1.0?

Also is this the official list? I want to track what we need to do for sure. I might try picking some of these up or having Emily pick this up...

• dbt-project-yaml-1
• column-quoting-unset
• models-key-mismatch
• materialization-return
• not-a-dictionary
• dispatch-packages

[jtcohen6]

@leahwicz Here's where we define the list of deprecation warnings that exist currently (in develop):
https://github.com/dbt-labs/dbt/blob/9d7a6556efb9bdbbfedf24b6f01ded90595a572d/core/dbt/deprecations.py#L171-L179

So dbt-project-yaml-1 we've already deprecated. Ahead of v1.0, we want to:

• Actually deprecate the functionality by doing the thing the deprecation warning promises, likely switching a default or removing backwards compatibility for the old setting. Where appropriate, we could consider raising a clear exception (not warning) if someone is still using the now-deprecated mechanism.
• Remove the deprecation warning from the set of "active deprecations" in deprecations.py.

There are some deprecations we'll want to keep around, e.g. Anna's work over in #3825

[jtcohen6]

This issue can really be split up into a bunch of smaller tasks, each one targeting one of the active deprecation warnings that we currently have in place. I can do my best to provide pointers for each. I'll include a few below, and add more when I have more time.

dispatch-packages

Introduced in: v0.19.2
In the last week, raised by: 276 projects (~9% of eligible projects)

Instead of raising a deprecation warning when the user has defined the packages arg, or has passed a list instead of a string, we should instead raise a clear error. That error message should point to the same documentation that the deprecation warning message currently does

https://github.com/dbt-labs/dbt/blob/96083dcaf51ab7725f1b554891581e1522e32d7d/core/dbt/context/providers.py#L137-L140

Then, we can:

• Remove DispatchPackagesDeprecation from the list of active deprecations
• Significantly simplify the logic (removing jank backwards compatibility) from jinja_static.py — to discuss with @gshank

materialization-return

Introduced in: v0.15
In the last week, raised by: 14 projects (<1%)

https://github.com/dbt-labs/dbt/blob/96083dcaf51ab7725f1b554891581e1522e32d7d/core/dbt/task/run.py#L208-L225

• Remove the conditional branch if isinstance(result, str) that raises a deprecation warning and handles the string result. If result isn't a dictionary that passes _validate_materialization_relations_dict, we should just raise an exception.
• Remove MaterializationReturnDeprecation from the list of active deprecations

not-a-dictionary

Introduced in: v0.15
In the last week, raised by: 17 projects (<1%)

This is a quite specific one, having to do with the fact that some internal dbt objects used to be simple dictionaries and are now dataclasses. I think we should just raise exceptions on the dunder methods of __iter__ and __len__ for all superclasses of FakeAPIObject (BaseRelation is probably the most prominent).

https://github.com/dbt-labs/dbt/blob/95cca277c90fb1c3139e1ae4990a29d8038afcd0/core/dbt/contracts/relation.py#L45-L52

column-quoting-unset

Introduced in: v0.15
In the last week, raised by: 619 projects (~9%)

We should do as the deprecation warning says, and switch the default from False to True. That's it in a nutshell.

I think we'll want to override/reimplement the quote_seed_column method on the dbt-snowflake adapter only, and set the default there to False in dbt-snowflake, since quoting is such a pain on Snowflake. There's precedent for doing that elsewhere: see quote policy for relations, and docs on project-level quoting config.

https://github.com/dbt-labs/dbt/blob/95cca277c90fb1c3139e1ae4990a29d8038afcd0/core/dbt/adapters/base/impl.py#L810-L815

models-key-mismatch

Introduced in: v0.16
In the last week, raised by: 34 projects (<1%)

This is a tricky one: We actually removed this deprecation warning in v0.20, when we removed the patch_nodes method. But the to-be-deprecated functionality is still supported.

https://github.com/dbt-labs/dbt/blob/745ae3e64ac03fedb19228b31252fea05eb0ed37/core/dbt/contracts/graph/manifest.py#L685-L691

I'd like to re-add the conditional check for whether a non-model (a seed, snapshot, etc) is specified under the models: key, and start raising a clear exception for this in v1.0. This may require some work with @gshank to find where this logic has moved in v0.20+.

Once done, we can also remove this from the list of active deprecations, since it's no longer referenced anywhere in the code!

execute-macro-release

Introduced in: v0.17.2
In the last week, raised by: 0 projects

Pretty straightforward: as the deprecation warning message states, we should remove the release arg from the execute_macro method. I don't think we need to worry too much about moving text_only_columns up one positional argument, but it may not be a bad idea to leave a placeholder in its spot.

https://github.com/dbt-labs/dbt/blob/484517416f3ec7affb22f3f1be4f19b2ef825332/core/dbt/adapters/base/impl.py#L961-L964

Then, we can remove it from the list of active deprecations.

adapter-macro

Introduced in: v0.18
In the last week, raised by: 203 projects (~3%)

Pretty simple: we deprecated the adapter_macro context member in favor of adapter.dispatch, and the former currently serves as a passthrough to the latter:

https://github.com/dbt-labs/dbt/blob/484517416f3ec7affb22f3f1be4f19b2ef825332/core/dbt/context/providers.py#L1152-L1212

Let's keep adapter_macro, but change it to immediately raise an exception that encourages users to switch it out for adapter.dispatch instead (perhaps with a link to https://docs.getdbt.com/reference/dbt-jinja-functions/dispatch). Then, we can remove adapter-macro from the list of active deprecations.

package-redirect

No action needed! Let's keep this one around for the foreseeable future. This is a warning about a package that's been deprecated, rather than a core interface that we're planning to change.

[emmyoop]

Opened #415 in dbt-utils because of newly introduced use of packages. Will need to be resolved before enacting deprecation for dispatch-packages.

[emmyoop]

All deprecations listed above have been merged into develop. Closing.

[jtcohen6]

woohoo !!