add codes to log events #4268
add codes to log events #4268
Conversation
c123e61 to
1236df8
Compare
core/dbt/events/types.py
Outdated
| code: int | ||
| returncode: int | ||
|
|
||
| def message(self) -> str: | ||
| return f"command return code={self.code}" |
There was a problem hiding this comment.
Should use the renamed version - self.returncode
There was a problem hiding this comment.
yes! thank you.
| @@ -266,7 +266,7 @@ def message(self) -> str: | |||
|
|
|||
| @dataclass | |||
| class SystemReportReturnCode(DebugLevel, Cli, File): | |||
| code: int | |||
| returncode: int | |||
There was a problem hiding this comment.
Why is this an int instead of a string?
There was a problem hiding this comment.
I think I just guessed. I'll change it to a string. Mypy should tell us once we put all the concrete codes in place and satisfy those abstract method errors.
|
@emmyoop |
48f5a17 to
eff0f6a
Compare
ab25bfe to
c5ca29f
Compare
eff0f6a to
9d88756
Compare
|
A proposal for event code prefixes:
The basic idea is that the event code prefixes represent the usual flow through a end-user experience of running a dbt task. It has the benefit of being able to intuit how far things have gotten at the point of the event for the purposes of troubleshooting. |
| def code(cls) -> str: | ||
| raise Exception("code() not implemented for event") |
There was a problem hiding this comment.
Here's the abstract method that's doing the enforcement via mypy. I couldn't figure out how to make mypy check that subclasses have defined an abstract property even though the internet seems to think you can do that. You'll need to do one of two things to get this to work:
- figure out how to make an abstract property such that if you delete
codefrom one of the concrete classes, mypy fails. - change all the codes from fields to functions that override this one in each of the concrete classes.
There was a problem hiding this comment.
I just tried this by commenting out the code on one of the concrete tests and running mypy, and it worked. I'm not sure why though. @iknox-fa do you know why a @dataclass field named code in the concrete classes counts as overriding this abstract method??
There was a problem hiding this comment.
Because it's now an abstract property.... and data fields are just automagic properties generated by an automagic __init__ method.
There was a problem hiding this comment.
too implicit for my liking, but I'm glad it works.
| # | ||
| # The basic idea is that event codes roughly translate to the natural order of running a dbt task | ||
|
|
||
| # can't use ABCs with @dataclass because of https://github.com/python/mypy/issues/5374 | ||
| @dataclass | ||
| @dataclass # type: ignore |
There was a problem hiding this comment.
oh wow I didn't even think of doing this. we can get rid of the comment that says we can't do this then.
wait to merge into the feature branch till #4266 is in.
Description
Uses mypy to enforce codes that uniquely identify each log event. This allows consumers of structured logs to rely on these codes while giving developers the freedom to change class names.
The proposed format for codes is one capital letter followed by 3 digits:
aka
^[A-Z][0-9]{3}E.g. -A123,E003etc.The letter is determined by where the log line was fired. e.g. - parsing, node status, sql, adapter, deps etc.
TODO
Checklist
CHANGELOG.mdand added information about my change