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 top level properties in YAML files #11338

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

Deprecate custom top level properties in YAML files #11338

QMalcolm opened this issue Feb 26, 2025 · 0 comments

Comments

@QMalcolm
Copy link
Contributor

QMalcolm commented Feb 26, 2025

Problem

Currently, we allow for custom top level properties in YAML files, custom config keys. Consider the following

# properties.yml

models:
  - name: my model
    ...

soucres:
  - name: my_source
    ...

cats:
  - name: Omellete
     color: orange
  - name: Misu
     color: brown

There are two primary issues with the above.

Firstly, it means anytime a new top level property is defined, it is potentially breaking change. This doesn't happen often, but it's still bad. I don't think we're going to add a cats node type, but hey you never know

The bigger problem is that it makes it nearly impossible to detect typos. You may not have noticed in the example YAML above, but I misspelled the "sources" key as "soucres". If you are using the refing the source somewhere you'd likely get an error later on, which is good. However, if you misspell something like models which you use to add data tests, then suddenly your data tests aren't being run (not good).

Solution

Deprecate the custom top level YAML properties. We want to make custom top level YAML properties 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.

Acceptance criteria

  • Deprecation warning is fired if a custom top level YAML property is present
  • A deprecation warning is not fired if a custom top level YAML property 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 top level YAML property is present
  • A deprecation warning is not fired if a custom top level YAML property 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

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