Page MenuHomeElementl

build_init_logger_context
ClosedPublic

Authored by cdecarolis on May 25 2021, 4:08 PM.

Details

Summary

This diff adds build_logger_init_context, in response to a user that requested it https://github.com/dagster-io/dagster/discussions/4174.
There's some weirdness here around having to provide a logger definition to the function. An alternative could be to make this a method on the logger definition itself, but that doesnt fit in with the other build_x_context patterns.

There's also the question of pipeline_def and run id. Should these be exposed at top level?

Test Plan

Added unit tests

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

python_modules/dagster/dagster/core/definitions/logger.py
99

build_init_logger_context would mirror the order of the words in InitLoggerContext. is swapping "logger" and "init" intentional?

100

can we make logger_def Optional? I suspect it won't be needed in most situations.

This revision now requires changes to proceed.May 25 2021, 10:46 PM
python_modules/dagster/dagster/core/definitions/logger.py
100

definitely. We literally never use it anywhere in the codebase

Update name to match context, make definition optional.

This revision is now accepted and ready to land.Wed, May 26, 3:15 PM