Support multiple paths for diagnostics (#1616)

## Changes
Some diagnostics can have multiple paths associated with them. For
instance, ensuring that unique resource keys are used across all
resources. This PR extends `diag.Diagnostic` to accept multiple paths.

This PR is symmetrical to
https://github.com/databricks/cli/pull/1610/files

## Tests
Unit tests
This commit is contained in:
shreyas-goenka 2024-07-25 20:46:27 +05:30 committed by GitHub
parent 90aaf2d20f
commit 37b9df96e6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 76 additions and 48 deletions

View File

@ -54,6 +54,10 @@ func parsePythonDiagnostics(input io.Reader) (diag.Diagnostics, error) {
if err != nil {
return nil, fmt.Errorf("failed to parse path: %s", err)
}
var paths []dyn.Path
if path != nil {
paths = []dyn.Path{path}
}
var locations []dyn.Location
location := convertPythonLocation(parsedLine.Location)
@ -66,7 +70,7 @@ func parsePythonDiagnostics(input io.Reader) (diag.Diagnostics, error) {
Summary: parsedLine.Summary,
Detail: parsedLine.Detail,
Locations: locations,
Path: path,
Paths: paths,
}
diags = diags.Append(diag)

View File

@ -56,7 +56,7 @@ func TestParsePythonDiagnostics(t *testing.T) {
{
Severity: diag.Error,
Summary: "error summary",
Path: dyn.MustPathFromString("resources.jobs.job0.name"),
Paths: []dyn.Path{dyn.MustPathFromString("resources.jobs.job0.name")},
},
},
},

View File

@ -180,7 +180,7 @@ func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
{
Severity: diag.Warning,
Summary: "You are using the legacy mode of run_as. The support for this mode is experimental and might be removed in a future release of the CLI. In order to run the DLT pipelines in your DAB as the run_as user this mode changes the owners of the pipelines to the run_as identity, which requires the user deploying the bundle to be a workspace admin, and also a Metastore admin if the pipeline target is in UC.",
Path: dyn.MustPathFromString("experimental.use_legacy_run_as"),
Paths: []dyn.Path{dyn.MustPathFromString("experimental.use_legacy_run_as")},
Locations: b.Config.GetLocations("experimental.use_legacy_run_as"),
},
}

View File

@ -6,6 +6,7 @@ import (
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/deploy/files"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
)
func FilesToSync() bundle.ReadOnlyMutator {
@ -48,7 +49,7 @@ func (v *filesToSync) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.
// Show all locations where sync.exclude is defined, since merging
// sync.exclude is additive.
Locations: loc.Locations(),
Path: loc.Path(),
Paths: []dyn.Path{loc.Path()},
})
}

View File

@ -46,7 +46,7 @@ func (v *jobClusterKeyDefined) Apply(ctx context.Context, rb bundle.ReadOnlyBund
// Other associated locations are not relevant since they are
// overridden during merging.
Locations: []dyn.Location{loc.Location()},
Path: loc.Path(),
Paths: []dyn.Path{loc.Path()},
})
}
}

View File

@ -68,7 +68,7 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di
Severity: diag.Warning,
Summary: fmt.Sprintf("Pattern %s does not match any files", p),
Locations: []dyn.Location{loc.Location()},
Path: loc.Path(),
Paths: []dyn.Path{loc.Path()},
})
mu.Unlock()
}

View File

@ -29,8 +29,8 @@ var renderFuncMap = template.FuncMap{
}
const errorTemplate = `{{ "Error" | red }}: {{ .Summary }}
{{- if .Path.String }}
{{ "at " }}{{ .Path.String | green }}
{{- range $index, $element := .Paths }}
{{ if eq $index 0 }}at {{else}} {{ end}}{{ $element.String | green }}
{{- end }}
{{- range $index, $element := .Locations }}
{{ if eq $index 0 }}in {{else}} {{ end}}{{ $element.String | cyan }}
@ -43,8 +43,8 @@ const errorTemplate = `{{ "Error" | red }}: {{ .Summary }}
`
const warningTemplate = `{{ "Warning" | yellow }}: {{ .Summary }}
{{- if .Path.String }}
{{ "at " }}{{ .Path.String | green }}
{{- range $index, $element := .Paths }}
{{ if eq $index 0 }}at {{else}} {{ end}}{{ $element.String | green }}
{{- end }}
{{- range $index, $element := .Locations }}
{{ if eq $index 0 }}in {{else}} {{ end}}{{ $element.String | cyan }}

View File

