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

Clean up project config rename deprecation warnings #4291

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Nov 17, 2021

resolves #4276

  • Create different deprecation warnings for each of source-paths and data-paths being renamed, so that both will display for users newly upgraded to v1
  • Add a null handler to the global FILE_LOG when it's first instantiated, to prevent the un-configured FILE_LOG and STDOUT_LOG from both writing to stdout. This shouldn't have any functional impact, since the file logger isn't meant to be doing anything until it's configured, anyway

Also:

  • Add \n\n to deprecation warning. ui.line_wrap_message() doesn't work perfectly with the new logging format just yet (it doesn't account for space taken up by timestamp + log level), so I'd prefer a cleaner break.
  • In ui.line_wrap_message(), prevent textwrap.fill() from breaking on hyphens. This is awkward for PackageRedirectDeprecation, which includes hyphen-bearing package names.

Before:

$ dbt test
07:16:14 | [ info  ] | Running with dbt=1.0.0-rc1
07:16:14 | [ warn  ] | * Deprecation Warning: The `source-paths` config has been deprecated in favor of
`model-paths`. Please update your `dbt_project.yml` configuration to reflect
this change.
07:16:14 | [ warn  ] | * Deprecation Warning: The `source-paths` config has been deprecated in favor of
`model-paths`. Please update your `dbt_project.yml` configuration to reflect
this change.

After:

$ dbt test
06:50:56 | [ info  ] | Running with dbt=1.0.0-rc1
06:50:56 | [ warn  ] | * Deprecation Warning:
The `source-paths` config has been deprecated in favor of `model-paths`. Please
update your `dbt_project.yml` configuration to reflect this change.
06:50:56 | [ warn  ] | * Deprecation Warning:
The `data-paths` config has been deprecated in favor of `seed-paths`. Please
update your `dbt_project.yml` configuration to reflect this change.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change

@cla-bot cla-bot bot added the cla:yes label Nov 17, 2021
@jtcohen6 jtcohen6 force-pushed the fix/config-path-deprecation branch from f5e3c68 to 8db784a Compare November 17, 2021 06:19
@jtcohen6 jtcohen6 requested a review from emmyoop November 17, 2021 07:00
@jtcohen6 jtcohen6 marked this pull request as ready for review November 17, 2021 07:00
Copy link
Contributor

@nathaniel-may nathaniel-may left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this when testing on one of my old projects and I found it annoying. Nice fix!

I do think a lot of this logic should actually live in the new events module, but I'm ok not doing that refactor right now. The bottom of the deprecation warnings looks like it drops out into events here, but I think most of the rest of this logic could live in there too. I've added it to the stretch goals list for phase 2. We can make a ticket for it if we don't get to it.

@jtcohen6
Copy link
Contributor Author

Agreed - we treat deprecations sorta like exceptions, and we should eventually move all the deprecation / exception / warn_error logic into events. That can be TK

@jtcohen6 jtcohen6 merged commit c55be16 into main Nov 17, 2021
@jtcohen6 jtcohen6 deleted the fix/config-path-deprecation branch November 17, 2021 17:01
iknox-fa pushed a commit that referenced this pull request Feb 8, 2022
automatic commit by git-black, original commits:
  c55be16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor logging for PackageRedirectDeprecation
2 participants