Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Paw/ct 1652 restore default logging #6447

Merged
merged 3 commits into from
Dec 14, 2022
Merged

Paw/ct 1652 restore default logging #6447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e7b1f8
CT-1652: Restore stdout logging before logger is fully configured
peterallenwebb Dec 14, 2022
641d2bd
CT-1652: Add changelog entry
peterallenwebb Dec 14, 2022
a390664
CT-1652: formatting fix for black
peterallenwebb Dec 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changes/unreleased/Fixes-20221214-155307.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Fixes
body: Repair a regression which prevented basic logging before the logging subsystem
is completely configured.
time: 2022-12-14T15:53:07.396512-05:00
custom:
Author: peterallenwebb
Issue: "6434"
17 changes: 8 additions & 9 deletions core/dbt/events/functions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def setup_event_logger(log_path: str, level_override: Optional[EventLevel] = Non
EVENT_MANAGER.add_logger(_get_logfile_config(os.path.join(log_path, "dbt.log")))


def _get_stdout_config(level: Optional[EventLevel]) -> LoggerConfig:
def _get_stdout_config(level: Optional[EventLevel] = None) -> LoggerConfig:
fmt = LineFormat.PlainText
if flags.LOG_FORMAT == "json":
fmt = LineFormat.Json
Expand Down Expand Up @@ -90,7 +90,7 @@ def _logfile_filter(log_cache_events: bool, evt: BaseEvent) -> bool:
)


def _get_logbook_log_config(level: Optional[EventLevel]) -> LoggerConfig:
def _get_logbook_log_config(level: Optional[EventLevel] = None) -> LoggerConfig:
config = _get_stdout_config(level)
config.name = "logbook_log"
config.filter = NoFilter if flags.LOG_CACHE_EVENTS else lambda e: not isinstance(e, Cache)
Expand All @@ -110,15 +110,14 @@ def cleanup_event_logger():
EVENT_MANAGER.callbacks.clear()


# The default event manager will not log anything, but some tests run code that
# generates events, without configuring the event manager, so we create an empty
# manager here until there is a better testing strategy in place.
# Since dbt-rpc does not do its own log setup, and since some events can
# currently fire before logs can be configured by setup_event_logger(), we
# create a default configuration with default settings and no file output.
EVENT_MANAGER: EventManager = EventManager()
EVENT_MANAGER.add_logger(
_get_logbook_log_config() if flags.ENABLE_LEGACY_LOGGER else _get_stdout_config()
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[answered here] Is there a risk of double-adding loggers downstream, leading to extraneous calls to logger.write?


# Since dbt-rpc does not do its own log setup, we set up logbook if legacy
# logging is enabled.
if flags.ENABLE_LEGACY_LOGGER:
EVENT_MANAGER.add_logger(_get_logbook_log_config(None))

# This global, and the following two functions for capturing stdout logs are
# an unpleasant hack we intend to remove as part of API-ification. The GitHub
Expand Down