Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PythonMutator: Fix relative path error #2253

Merged
merged 6 commits into from
Jan 29, 2025
+36 −8
Merged

PythonMutator: Fix relative path error #2253

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0dd8d21
PythonMutator: Fix relative path error
kanterov Jan 28, 2025
354d7db
Fix tests
kanterov Jan 29, 2025
e018627
Fix tests
kanterov Jan 29, 2025
775c0cf
Address CR comments
kanterov Jan 29, 2025
f845329
Clean absolute paths
kanterov Jan 29, 2025
d33ef35
Fix tests
kanterov Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion bundle/config/mutator/python/python_locations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"io"
pathlib "path"
"path/filepath"

"github.com/databricks/cli/libs/dyn"
Expand Down Expand Up @@ -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()

Expand All @@ -116,6 +117,10 @@ func parsePythonLocations(input io.Reader) (*pythonLocations, error) {
return nil, fmt.Errorf("failed to parse python location: %s", err)
}

if !pathlib.IsAbs(entry.File) {
kanterov marked this conversation as resolved.
Show resolved Hide resolved
entry.File = filepath.Join(bundleRoot, entry.File)
}

location := dyn.Location{
File: entry.File,
Line: entry.Line,
Expand Down
4 changes: 2 additions & 2 deletions bundle/config/mutator/python/python_locations_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,11 @@ func TestLoadOutput(t *testing.T) {
}

func TestParsePythonLocations(t *testing.T) {
expected := dyn.Location{File: "foo.py", Line: 1, Column: 2}
expected := dyn.Location{File: "/tmp/my_project/foo.py", Line: 1, Column: 2}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can run filepath.Clean to make is platform native.


input := `{"path": "foo", "file": "foo.py", "line": 1, "column": 2}`
reader := bytes.NewReader([]byte(input))
locations, err := parsePythonLocations(reader)
locations, err := parsePythonLocations("/tmp/my_project", reader)

assert.NoError(t, err)

Expand Down
6 changes: 3 additions & 3 deletions bundle/config/mutator/python/python_mutator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down
7 changes: 6 additions & 1 deletion bundle/config/mutator/python/python_mutator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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{
Expand Down Expand Up @@ -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,
},
Expand Down
Loading