diff --git a/bundle/config/mutator/python/python_locations.go b/bundle/config/mutator/python/python_locations.go index 2fa86bea0..9cb65c302 100644 --- a/bundle/config/mutator/python/python_locations.go +++ b/bundle/config/mutator/python/python_locations.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "io" + pathlib "path" "path/filepath" "github.com/databricks/cli/libs/dyn" @@ -99,7 +100,7 @@ func removeVirtualLocations(locations []dyn.Location) []dyn.Location { // parsePythonLocations parses locations.json from the Python mutator. // // locations file is newline-separated JSON objects with pythonLocationEntry structure. -func parsePythonLocations(input io.Reader) (*pythonLocations, error) { +func parsePythonLocations(bundleRoot string, input io.Reader) (*pythonLocations, error) { decoder := json.NewDecoder(input) locations := newPythonLocations() @@ -116,6 +117,12 @@ func parsePythonLocations(input io.Reader) (*pythonLocations, error) { return nil, fmt.Errorf("failed to parse python location: %s", err) } + // Output can contain both relative paths and absolute paths outside of bundle root. + // Mutator pipeline expects all path to be absolute at this point, so make all paths absolute. + if !pathlib.IsAbs(entry.File) { + entry.File = filepath.Join(bundleRoot, entry.File) + } + location := dyn.Location{ File: entry.File, Line: entry.Line, diff --git a/bundle/config/mutator/python/python_locations_test.go b/bundle/config/mutator/python/python_locations_test.go index 32afcc92b..2860af820 100644 --- a/bundle/config/mutator/python/python_locations_test.go +++ b/bundle/config/mutator/python/python_locations_test.go @@ -165,12 +165,28 @@ func TestLoadOutput(t *testing.T) { require.Equal(t, filepath.Join(bundleRoot, generatedFileName), notebookPath.Locations()[0].File) } -func TestParsePythonLocations(t *testing.T) { - expected := dyn.Location{File: "foo.py", Line: 1, Column: 2} +func TestParsePythonLocations_absolutePath(t *testing.T) { + // output can contain absolute path that is outside of the bundle root + expected := dyn.Location{File: "/Shared/foo.py", Line: 1, Column: 2} - input := `{"path": "foo", "file": "foo.py", "line": 1, "column": 2}` + input := `{"path": "foo", "file": "/Shared/foo.py", "line": 1, "column": 2}` reader := bytes.NewReader([]byte(input)) - locations, err := parsePythonLocations(reader) + locations, err := parsePythonLocations("/tmp/", reader) + + assert.NoError(t, err) + + assert.True(t, locations.keys["foo"].exists) + assert.Equal(t, expected, locations.keys["foo"].location) +} + +func TestParsePythonLocations_relativePath(t *testing.T) { + // output can contain relative paths, we expect all locations to be absolute + // at this stage of mutator pipeline + expected := dyn.Location{File: filepath.Clean("/tmp/my_project/foo.py"), Line: 1, Column: 2} + + input := `{"path": "foo", "file": "foo.py", "line": 1, "column": 2}` + reader := bytes.NewReader([]byte(input)) + locations, err := parsePythonLocations(filepath.Clean("/tmp/my_project"), reader) assert.NoError(t, err) diff --git a/bundle/config/mutator/python/python_mutator.go b/bundle/config/mutator/python/python_mutator.go index cd2e286e5..f75f111cf 100644 --- a/bundle/config/mutator/python/python_mutator.go +++ b/bundle/config/mutator/python/python_mutator.go @@ -331,7 +331,7 @@ func (m *pythonMutator) runPythonMutator(ctx context.Context, root dyn.Value, op return dyn.InvalidValue, diag.Errorf("failed to load diagnostics: %s", pythonDiagnosticsErr) } - locations, err := loadLocationsFile(locationsPath) + locations, err := loadLocationsFile(opts.bundleRootPath, locationsPath) if err != nil { return dyn.InvalidValue, diag.Errorf("failed to load locations: %s", err) } @@ -381,7 +381,7 @@ func writeInputFile(inputPath string, input dyn.Value) error { } // loadLocationsFile loads locations.json containing source locations for generated YAML. -func loadLocationsFile(locationsPath string) (*pythonLocations, error) { +func loadLocationsFile(bundleRoot, locationsPath string) (*pythonLocations, error) { locationsFile, err := os.Open(locationsPath) if errors.Is(err, fs.ErrNotExist) { return newPythonLocations(), nil @@ -391,7 +391,7 @@ func loadLocationsFile(locationsPath string) (*pythonLocations, error) { defer locationsFile.Close() - return parsePythonLocations(locationsFile) + return parsePythonLocations(bundleRoot, locationsFile) } func loadOutputFile(rootPath, outputPath string, locations *pythonLocations) (dyn.Value, diag.Diagnostics) { diff --git a/bundle/config/mutator/python/python_mutator_test.go b/bundle/config/mutator/python/python_mutator_test.go index 322fb79e8..9d957e797 100644 --- a/bundle/config/mutator/python/python_mutator_test.go +++ b/bundle/config/mutator/python/python_mutator_test.go @@ -54,6 +54,8 @@ func TestPythonMutator_Name_applyMutators(t *testing.T) { func TestPythonMutator_loadResources(t *testing.T) { withFakeVEnv(t, ".venv") + rootPath := filepath.Join(t.TempDir(), "my_project") + b := loadYaml("databricks.yml", ` experimental: python: @@ -64,6 +66,9 @@ func TestPythonMutator_loadResources(t *testing.T) { job0: name: job_0`) + // set rootPath so that we can make absolute paths in dyn.Location + b.BundleRootPath = rootPath + ctx := withProcessStub( t, []string{ @@ -120,7 +125,7 @@ func TestPythonMutator_loadResources(t *testing.T) { assert.Equal(t, []dyn.Location{ { - File: "src/examples/job1.py", + File: filepath.Join(rootPath, "src/examples/job1.py"), Line: 5, Column: 7, },