RFC: Change logger behavior so that setting a default logger in a Definitions object changes the logger by default, without needing to set run config to enable it #24338
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Right now if you customize a logger on a job, you also need to set additional configuration on the run when launching the job in order for it to actually use that logger and not fall back to the detault system logger. This is quite different than what happens when you set an executor on a job, and is generally confusing.
This PR instead makes setting logger config more like executor config - if you set it, it is assumed that you want to use that logger, and applies that configuraiton as the default even if no configuration is set. Much like with executors, this could cause the job config to be invalid if the logger has non-default config options and they are not set (none of our default loggers have this property, but e.g. cloudwatch_logger in dagster-aws does, and I could imagine user-defined loggers having them as well).
The main blocker here is the potential for this to be breaking change - before, when you specified a logger on a job you were giving your self the option of opting into it, and now you are overriding whatever loggers you set as the default loggers. Perhaps there is some way to make this behavior change opt-in to alleviate the pain before changing the default behavior?
Summary & Motivation
How I Tested These Changes
Changelog
Insert changelog entry or "NOCHANGELOG" here.
NEW
(added new feature or capability)BUGFIX
(fixed a bug)DOCS
(added or updated documentation)