@ -275,7 +275,7 @@ func TestRenderDiagnostics(t *testing.T) {
Severity: diag.Error,
Detail: "'name' is required",
Summary: "failed to load xxx",
Path: dyn.MustPathFromString("resources.jobs.xxx"),
Paths: []dyn.Path{dyn.MustPathFromString("resources.jobs.xxx")},
},
},
expected: "Error: failed to load xxx\n" +
@ -283,6 +283,27 @@ func TestRenderDiagnostics(t *testing.T) {
"\n" +
"'name' is required\n\n",
},
{
name: "error with multiple paths",
diags: diag.Diagnostics{
{
Severity: diag.Error,
Detail: "'name' is required",
Summary: "failed to load xxx",
Paths: []dyn.Path{
dyn.MustPathFromString("resources.jobs.xxx"),
dyn.MustPathFromString("resources.jobs.yyy"),
dyn.MustPathFromString("resources.jobs.zzz"),
},
},
},
expected: "Error: failed to load xxx\n" +
" at resources.jobs.xxx\n" +
" resources.jobs.yyy\n" +
" resources.jobs.zzz\n" +
"\n" +
"'name' is required\n\n",
},
}
for _, tc := range testCases {

View File

@ -22,7 +22,9 @@ func TestSyncIncludeExcludeNoMatchesTest(t *testing.T) {
require.Equal(t, diags[0].Severity, diag.Warning)
require.Equal(t, diags[0].Summary, "Pattern dist does not match any files")
require.Equal(t, diags[0].Path.String(), "sync.exclude[0]")
require.Len(t, diags[0].Paths, 1)
require.Equal(t, diags[0].Paths[0].String(), "sync.exclude[0]")
assert.Len(t, diags[0].Locations, 1)
require.Equal(t, diags[0].Locations[0].File, filepath.Join("sync", "override", "databricks.yml"))

View File

@ -21,9 +21,9 @@ type Diagnostic struct {
// It may be empty if there are no associated locations.
Locations []dyn.Location
// Path is a path to the value in a configuration tree that the diagnostic is associated with.
// It may be nil if there is no associated path.
Path dyn.Path
// Paths are paths to the values in the configuration tree that the diagnostic is associated with.
// It may be nil if there are no associated paths.
Paths []dyn.Path
}
// Errorf creates a new error diagnostic.

View File

@ -68,7 +68,7 @@ func nullWarning(expected dyn.Kind, src dyn.Value, path dyn.Path) diag.Diagnosti
Severity: diag.Warning,
Summary: fmt.Sprintf("expected a %s value, found null", expected),
Locations: []dyn.Location{src.Location()},
Path: path,
Paths: []dyn.Path{path},
}
}
@ -77,7 +77,7 @@ func typeMismatch(expected dyn.Kind, src dyn.Value, path dyn.Path) diag.Diagnost
Severity: diag.Warning,
Summary: fmt.Sprintf("expected %s, found %s", expected, src.Kind()),
Locations: []dyn.Location{src.Location()},
Path: path,
Paths: []dyn.Path{path},
}
}
@ -100,7 +100,7 @@ func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen
Summary: fmt.Sprintf("unknown field: %s", pk.MustString()),
// Show all locations the unknown field is defined at.
Locations: pk.Locations(),
Path: path,
Paths: []dyn.Path{path},
})
}
continue
@ -324,7 +324,7 @@ func (n normalizeOptions) normalizeInt(typ reflect.Type, src dyn.Value, path dyn
Severity: diag.Warning,
Summary: fmt.Sprintf(`cannot accurately represent "%g" as integer due to precision loss`, src.MustFloat()),
Locations: []dyn.Location{src.Location()},
Path: path,
Paths: []dyn.Path{path},
})
}
case dyn.KindString:
@ -340,7 +340,7 @@ func (n normalizeOptions) normalizeInt(typ reflect.Type, src dyn.Value, path dyn
Severity: diag.Warning,
Summary: fmt.Sprintf("cannot parse %q as an integer", src.MustString()),
Locations: []dyn.Location{src.Location()},
Path: path,
Paths: []dyn.Path{path},
})
}
case dyn.KindNil:
@ -367,7 +367,7 @@ func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value, path d
Severity: diag.Warning,
Summary: fmt.Sprintf(`cannot accurately represent "%d" as floating point number due to precision loss`, src.MustInt()),
Locations: []dyn.Location{src.Location()},
Path: path,
Paths: []dyn.Path{path},
})
}
case dyn.KindString:
@ -383,7 +383,7 @@ func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value, path d
Severity: diag.Warning,
Summary: fmt.Sprintf("cannot parse %q as a floating point number", src.MustString()),
Locations: []dyn.Location{src.Location()},
Path: path,
Paths: []dyn.Path{path},
})
}
case dyn.KindNil:

View File

