Adapter logging interface #4161
Conversation
nathaniel-may
changed the base branch from
feature/structured-logging
to
first-pass-structured-logging
nathaniel-may
force-pushed
the
adapter-logging-interface
branch
from
1d2ff5f to
98c692f
Compare
|
commit history looks a little wonky which is probably because I rebased onto a different branch than the one I branched from. I'm gonna squash and merge anyway so it should be fine. |
|
I should probably add the |
nathaniel-may
force-pushed
the
adapter-logging-interface
branch
from
cc76621 to
e35106d
Compare
|
re-requesting prior approvals because I made a significant change since then. |
| def exception( | ||
| self, | ||
| msg: str, | ||
| exc_info: Any = True, # this default is what makes this method different |
|
Should this object have some kind of doc string? What should it say? |
There was a problem hiding this comment.
The interface and explainer look great to me.
Could this PR also do the two-line work of switching over the Postgres adapter? It looks like there's one spot where it imports dbt.logger and should instead use AdapterLogger("Postgres")? That could be good as far as having a concrete example to point to when cutting over more adapters.
If you'd rather than come in a subsequent PR, that's also fine by me.
| To integrate existing log messages from adapters, you likely have a line of code like this in your adapter already: | ||
| ```python | ||
| from dbt.logger import GLOBAL_LOGGER as logger | ||
| ``` | ||
|
|
||
| Simply change it to these two lines with your adapter's database name, and all your existing call sites will now use the new system for v1.0: | ||
| ```python | ||
| from dbt.events import AdapterLogger | ||
| logger = AdapterLogger("<database name>") | ||
| # e.g. AdapterLogger("Snowflake") |
nathaniel-may
force-pushed
the
adapter-logging-interface
branch
from
6e7763a to
e726824
Compare
|
Logbook is causing issues when I moved the Postgres adapter over to the new interface. Instead of spinning my wheels investigating, I migrated all these changes into #4176 because Logbook isn't a problem over there. |
|
Tests passed over there so I'm verifying that it works over here for record. |
waiting for #4137 to merge first
Description
Adapter maintainers currently use our old logging system. This exposes the new eventing system to them so that they have to make minimal changes to stay up-to-date for v1. Adapter maintainers will not have to touch every log call site to use the new system.
Checklist
CHANGELOG.mdand added information about my change