-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fully remove legacy logger #9353
Conversation
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## add-dbt-common-requirement #9353 +/- ##
==============================================================
+ Coverage 86.73% 87.06% +0.33%
==============================================================
Files 179 178 -1
Lines 24245 23860 -385
==============================================================
- Hits 21028 20773 -255
+ Misses 3217 3087 -130
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay!
@@ -48,7 +42,7 @@ def error(self): | |||
|
|||
@dataclass | |||
@schema_version("remote-execution-result", 1) | |||
class RemoteExecutionResult(ExecutionResult, RemoteResult): | |||
class RemoteExecutionResult(ExecutionResult): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that the schema for these "remote result" objects has changed and needs to be bumped? Even if the schema has changed, it looks these objects are only ever used by dbt-rpc. If that is indeed the case, can we remove them entirely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to push off this code cleanup to #9359
db12503
to
45e4a8a
Compare
resolves #8027
Problem
We have a new logging system but still have the old logger that depends on an abandoned package in code code.
Solution
Rip out logger.py
Rip out everywhere it was used
Rip out
pytest-logbook
fromdev-requirements.txt
Rip out logbook stub
Checklist