@ -43,7 +43,7 @@ func TestNormalizeStructElementDiagnostic(t *testing.T) {
Severity: diag.Warning,
Summary: `expected string, found map`,
Locations: []dyn.Location{{}},
Path: dyn.NewPath(dyn.Key("bar")),
Paths: []dyn.Path{dyn.NewPath(dyn.Key("bar"))},
}, err[0])
// Elements that encounter an error during normalization are dropped.
@ -79,7 +79,7 @@ func TestNormalizeStructUnknownField(t *testing.T) {
{File: "hello.yaml", Line: 1, Column: 1},
{File: "world.yaml", Line: 2, Column: 2},
},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
// The field that can be mapped to the struct field is retained.
@ -113,7 +113,7 @@ func TestNormalizeStructError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected map, found string`,
Locations: []dyn.Location{vin.Get("foo").Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -258,7 +258,7 @@ func TestNormalizeStructRandomStringError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected map, found string`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -275,7 +275,7 @@ func TestNormalizeStructIntError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected map, found int`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -304,7 +304,7 @@ func TestNormalizeMapElementDiagnostic(t *testing.T) {
Severity: diag.Warning,
Summary: `expected string, found map`,
Locations: []dyn.Location{{}},
Path: dyn.NewPath(dyn.Key("bar")),
Paths: []dyn.Path{dyn.NewPath(dyn.Key("bar"))},
}, err[0])
// Elements that encounter an error during normalization are dropped.
@ -330,7 +330,7 @@ func TestNormalizeMapError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected map, found string`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -385,7 +385,7 @@ func TestNormalizeMapRandomStringError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected map, found string`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -398,7 +398,7 @@ func TestNormalizeMapIntError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected map, found int`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -428,7 +428,7 @@ func TestNormalizeSliceElementDiagnostic(t *testing.T) {
Severity: diag.Warning,
Summary: `expected string, found map`,
Locations: []dyn.Location{{}},
Path: dyn.NewPath(dyn.Index(2)),
Paths: []dyn.Path{dyn.NewPath(dyn.Index(2))},
}, err[0])
// Elements that encounter an error during normalization are dropped.
@ -452,7 +452,7 @@ func TestNormalizeSliceError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected sequence, found string`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -507,7 +507,7 @@ func TestNormalizeSliceRandomStringError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected sequence, found string`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -520,7 +520,7 @@ func TestNormalizeSliceIntError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected sequence, found int`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -541,7 +541,7 @@ func TestNormalizeStringNil(t *testing.T) {
Severity: diag.Warning,
Summary: `expected a string value, found null`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -578,7 +578,7 @@ func TestNormalizeStringError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected string, found map`,
Locations: []dyn.Location{{}},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -599,7 +599,7 @@ func TestNormalizeBoolNil(t *testing.T) {
Severity: diag.Warning,
Summary: `expected a bool value, found null`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -641,7 +641,7 @@ func TestNormalizeBoolFromStringError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected bool, found string`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -654,7 +654,7 @@ func TestNormalizeBoolError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected bool, found map`,
Locations: []dyn.Location{{}},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -675,7 +675,7 @@ func TestNormalizeIntNil(t *testing.T) {
Severity: diag.Warning,
Summary: `expected a int value, found null`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -696,7 +696,7 @@ func TestNormalizeIntFromFloatError(t *testing.T) {
Severity: diag.Warning,
Summary: `cannot accurately represent "1.5" as integer due to precision loss`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -725,7 +725,7 @@ func TestNormalizeIntFromStringError(t *testing.T) {
Severity: diag.Warning,
Summary: `cannot parse "abc" as an integer`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -738,7 +738,7 @@ func TestNormalizeIntError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected int, found map`,
Locations: []dyn.Location{{}},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -759,7 +759,7 @@ func TestNormalizeFloatNil(t *testing.T) {
Severity: diag.Warning,
Summary: `expected a float value, found null`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -784,7 +784,7 @@ func TestNormalizeFloatFromIntError(t *testing.T) {
Severity: diag.Warning,
Summary: `cannot accurately represent "9007199254740993" as floating point number due to precision loss`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -813,7 +813,7 @@ func TestNormalizeFloatFromStringError(t *testing.T) {
Severity: diag.Warning,
Summary: `cannot parse "abc" as a floating point number`,
Locations: []dyn.Location{vin.Location()},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}
@ -826,7 +826,7 @@ func TestNormalizeFloatError(t *testing.T) {
Severity: diag.Warning,
Summary: `expected float, found map`,
Locations: []dyn.Location{{}},
Path: dyn.EmptyPath,
Paths: []dyn.Path{dyn.EmptyPath},
}, err[0])
}