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

Add an alias for source-paths configuration key #1607

Closed
clrcrl opened this issue Jul 12, 2019 · 4 comments · Fixed by #4008
Closed

Add an alias for source-paths configuration key #1607

clrcrl opened this issue Jul 12, 2019 · 4 comments · Fixed by #4008
Assignees
Labels
1.0.0 Issues related to the 1.0.0 release of dbt

Comments

@clrcrl
Copy link
Contributor

clrcrl commented Jul 12, 2019

This is a pretty confusing key now that we have sources!

source-paths: ["models"]

Can we add an alias, model-paths for this key? And then I'll update the starter project accordingly.

While we are here, would also love if we could give the rest of these more sensible aliases, e.g.:

  • data-paths --> seed-paths
  • test-paths --> something that doesn't make me think this is where I define my unique and not_null tests (let me think on the best alias for this
@clrcrl clrcrl changed the title Update source-paths configuration key Add an alias for source-paths configuration key Jul 12, 2019
@jtcohen6 jtcohen6 added the 1.0.0 Issues related to the 1.0.0 release of dbt label Jan 4, 2021
@jtcohen6 jtcohen6 mentioned this issue Jun 16, 2021
11 tasks
@leahwicz
Copy link
Contributor

leahwicz commented Jul 8, 2021

@jtcohen6 lets move all these requests to #2659 and close this issue out. It will be easier to get these all done at the same time

Nvm the other ticket focuses more on default values and this ticket focuses on the keys. Let's schedule the work together though

@leahwicz leahwicz closed this as completed Jul 8, 2021
@leahwicz leahwicz reopened this Jul 8, 2021
@jtcohen6
Copy link
Contributor

jtcohen6 commented Jul 9, 2021

Let's schedule the work together though

Agreed! I wouldn't mind consolidating them into one issue, whatever feels most straightforward for implementation.

@emmyoop emmyoop self-assigned this Oct 5, 2021
@emmyoop
Copy link
Member

emmyoop commented Oct 5, 2021

@jtcohen6 I have a few questions on this

First, I updated source-paths to be model-paths which makes so much sense I love it. I think we also need to add a clear exception on what's going on. Current code catches it but it isn't super clear. The error you get if you are using source-paths is:

Runtime Error
  at path []: Additional properties are not allowed ('source-paths' was unexpected)

I was thinking something like this (but perfectly happy with you rewording it) which makes it more clear something changed and users need to take action.

Project loading failed for the following reason:
Runtime Error
  The `source-paths` alias has been deprecated in favor of `models-paths`. Please update `dbt_project.yml` to reflect this change.

Next - should I do the same with data-paths and test-paths as suggested in the original task?

Finally - I am now second guessing if I interpreted this task correctly. Can you take a look at the draft PR (tests are failing for known reasons)?

@emmyoop emmyoop mentioned this issue Oct 5, 2021
4 tasks
@jtcohen6
Copy link
Contributor

jtcohen6 commented Oct 6, 2021

@emmyoop Nice! I think you're right on with what this one is after. Thanks for opening the PR, I'm totally able to see what you're saying above.

IMO we should:

  • Keep supporting source-paths as a valid dbt_project.yml config, and pass its contents to the new model-paths config—basically, let's avoid breaking every project. But, we should raise a deprecation warning encouraging users to rename the config to model-paths, and raise an error if both source-paths and model-paths are defined. In a future version, based on the continued prevalence of the deprecation warning, we can remove support for source-paths entirely.
  • Rename data-paths to seed-paths, following the exact same procedure to avoid breaking projects. Also, let's switch the default value from data to seeds, both within config/project.py and in the init starter project

Let's keep test-paths just the way it is (beyond switching the default from test to tests in #4011). Instead of renaming this path, I'd like to see us do #3250: support custom generic test definitions inside test-paths (though retain support for them in macros/ for backwards compatibility). It's been a little while since we took a look at that one, and @gshank could advise as to how tricky it might be—conceptually, at least, it feels like a natural next issue to tackle after this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0.0 Issues related to the 1.0.0 release of dbt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants