Fix/update default log-level handoff for databricks labs tools
#4019
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.
Changes
This PR updates log-level handoff to the labs tools: they expect
disabledinstead of the default (WARN) if the user didn't explicitly specify a log-level.Why
For various historical reasons, the labs tools were written with a default of
INFOin mind and use this if the user doesn't explicitly provide a log-level when running a command. Prior to #2213 we used to receivedisabledand it all worked as intended because the tools checked for this and could default toINFOthemselves when it was detected.However since #2213 the tools receive
warnif the user didn't specify anything, and the tools can't distinguish between explicit--log-level=WARNand the default situation. This has lead to confusion amongst our users (and the FE teams involved) because suddenly the tool output changed.With this change we're effectively reverting to the pre-#2213 behaviour which allows the labs tools to default to
INFOwhile still respecting--debug,--log-levelandDATABRICKS_LOG_LEVEL.Tests
Table-driven tests check various scenarios, and also verify that there's no drift between the value we handoff and the log-level in use by the CLI. These changes have also been tested manually.
Caveats
One thing that's worth considering: it feels brittle that we have to mirror the logic from
root/logger.go. The changes here are deliberately limited to just the labs area of the code, but if that constraint is relaxed there's probably a better way of handling it although it would mean changes outside the labs tree.