PythonMutator: preserve normalize diagnostics (#1735)

## Changes
Preserve diagnostics if there are any errors or warnings when
PythonMutator normalizes output. If anything goes wrong during
conversion, diagnostics contain the relevant location and path.

## Tests
Unit tests
This commit is contained in:
Gleb Kanterov 2024-08-30 15:29:00 +02:00 committed by GitHub
parent ed4a4585c0
commit 70ce802518
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 39 additions and 16 deletions

View File

@ -205,10 +205,8 @@ func (m *pythonMutator) runPythonMutator(ctx context.Context, cacheDir string, r
return dyn.InvalidValue, diag.Errorf("failed to load diagnostics: %s", pythonDiagnosticsErr) return dyn.InvalidValue, diag.Errorf("failed to load diagnostics: %s", pythonDiagnosticsErr)
} }
output, err := loadOutputFile(rootPath, outputPath) output, outputDiags := loadOutputFile(rootPath, outputPath)
if err != nil { pythonDiagnostics = pythonDiagnostics.Extend(outputDiags)
return dyn.InvalidValue, diag.Errorf("failed to load Python mutator output: %s", err)
}
// we pass through pythonDiagnostic because it contains warnings // we pass through pythonDiagnostic because it contains warnings
return output, pythonDiagnostics return output, pythonDiagnostics
@ -225,10 +223,10 @@ func writeInputFile(inputPath string, input dyn.Value) error {
return os.WriteFile(inputPath, rootConfigJson, 0600) return os.WriteFile(inputPath, rootConfigJson, 0600)
} }
func loadOutputFile(rootPath string, outputPath string) (dyn.Value, error) { func loadOutputFile(rootPath string, outputPath string) (dyn.Value, diag.Diagnostics) {
outputFile, err := os.Open(outputPath) outputFile, err := os.Open(outputPath)
if err != nil { if err != nil {
return dyn.InvalidValue, fmt.Errorf("failed to open output file: %w", err) return dyn.InvalidValue, diag.FromErr(fmt.Errorf("failed to open output file: %w", err))
} }
defer outputFile.Close() defer outputFile.Close()
@ -243,27 +241,34 @@ func loadOutputFile(rootPath string, outputPath string) (dyn.Value, error) {
// for that, we pass virtualPath instead of outputPath as file location // for that, we pass virtualPath instead of outputPath as file location
virtualPath, err := filepath.Abs(filepath.Join(rootPath, "__generated_by_pydabs__.yml")) virtualPath, err := filepath.Abs(filepath.Join(rootPath, "__generated_by_pydabs__.yml"))
if err != nil { if err != nil {
return dyn.InvalidValue, fmt.Errorf("failed to get absolute path: %w", err) return dyn.InvalidValue, diag.FromErr(fmt.Errorf("failed to get absolute path: %w", err))
} }
generated, err := yamlloader.LoadYAML(virtualPath, outputFile) generated, err := yamlloader.LoadYAML(virtualPath, outputFile)
if err != nil { if err != nil {
return dyn.InvalidValue, fmt.Errorf("failed to parse output file: %w", err) return dyn.InvalidValue, diag.FromErr(fmt.Errorf("failed to parse output file: %w", err))
} }
normalized, diagnostic := convert.Normalize(config.Root{}, generated) return strictNormalize(config.Root{}, generated)
if diagnostic.Error() != nil {
return dyn.InvalidValue, fmt.Errorf("failed to normalize output: %w", diagnostic.Error())
} }
func strictNormalize(dst any, generated dyn.Value) (dyn.Value, diag.Diagnostics) {
normalized, diags := convert.Normalize(dst, generated)
// warnings shouldn't happen because output should be already normalized // warnings shouldn't happen because output should be already normalized
// when it happens, it's a bug in the mutator, and should be treated as an error // when it happens, it's a bug in the mutator, and should be treated as an error
for _, d := range diagnostic.Filter(diag.Warning) { strictDiags := diag.Diagnostics{}
return dyn.InvalidValue, fmt.Errorf("failed to normalize output: %s", d.Summary)
for _, d := range diags {
if d.Severity == diag.Warning {
d.Severity = diag.Error
} }
return normalized, nil strictDiags = strictDiags.Append(d)
}
return normalized, strictDiags
} }
// loadDiagnosticsFile loads diagnostics from a file. // loadDiagnosticsFile loads diagnostics from a file.

View File

@ -10,6 +10,8 @@ import (
"runtime" "runtime"
"testing" "testing"
"github.com/databricks/cli/libs/dyn/convert"
"github.com/databricks/cli/libs/dyn/merge" "github.com/databricks/cli/libs/dyn/merge"
"github.com/databricks/cli/bundle/env" "github.com/databricks/cli/bundle/env"
@ -255,7 +257,7 @@ func TestPythonMutator_badOutput(t *testing.T) {
mutator := PythonMutator(PythonMutatorPhaseLoad) mutator := PythonMutator(PythonMutatorPhaseLoad)
diag := bundle.Apply(ctx, b, mutator) diag := bundle.Apply(ctx, b, mutator)
assert.EqualError(t, diag.Error(), "failed to load Python mutator output: failed to normalize output: unknown field: unknown_property") assert.EqualError(t, diag.Error(), "unknown field: unknown_property")
} }
func TestPythonMutator_disabled(t *testing.T) { func TestPythonMutator_disabled(t *testing.T) {
@ -546,6 +548,22 @@ func TestInterpreterPath(t *testing.T) {
} }
} }
func TestStrictNormalize(t *testing.T) {
// NB: there is no way to trigger diag.Error, so we don't test it
type TestStruct struct {
A int `json:"a"`
}
value := dyn.NewValue(map[string]dyn.Value{"A": dyn.NewValue("abc", nil)}, nil)
_, diags := convert.Normalize(TestStruct{}, value)
_, strictDiags := strictNormalize(TestStruct{}, value)
assert.False(t, diags.HasError())
assert.True(t, strictDiags.HasError())
}
func withProcessStub(t *testing.T, args []string, output string, diagnostics string) context.Context { func withProcessStub(t *testing.T, args []string, output string, diagnostics string) context.Context {
ctx := context.Background() ctx := context.Background()
ctx, stub := process.WithStub(ctx) ctx, stub := process.WithStub(ctx)