From 1908c95864277659c825d63adb323e6300ff93e0 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 27 Nov 2025 14:16:49 +0100 Subject: [PATCH 1/2] Revise log-level handoff to labs subcommands: use notset when not specified by user. Specifically, the handoff shouldn't use the default (WARN) if the user didn't specify it explicitly. --- cmd/labs/project/command_test.go | 103 +++++++++++++++++++++++++++++++ cmd/labs/project/proxy.go | 27 +++++++- 2 files changed, 128 insertions(+), 2 deletions(-) diff --git a/cmd/labs/project/command_test.go b/cmd/labs/project/command_test.go index 453329e1d1..afac17be1a 100644 --- a/cmd/labs/project/command_test.go +++ b/cmd/labs/project/command_test.go @@ -2,12 +2,14 @@ package project_test import ( "context" + "os" "path/filepath" "testing" "time" "github.com/databricks/cli/internal/testcli" "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/python" "github.com/databricks/databricks-sdk-go" "github.com/stretchr/testify/assert" @@ -67,3 +69,104 @@ func TestRenderingTable(t *testing.T) { Third Fourth `) } + +func TestLogLevelHandoff(t *testing.T) { + if _, ok := os.LookupEnv("DATABRICKS_LOG_LEVEL"); ok { + t.Fatal("DATABRICKS_LOG_LEVEL must not be set when running this test") + } + + testCases := []struct { + name string + envVar string + args []string + expectedLevel string + }{ + { + name: "not set by default", + expectedLevel: "notset", + }, + { + name: "set explicitly with --log-level", + args: []string{"--log-level", "iNFo"}, + expectedLevel: "info", + }, + { + name: "set by --debug flag", + args: []string{"--debug"}, + expectedLevel: "debug", + }, + { + name: "set by env var", + envVar: "tRaCe", + expectedLevel: "trace", + }, + { + name: "set to default", + envVar: "warn", + expectedLevel: "warn", + }, + { + name: "disabled log level", + args: []string{"--log-level", "disabled"}, + expectedLevel: "disabled", + }, + { + name: "invalid env var ignored", + envVar: "invalid-level", + expectedLevel: "notset", + }, + { + name: "conflict: --debug trumps --log-level and env var", + envVar: "error", + args: []string{"--debug", "--log-level", "trace"}, + expectedLevel: "debug", + }, + { + name: "conflict: --log-level trumps env var", + envVar: "error", + args: []string{"--log-level", "info"}, + expectedLevel: "info", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := devEnvContext(t) + if tc.envVar != "" { + ctx = env.Set(ctx, "DATABRICKS_LOG_LEVEL", tc.envVar) + } + args := append([]string{"labs", "blueprint", "echo"}, tc.args...) + r := testcli.NewRunner(t, ctx, args...) + var out echoOut + r.RunAndParseJSON(&out) + usedLevel := out.Flags["log_level"] + assert.Equal(t, tc.expectedLevel, usedLevel) + + // Verify that our expectation matches what the logger has actually been configured to use. + // This should catch drift between the logic in cmd/root/logger.go and cmd/labs/project/proxy.go. + if tc.expectedLevel != "notset" { + actualLoggerLevel := getLoggerLevel(ctx) + assert.Equal(t, tc.expectedLevel, actualLoggerLevel) + } + }) + } +} + +func getLoggerLevel(ctx context.Context) string { + logger := log.GetLogger(ctx) + var level string + if logger.Enabled(ctx, log.LevelTrace) { + level = "trace" + } else if logger.Enabled(ctx, log.LevelDebug) { + level = "debug" + } else if logger.Enabled(ctx, log.LevelInfo) { + level = "info" + } else if logger.Enabled(ctx, log.LevelWarn) { + level = "warn" + } else if logger.Enabled(ctx, log.LevelError) { + level = "error" + } else { + level = "disabled" + } + return level +} diff --git a/cmd/labs/project/proxy.go b/cmd/labs/project/proxy.go index 82c7fb90ca..abe389da06 100644 --- a/cmd/labs/project/proxy.go +++ b/cmd/labs/project/proxy.go @@ -10,12 +10,15 @@ import ( "strings" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/process" "github.com/spf13/cobra" "github.com/spf13/pflag" ) +const envLogLevel = "DATABRICKS_LOG_LEVEL" + type proxy struct { Entrypoint `yaml:",inline"` Name string `yaml:"name"` @@ -107,12 +110,32 @@ func (cp *proxy) commandInput(cmd *cobra.Command) ([]string, error) { } commandInput.Flags[f.Name] = v } + ctx := cmd.Context() + /* + * Although we _could_ get the log-level from the logger in context, that would not tell us + * whether the user explicitly set it or whether it's just the default. So instead here we + * check the same places that the root command does when initializing logging: + * DATABRICKS_LOG_LEVEL (env), --log-level and --debug. + * Note: we rely on tests to catch any drift between here and the root log-level. + */ logLevelFlag := flags.Lookup("log-level") if logLevelFlag != nil { - commandInput.Flags["log_level"] = logLevelFlag.Value.String() + debugFlag := flags.Lookup("debug") + envValue, hasEnvLogLevel := env.Lookup(ctx, envLogLevel) + logLevelInUse := logLevelFlag.Value.String() + // Quirk: env var is ignored if invalid, so only treat as user-supplied if it matches + // the value in use. + userSupplied := logLevelFlag.Changed || (debugFlag != nil && debugFlag.Changed) || + hasEnvLogLevel && strings.EqualFold(envValue, logLevelInUse) + var logLevel string + if userSupplied { + logLevel = logLevelInUse + } else { + logLevel = "notset" + } + commandInput.Flags["log_level"] = logLevel } var args []string - ctx := cmd.Context() if cp.IsPythonProject() { args = append(args, cp.virtualEnvPython(ctx)) libDir := cp.EffectiveLibDir() From b51aba8cab16fc75c325e5f087ca9602221e6165 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 27 Nov 2025 14:59:58 +0100 Subject: [PATCH 2/2] Use 'disabled' as the handoff value for labs tools. This is what the Python tools all expect when no logging level has been provided. --- cmd/labs/project/command_test.go | 12 ++++-------- cmd/labs/project/proxy.go | 3 ++- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/cmd/labs/project/command_test.go b/cmd/labs/project/command_test.go index afac17be1a..ec6476c4aa 100644 --- a/cmd/labs/project/command_test.go +++ b/cmd/labs/project/command_test.go @@ -82,8 +82,9 @@ func TestLogLevelHandoff(t *testing.T) { expectedLevel string }{ { + // Historical handoff value when the user does not set an explicit log level. name: "not set by default", - expectedLevel: "notset", + expectedLevel: "disabled", }, { name: "set explicitly with --log-level", @@ -105,15 +106,10 @@ func TestLogLevelHandoff(t *testing.T) { envVar: "warn", expectedLevel: "warn", }, - { - name: "disabled log level", - args: []string{"--log-level", "disabled"}, - expectedLevel: "disabled", - }, { name: "invalid env var ignored", envVar: "invalid-level", - expectedLevel: "notset", + expectedLevel: "disabled", }, { name: "conflict: --debug trumps --log-level and env var", @@ -144,7 +140,7 @@ func TestLogLevelHandoff(t *testing.T) { // Verify that our expectation matches what the logger has actually been configured to use. // This should catch drift between the logic in cmd/root/logger.go and cmd/labs/project/proxy.go. - if tc.expectedLevel != "notset" { + if tc.expectedLevel != "disabled" { actualLoggerLevel := getLoggerLevel(ctx) assert.Equal(t, tc.expectedLevel, actualLoggerLevel) } diff --git a/cmd/labs/project/proxy.go b/cmd/labs/project/proxy.go index abe389da06..54da6de5a0 100644 --- a/cmd/labs/project/proxy.go +++ b/cmd/labs/project/proxy.go @@ -131,7 +131,8 @@ func (cp *proxy) commandInput(cmd *cobra.Command) ([]string, error) { if userSupplied { logLevel = logLevelInUse } else { - logLevel = "notset" + // Historical value to indicate "not set by user". + logLevel = "disabled" } commandInput.Flags["log_level"] = logLevel }