This essentially implements Option 2 in https://github.com/dagster-io/dagster/discussions/4195.
Details
Details
Diff Detail
Diff Detail
- Repository
- R1 dagster
- Branch
- logging-capture (branched from master)
- Lint
Lint Passed - Unit
No Test Coverage
Event Timeline
Comment Actions
does this work with a log line emitted by a solid that isn't defined inline (but loaded from a file/module); does this work with log lines emitted by libraries called by solids?
| python_modules/dagster/dagster/core/execution/plan/python_logging.py | ||
|---|---|---|
| 18 | typo | |
Comment Actions
does this work with a log line emitted by a solid that isn't defined inline (but loaded from a file/module); does this work with log lines emitted by libraries called by solids?
I can write a test to verify, but it should. The root logger is global.
Comment Actions
One nice thing we could do here would be to make it easy to filter down log messages in the structured viewer to specific loggers. That could actually be a pretty nice feature.
Comment Actions
One nice thing we could do here would be to make it easy to filter down log messages in the structured viewer to specific loggers. That could actually be a pretty nice feature.
Oo yeah, that would be cool.
Comment Actions
if we did this we would want it behind an opt-in gating of some kind right?
In 0.11.0 yes, but IMO we'd want to have it on by default in 0.12.0. The goal is that hello world could be:
import logging
@solid
def hello(name: str):
logging.info(f"Hello, {name}!")Comment Actions
sending to your queue, I am guessing we are not going to move on this for 0.12.0 given the current timing.
I am very very very hesitant about sending all python logging to the event DB by default.
I am cool with an easy explicit opt in, but worry about the impact this would have as an implicit default. Especially with our current poor tooling around cleaning up or understanding whats in the DB.