[Feature] Add a behaviour change flag to disallow the legacy target_* configs on snapshots

[joellabes]

Is this your first time submitting a feature request?

• I have read the expectations for open source contributors
• I have searched the existing issues, and I could not find an existing issue for this feature
• I am requesting a straightforward extension of existing dbt functionality, rather than a Big Idea better suited to a discussion

Describe the feature

I was just trying to create a YAML snapshot for the first time, and I accidentally went with the old target_schema config:

snapshots:
  - name: a_slightly_bad_snapshot
    relation: ref('my_excellent_model')
    config:
      target_schema: snapshots

      strategy: timestamp
      updated_at: modified_at
      unique_key: unique_key

Which meant that when I ran dbt snapshot, my snapshot was built into snapshots instead of dbt_jlabes_snapshots as I expected.

To fix it, I had to use schema instead:

snapshots:
  - name: my_excellent_snapshot
    relation: ref('my_excellent_model')
    config:
      schema: snapshots 

      strategy: timestamp
      updated_at: modified_at
      unique_key: unique_key

I think we should let people block usage of the old target_schema and target_database configs (in YAML-based snapshots at minimum - this isn't available in the classic jinja ones right?) with a behaviour flag.

Describe alternatives you've considered

No response

Who will this benefit?

• People using the autocomplete from the JSON Schema, who pick a sensible-looking option
• Admins at large orgs who want to enforce consistent behaviour across all their snapshots without only picking it up in code review
• Fans of environmental separation

Are you interested in contributing this feature?

👎

Anything else?

No response

[dbeatty10]

An alternative:
What if target_schema / target_database were just treated as aliases of schema / database?

This would have the double advantage of building in the place you intended without you needing to know to enable a specific behavior flag or remember that there's a new name.

[graciegoheen]

@dbeatty10 I think we need to maintain target_schema and target_database for backwards compatibility - if we want to get rid of them, we should go the behavior change flag route

is there a desire to "sunset" (via a behavior change flag) the old jinja snapshot spec? or you're thinking narrowly sunset "target_schema" / "target_database"? i believe this applies to both the new snapshot YML spec and the old jinja-based one

[dbeatty10]

I think we need to maintain target_schema and target_database for backwards compatibility

@graciegoheen that aligns with what I'm saying: treat target_schema / target_database as valid options within YAML snapshots. We'd only allow configuring either target_schema or schema, but treat them both the same.

I think this would have the advantage of preserving backwards-compatibility without needing the extra effort to "sunset" via a behavior change flag.

What do you think?

[graciegoheen]

I'm saying we need to maintain the legacy behavior of target_schema as well - if suddenly target_schema started behaving the same way as schema, that would be a breaking change

[joellabes]

is there a desire to "sunset" (via a behavior change flag) the old jinja snapshot spec?

I was not pitching that, no!

or you're thinking narrowly sunset "target_schema" / "target_database"?

This was what I meant, and even then I was only meaning in the context of yaml-based snapshots. I was going to leave the jinja ones completely alone

i believe this applies to both the new snapshot YML spec and the old jinja-based one

As far as I can tell, the new schema and database configs aren't available on Jinja-based snapshots, right? So preventing people from defining target_schema in Jinja would be a problem. But if the new configs are available on the old snapshots, then I would be completely on board with blocking the old configs' use everywhere.