mirror of https://github.com/databricks/cli.git
PythonMutator: Fix relative path error (#2253)
## Changes Fix relative path errors in the Python mutator that was failing during deployment since v0.239.1. Before that: ``` % databricks bundle deploy Deploying resources... Updating deployment state... Error: failed to compute relative path for job jobs_as_code_project_job: Rel: can't make resources/jobs_as_code_project_job.py relative to /Users/$USER/jobs_as_code_project ``` As a result, the bundle was deployed, but the deployment state wasn't updated. ## Tests Unit tests, adding acceptance tests in https://github.com/databricks/cli/pull/2254
This commit is contained in:
parent
ec7808da34
commit
13596eb605
|
@ -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,
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -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,
|
||||
},
|
||||
|
|
Loading…
Reference in New Issue