fix: surface WARNING/ERROR logs by default instead of suppressing them#1264
Merged
Conversation
Every command called enable_debug_logging(debug), which added a logging.NullHandler to the root logger when --debug was off. That disabled Python's lastResort handler, silencing all logging.warning/ logging.error output across the codebase unless --debug was passed. Default the root level to WARNING (on stderr) so diagnostics like the `import sql` placeholder-connection warning are visible. test/ci/lint render run.logs to the console themselves and would otherwise print the mirrored Run.log_* records twice, so they pass otherwise_disable_stderr to keep the old NullHandler behavior. Noisy third-party loggers are pinned to ERROR to preserve the original intent of muting library chatter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
Every CLI command calls
enable_debug_logging(debug)as its first action. When--debugis off, that function added alogging.NullHandlerto the root logger, which disables Python'slastResorthandler — so alllogging.warning/logging.errorcalls in the codebase were silent by default.The primary output of each command was unaffected (it goes through the rich
consoleon stdout), but thelogging-channel diagnostics were lost, e.g.:The
sql_importer"placeholder connection values" warning (andresolve.py/excel_importer/spark_importerwarnings) only appeared with--debug.Fix
enable_debug_loggingnow defaults the root level toWARNINGon stderr (so stdout stays clean for pipedimport/exportYAML), and raises it toDEBUGwith--debug. Noisy third-party loggers (snowflake.connector,py4j,pyspark,urllib3,soda) are pinned toERRORto preserve the original intent of theNullHandler(muting library chatter) without blanket-suppressing everything.Why
test/ci/lintare gatedRun.log_info/warn/errordeliberately emit to two sinks: the structuredrun.logsrecord (rendered to the console, exported to JUnit, published) and stdliblogging(the live stream, and the only output for library callers ofDataContract(...).test()). Fortest/ci/lint, which renderrun.logsto the console themselves, un-suppressing stderr would print the same WARNING/ERROR twice. Those three commands therefore passotherwise_disable_stderr=Trueto keep the oldNullHandlerbehavior. Commands that don't renderrun.logs(import,export,changelog,catalog,dbt,publish) get the WARNING-on-stderr default — a clean win with no duplication.Verification
Ran every command before/after, capturing stdout and stderr separately:
import sql: warning now on stderr; piped stdout YAML stays clean (warning not in the file).lint/test/ci: failure/warnings shown once (on stdout, as before) — no doubling on stderr.--debug: WARNING/ERROR/DEBUG still on stderr for all commands.743 passed, 13 skipped(the only errors are Docker-unavailable integration tests, unrelated).🤖 Generated with Claude Code