Paw/ct 1844 log params take 2 #6994
Conversation
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
|
This doesn't touch UserConfig (in core/dbt/contracts/project.py) where users can set log_format and use_colors. So I'm wondering how those settings integrate with these cli params, and whether we shouldn't allow setting the new params (somehow) in user config also. If I have a logging configuration that I want to use all the time, I would prefer not to have to pass in cli params every time, but have the settings persistent in user config. |
|
@gshank Yes, that is an interesting point which I did not consider. @jtcohen6 Do you agree that we should let users set these same flags in their user config, following the pattern of log_format and use_colors? I worry that the interaction patterns between all the different places these get set could start to become complicated, but Gerda's observations do make sense to me. On the other hand, maybe Gerda's use case could be supported by the work we are considering for #6993. |
|
@peterallenwebb @gshank Two-part answer:
|
|
@gshank Still waiting for the checks to pass here, but let me know whether the latest changes address your concerns. |
| @@ -172,6 +177,12 @@ def assign_params(ctx, params_assigned_from_default): | |||
| for param in params: | |||
| object.__setattr__(self, param.lower(), getattr(self, param)) | |||
|
|
|||
| # If the value of the lead parameter was set explicitly, apply the value to follow, | |||
| # unless follow was also set explicitly. | |||
There was a problem hiding this comment.
tiny nit: documenting the method in a docstring below the signature would be more pythonic + consistent with the rest of this file.
| log_cache_events: bool, | ||
| quiet: bool, | ||
| ) -> None: | ||
| def setup_event_logger(flags) -> None: |
There was a problem hiding this comment.
are we able to annotate the type of flags here?
core/dbt/events/functions.py
Outdated
| ) | ||
| else: | ||
| if flags.LOG_LEVEL != "none": | ||
| log_format = _line_format_from_str(flags.LOG_FORMAT, LineFormat.PlainText) |
There was a problem hiding this comment.
is there a difference between log_format and line_format? If not, line_format would be more intuitive given the naming of _line_format_from_str + parameter name of _get_stdout_config.
There was a problem hiding this comment.
I agree, the variable being assigned here should be line_format. There is no difference between the two things, and the difference in naming is unfortunate.
resolves #6639
Description
Adds the following cli parameters for finer grained control of console and file log behavior, as described in #6639:
--log-format
--log-format-file
--log-level
--log-level-file
--use-colors
--use-colors-file
--no-use-colors
--no-use-colors-file the
Checklist
changie newto create a changelog entry