Skip to content

Commit

Permalink
Enable 'govet' linter; expand log/diag with non-f functions (#1996)
Browse files Browse the repository at this point in the history
## Changes
Fix all the govet-found issues and enable govet linter.

This prompts adding non-formatting variants of logging functions (Errorf
-> Error).

## Tests
Existing tests.
  • Loading branch information
denik authored Dec 11, 2024
1 parent 2ee7d56 commit 5924748
Show file tree
Hide file tree
Showing 11 changed files with 62 additions and 15 deletions.
7 changes: 6 additions & 1 deletion .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,17 @@ linters:
- bodyclose
- errcheck
- gosimple
#- govet
- govet
- ineffassign
- staticcheck
- unused
- gofmt
linters-settings:
govet:
enable-all: true
disable:
- fieldalignment
- shadow
gofmt:
rewrite-rules:
- pattern: 'a[b:len(a)]'
Expand Down
2 changes: 1 addition & 1 deletion bundle/artifacts/whl/autodetect.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic
return nil
}

log.Infof(ctx, fmt.Sprintf("Found Python wheel project at %s", b.BundleRootPath))
log.Infof(ctx, "Found Python wheel project at %s", b.BundleRootPath)
module := extractModuleName(setupPy)

if b.Config.Artifacts == nil {
Expand Down
2 changes: 1 addition & 1 deletion bundle/deferred_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (t *mutatorWithError) Name() string {

func (t *mutatorWithError) Apply(_ context.Context, b *Bundle) diag.Diagnostics {
t.applyCalled++
return diag.Errorf(t.errorMsg)
return diag.Errorf(t.errorMsg) // nolint:govet
}

func TestDeferredMutatorWhenAllMutatorsSucceed(t *testing.T) {
Expand Down
3 changes: 1 addition & 2 deletions bundle/deploy/terraform/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package terraform

import (
"context"
"fmt"
"path/filepath"

"github.com/databricks/cli/bundle"
Expand Down Expand Up @@ -57,7 +56,7 @@ func (p *plan) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
IsEmpty: !notEmpty,
}

log.Debugf(ctx, fmt.Sprintf("Planning complete and persisted at %s\n", planPath))
log.Debugf(ctx, "Planning complete and persisted at %s\n", planPath)
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion bundle/libraries/filer_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func TestFilerForVolumeNotFoundAndInBundle(t *testing.T) {
{
Severity: diag.Error,
Summary: "volume /Volumes/main/my_schema/my_volume does not exist: error from API",
Locations: []dyn.Location{{"config.yml", 1, 2}, {"volume.yml", 1, 2}},
Locations: []dyn.Location{{File: "config.yml", Line: 1, Column: 2}, {File: "volume.yml", Line: 1, Column: 2}},
Paths: []dyn.Path{dyn.MustPathFromString("workspace.artifact_path"), dyn.MustPathFromString("resources.volumes.foo")},
Detail: `You are using a volume in your artifact_path that is managed by
this bundle but which has not been deployed yet. Please first deploy
Expand Down
4 changes: 2 additions & 2 deletions bundle/run/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func logProgressCallback(ctx context.Context, progressLogger *cmdio.Logger) func
progressLogger.Log(event)

// log progress events in using the default logger
log.Infof(ctx, event.String())
log.Info(ctx, event.String())
}
}

Expand Down Expand Up @@ -203,7 +203,7 @@ func (r *jobRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, e
logDebug(r)
logProgress(r)
}).GetWithTimeout(jobRunTimeout)
if err != nil && runId != nil {
if err != nil {
r.logFailedTasks(ctx, *runId)
}
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions bundle/run/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (r *pipelineRunner) logEvent(ctx context.Context, event pipelines.PipelineE
}
}
if logString != "" {
log.Errorf(ctx, fmt.Sprintf("[%s] %s", event.EventType, logString))
log.Errorf(ctx, "[%s] %s", event.EventType, logString)
}
}

Expand Down Expand Up @@ -132,7 +132,7 @@ func (r *pipelineRunner) Run(ctx context.Context, opts *Options) (output.RunOutp
}
for _, event := range events {
progressLogger.Log(&event)
log.Infof(ctx, event.String())
log.Info(ctx, event.String())
}

update, err := w.Pipelines.GetUpdateByPipelineIdAndUpdateId(ctx, pipelineID, updateID)
Expand Down
3 changes: 0 additions & 3 deletions cmd/labs/project/entrypoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,6 @@ func (e *Entrypoint) getLoginConfig(cmd *cobra.Command) (*loginConfig, *config.C
if isNoLoginConfig && !e.IsBundleAware {
return nil, nil, ErrNoLoginConfig
}
if !isNoLoginConfig && err != nil {
return nil, nil, fmt.Errorf("load: %w", err)
}
if e.IsAccountLevel {
log.Debugf(ctx, "Using account-level login profile: %s", lc.AccountProfile)
cfg, err := e.envAwareConfigWithProfile(ctx, lc.AccountProfile)
Expand Down
3 changes: 2 additions & 1 deletion cmd/root/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import (
)

func TestEmptyHttpRequest(t *testing.T) {
ctx, _ := context.WithCancel(context.Background())
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
req := emptyHttpRequest(ctx)
assert.Equal(t, req.Context(), ctx)
}
Expand Down
45 changes: 45 additions & 0 deletions libs/log/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,51 @@ func log(logger *slog.Logger, ctx context.Context, level slog.Level, msg string)
_ = logger.Handler().Handle(ctx, r)
}

// Trace logs a string using the context-local or global logger.
func Trace(ctx context.Context, msg string) {
logger := GetLogger(ctx)
if !logger.Enabled(ctx, LevelTrace) {
return
}
log(logger, ctx, LevelTrace, msg)
}

// Debug logs a string using the context-local or global logger.
func Debug(ctx context.Context, msg string) {
logger := GetLogger(ctx)
if !logger.Enabled(ctx, LevelDebug) {
return
}
log(logger, ctx, LevelDebug, msg)
}

// Info logs a string using the context-local or global logger.
func Info(ctx context.Context, msg string) {
logger := GetLogger(ctx)
if !logger.Enabled(ctx, LevelInfo) {
return
}
log(logger, ctx, LevelInfo, msg)
}

// Warn logs a string using the context-local or global logger.
func Warn(ctx context.Context, msg string) {
logger := GetLogger(ctx)
if !logger.Enabled(ctx, LevelWarn) {
return
}
log(logger, ctx, LevelWarn, msg)
}

// Error logs a string using the context-local or global logger.
func Error(ctx context.Context, msg string) {
logger := GetLogger(ctx)
if !logger.Enabled(ctx, LevelError) {
return
}
log(logger, ctx, LevelError, msg)
}

// Tracef logs a formatted string using the context-local or global logger.
func Tracef(ctx context.Context, format string, v ...any) {
logger := GetLogger(ctx)
Expand Down
2 changes: 1 addition & 1 deletion libs/template/renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ func (r *renderer) persistToDisk(ctx context.Context, out filer.Filer) error {
if err == nil {
return fmt.Errorf("failed to initialize template, one or more files already exist: %s", path)
}
if err != nil && !errors.Is(err, fs.ErrNotExist) {
if !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("error while verifying file %s does not already exist: %w", path, err)
}
}
Expand Down

0 comments on commit 5924748

Please sign in to comment.