This commit is contained in:
shreyas-goenka 2024-10-17 16:28:07 +05:30 committed by GitHub
commit 655346d81b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 118 additions and 69 deletions

View File

@ -30,10 +30,14 @@ func createGlobError(v dyn.Value, p dyn.Path, message string) diag.Diagnostic {
}
return diag.Diagnostic{
Severity: diag.Error,
Summary: fmt.Sprintf("%s: %s", source, message),
Locations: []dyn.Location{v.Location()},
Paths: []dyn.Path{p},
Severity: diag.Error,
Summary: fmt.Sprintf("%s: %s", source, message),
LocationPathPairs: []diag.LocationPathPair{
{
L: v.Location(),
P: p,
},
},
}
}

View File

@ -107,27 +107,24 @@ func validateSingleResourceDefined(configRoot dyn.Value, ext, typ string) diag.D
detail.WriteString(l)
}
locations := []dyn.Location{}
paths := []dyn.Path{}
// TODO: test this.
locationPathPairs := []diag.LocationPathPair{}
for _, rr := range resources {
locations = append(locations, rr.value.Locations()...)
paths = append(paths, rr.path)
for _, l := range rr.value.Locations() {
locationPathPairs = append(locationPathPairs, diag.LocationPathPair{L: l, P: rr.path})
}
}
// Sort the locations and paths to make the output deterministic.
sort.Slice(locations, func(i, j int) bool {
return locations[i].String() < locations[j].String()
})
sort.Slice(paths, func(i, j int) bool {
return paths[i].String() < paths[j].String()
// Sort the location-path pairs to make the output deterministic.
sort.Slice(locationPathPairs, func(i, j int) bool {
return locationPathPairs[i].L.String() < locationPathPairs[j].L.String()
})
return diag.Diagnostics{
{
Severity: diag.Recommendation,
Summary: fmt.Sprintf("define a single %s in a file with the %s extension.", strings.ReplaceAll(typ, "_", " "), ext),
Detail: detail.String(),
Locations: locations,
Paths: paths,
Severity: diag.Recommendation,
Summary: fmt.Sprintf("define a single %s in a file with the %s extension.", strings.ReplaceAll(typ, "_", " "), ext),
Detail: detail.String(),
LocationPathPairs: locationPathPairs,
},
}
}

View File

@ -220,10 +220,16 @@ func validatePauseStatus(b *bundle.Bundle) diag.Diagnostics {
if p == "" || p == config.Paused || p == config.Unpaused {
return nil
}
// TODO: test this.
return diag.Diagnostics{{
Summary: "Invalid value for trigger_pause_status, should be PAUSED or UNPAUSED",
Severity: diag.Error,
Locations: []dyn.Location{b.Config.GetLocation("presets.trigger_pause_status")},
Summary: "Invalid value for trigger_pause_status, should be PAUSED or UNPAUSED",
Severity: diag.Error,
LocationPathPairs: []diag.LocationPathPair{
{
L: b.Config.GetLocation("presets.trigger_pause_status"),
P: dyn.MustPathFromString("presets.trigger_pause_status"),
},
},
}}
}

View File

@ -52,11 +52,19 @@ func rewriteComputeIdToClusterId(v dyn.Value, p dyn.Path) (dyn.Value, diag.Diagn
return v, nil
}
// TODO: test this.
locationPathPaths := []diag.LocationPathPair{}
for _, l := range computeId.Locations() {
locationPathPaths = append(locationPathPaths, diag.LocationPathPair{
L: l,
P: computeIdPath,
})
}
diags = diags.Append(diag.Diagnostic{
Severity: diag.Warning,
Summary: "compute_id is deprecated, please use cluster_id instead",
Locations: computeId.Locations(),
Paths: []dyn.Path{computeIdPath},
Severity: diag.Warning,
Summary: "compute_id is deprecated, please use cluster_id instead",
LocationPathPairs: locationPathPaths,
})
clusterIdPath := p.Append(dyn.Key("cluster_id"))

View File

@ -76,9 +76,15 @@ func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics {
// historically allowed.)
if p.TriggerPauseStatus == config.Unpaused {
diags = diags.Append(diag.Diagnostic{
Severity: diag.Error,
Summary: "target with 'mode: development' cannot set trigger pause status to UNPAUSED by default",
Locations: []dyn.Location{b.Config.GetLocation("presets.trigger_pause_status")},
Severity: diag.Error,
Summary: "target with 'mode: development' cannot set trigger pause status to UNPAUSED by default",
// TODO: test this.
LocationPathPairs: []diag.LocationPathPair{
{
L: b.Config.GetLocation("presets.trigger_pause_status"),
P: dyn.MustPathFromString("presets.trigger_pause_status"),
},
},
})
}
@ -98,9 +104,15 @@ func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics {
// it's a pitfall for users if they don't include it and later find out that
// only a single user can do development deployments.
diags = diags.Append(diag.Diagnostic{
Severity: diag.Error,
Summary: "prefix should contain the current username or ${workspace.current_user.short_name} to ensure uniqueness when using 'mode: development'",
Locations: []dyn.Location{b.Config.GetLocation("presets.name_prefix")},
Severity: diag.Error,
Summary: "prefix should contain the current username or ${workspace.current_user.short_name} to ensure uniqueness when using 'mode: development'",
// TODO: test this.
LocationPathPairs: []diag.LocationPathPair{
{
L: b.Config.GetLocation("presets.name_prefix"),
P: dyn.MustPathFromString("presets.name_prefix"),
},
},
})
}
return diags

View File

@ -54,23 +54,23 @@ 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)
if location != (dyn.Location{}) {
locations = append(locations, location)
// TODO: test this.
locationPathPairs := []diag.LocationPathPair{}
if path != nil || location != (dyn.Location{}) {
locationPathPairs = append(locationPathPairs, diag.LocationPathPair{
L: location,
P: path,
})
}
diag := diag.Diagnostic{
Severity: severity,
Summary: parsedLine.Summary,
Detail: parsedLine.Detail,
Locations: locations,
Paths: paths,
Severity: severity,
Summary: parsedLine.Summary,
Detail: parsedLine.Detail,
LocationPathPairs: locationPathPairs,
}
diags = diags.Append(diag)

View File

@ -47,12 +47,21 @@ func (m *rewriteWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) di
for path, replacePath := range paths {
if strings.Contains(vv, path) {
newPath := strings.Replace(vv, path, replacePath, 1)
locationPathPairs := []diag.LocationPathPair{}
for _, l := range v.Locations() {
locationPathPairs = append(locationPathPairs, diag.LocationPathPair{
L: l,
P: p,
})
}
// TODO: test this.
diags = append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: fmt.Sprintf("substring %q found in %q. Please update this to %q.", path, vv, newPath),
Detail: "For more information, please refer to: https://docs.databricks.com/en/release-notes/dev-tools/bundles.html#workspace-paths",
Locations: v.Locations(),
Paths: []dyn.Path{p},
Severity: diag.Warning,
Summary: fmt.Sprintf("substring %q found in %q. Please update this to %q.", path, vv, newPath),
Detail: "For more information, please refer to: https://docs.databricks.com/en/release-notes/dev-tools/bundles.html#workspace-paths",
LocationPathPairs: locationPathPairs,
})
// Remove the workspace prefix from the string.

View File

@ -30,11 +30,19 @@ func (m *setRunAs) Name() string {
return "SetRunAs"
}
func reportRunAsNotSupported(resourceType string, location dyn.Location, currentUser string, runAsUser string) diag.Diagnostics {
// TODO: CONTINUE with transformation to pair and test this.
func reportRunAsNotSupported(resourceType string, location dyn.Location, path dyn.Path, currentUser string, runAsUser string) diag.Diagnostics {
return diag.Diagnostics{{
Summary: fmt.Sprintf("%s do not support a setting a run_as user that is different from the owner.\n"+
"Current identity: %s. Run as identity: %s.\n"+
"See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", resourceType, currentUser, runAsUser),
LocationPathPairs: []diag.LocationPathPair{
{
L: location,
},
},
Locations: []dyn.Location{location},
Severity: diag.Error,
}}

View File

@ -112,18 +112,19 @@ func renderSummaryTemplate(out io.Writer, b *bundle.Bundle, diags diag.Diagnosti
}
func renderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error {
// TODO: test this.
for _, d := range diags {
for i := range d.Locations {
for i := range d.LocationPathPairs {
if b == nil {
break
}
// Make location relative to bundle root
if d.Locations[i].File != "" {
out, err := filepath.Rel(b.BundleRootPath, d.Locations[i].File)
if d.LocationPathPairs[i].L.File != "" {
out, err := filepath.Rel(b.BundleRootPath, d.LocationPathPairs[i].L.File)
// if we can't relativize the path, just use path as-is
if err == nil {
d.Locations[i].File = out
d.LocationPathPairs[i].L.File = out
}
}
}

View File

@ -394,10 +394,7 @@ func fancyJSON(v any) ([]byte, error) {
const errorTemplate = `{{ "Error" | red }}: {{ .Summary }}
{{- 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 }}
{{ if eq $index 0 }}at {{else}} {{ end}}{{ $element.P.String | green }} in {{ $element.L.String | cyan }}
{{- end }}
{{- if .Detail }}
@ -406,12 +403,11 @@ const errorTemplate = `{{ "Error" | red }}: {{ .Summary }}
`
// TODO: Only print "at" and "in" if both are not nil.
// TODO: also add tests for this.
const warningTemplate = `{{ "Warning" | yellow }}: {{ .Summary }}
{{- 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 }}
{{ if eq $index 0 }}at {{else}} {{ end}}{{ $element.P.String | green }} in {{ $element.L.String | cyan }}
{{- end }}
{{- if .Detail }}
@ -421,11 +417,8 @@ const warningTemplate = `{{ "Warning" | yellow }}: {{ .Summary }}
`
const recommendationTemplate = `{{ "Recommendation" | blue }}: {{ .Summary }}
{{- 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 }}
{{- range $index, $element := .LocationPathPairs}}
{{ if eq $index 0 }}at {{else}} {{ end}}{{ $element.P.String | green }} in {{ $element.L.String | cyan }}
{{- end }}
{{- if .Detail }}

View File

@ -7,6 +7,15 @@ import (
"github.com/databricks/cli/libs/dyn"
)
// Convenience struct for retaining a 1:1 mapping between locations and paths.
type LocationPathPair struct {
// Locations are the source code locations associated with the diagnostic message.
L dyn.Location
// Paths are paths to the values in the configuration tree that the diagnostic is associated with.
P dyn.Path
}
type Diagnostic struct {
Severity Severity
@ -18,11 +27,13 @@ type Diagnostic struct {
// This may be multiple lines and may be nil.
Detail string
// Locations are the source code locations associated with the diagnostic message.
// LocationPathPairs are the source code locations and paths associated with the diagnostic message.
// It may be empty if there are no associated locations and paths.
LocationPathPairs []LocationPathPair
// It may be empty if there are no associated locations.
Locations []dyn.Location
// 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