steve/databricks-cli
steve
/
databricks-cli
mirror of https://github.com/databricks/cli.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert "Retain location metadata for values in `convert.FromTyped`" (#1528) 

## Changes

This reverts commit dac5f09556 (#1523).

Retaining the location for nil values means equality checks no longer
pass.

We need #1520 to be merged first.

## Tests

Integration test `TestAccPythonWheelTaskDeployAndRunWithWrapper`.
This commit is contained in:
Pieter Noordhuis 2024-06-26 11:26:40 +02:00 committed by GitHub
parent dac5f09556
commit 482d83cba8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 38 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
libs/dyn/convert/from_typed.go
View File

@ -42,7 +42,7 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
// Dereference pointer if necessary // Dereference pointer if necessary
for srcv.Kind() == reflect.Pointer { for srcv.Kind() == reflect.Pointer {
if srcv.IsNil() { if srcv.IsNil() {
return dyn.NilValue.WithLocation(ref.Location()), nil return dyn.NilValue, nil
} }
srcv = srcv.Elem() srcv = srcv.Elem()
@ -55,35 +55,27 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
} }
} }
var v dyn.Value
var err error
switch srcv.Kind() { switch srcv.Kind() {
case reflect.Struct: case reflect.Struct:
v, err = fromTypedStruct(srcv, ref, options...) return fromTypedStruct(srcv, ref, options...)
case reflect.Map: case reflect.Map:
v, err = fromTypedMap(srcv, ref) return fromTypedMap(srcv, ref)
case reflect.Slice: case reflect.Slice:
v, err = fromTypedSlice(srcv, ref) return fromTypedSlice(srcv, ref)
case reflect.String: case reflect.String:
v, err = fromTypedString(srcv, ref, options...) return fromTypedString(srcv, ref, options...)
case reflect.Bool: case reflect.Bool:
v, err = fromTypedBool(srcv, ref, options...) return fromTypedBool(srcv, ref, options...)
case reflect.Int, reflect.Int32, reflect.Int64: case reflect.Int, reflect.Int32, reflect.Int64:
v, err = fromTypedInt(srcv, ref, options...) return fromTypedInt(srcv, ref, options...)
case reflect.Float32, reflect.Float64: case reflect.Float32, reflect.Float64:
v, err = fromTypedFloat(srcv, ref, options...) return fromTypedFloat(srcv, ref, options...)
case reflect.Invalid: case reflect.Invalid:
// If the value is untyped and not set (e.g. any type with nil value), we return nil. // If the value is untyped and not set (e.g. any type with nil value), we return nil.
v, err = dyn.NilValue, nil return dyn.NilValue, nil
default:
return dyn.InvalidValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
} }
// Ensure the location metadata is retained. return dyn.InvalidValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
if err != nil {
return dyn.InvalidValue, err
}
return v.WithLocation(ref.Location()), err
} }
func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) { func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
@ -125,7 +117,7 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptio
// 2. The reference is a map (i.e. the struct was and still is empty). // 2. The reference is a map (i.e. the struct was and still is empty).
// 3. The "includeZeroValues" option is set (i.e. the struct is a non-nil pointer). // 3. The "includeZeroValues" option is set (i.e. the struct is a non-nil pointer).
if out.Len() > 0 || ref.Kind() == dyn.KindMap || slices.Contains(options, includeZeroValues) { if out.Len() > 0 || ref.Kind() == dyn.KindMap || slices.Contains(options, includeZeroValues) {
return dyn.V(out), nil return dyn.NewValue(out, ref.Location()), nil
} }
// Otherwise, return nil. // Otherwise, return nil.
@ -172,7 +164,7 @@ func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
out.Set(refk, nv) out.Set(refk, nv)
} }
return dyn.V(out), nil return dyn.NewValue(out, ref.Location()), nil
} }
func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) { func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
@ -207,7 +199,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
out[i] = nv out[i] = nv
} }
return dyn.V(out), nil return dyn.NewValue(out, ref.Location()), nil
} }
func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) { func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {

109
libs/dyn/convert/from_typed_test.go
View File

@ -49,7 +49,7 @@ func TestFromTypedStructPointerZeroFields(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, dyn.NilValue, nv) assert.Equal(t, dyn.NilValue, nv)
// For an initialized pointer with a nil reference we expect an empty map. // For an initialized pointer with a nil reference we expect a nil.
src = &Tmp{} src = &Tmp{}
nv, err = FromTyped(src, dyn.NilValue) nv, err = FromTyped(src, dyn.NilValue)
require.NoError(t, err) require.NoError(t, err)
@ -103,7 +103,7 @@ func TestFromTypedStructSetFields(t *testing.T) {
}), nv) }), nv)
} }
func TestFromTypedStructSetFieldsRetainLocation(t *testing.T) { func TestFromTypedStructSetFieldsRetainLocationIfUnchanged(t *testing.T) {
type Tmp struct { type Tmp struct {
Foo string `json:"foo"` Foo string `json:"foo"`
Bar string `json:"bar"` Bar string `json:"bar"`
@ -122,9 +122,11 @@ func TestFromTypedStructSetFieldsRetainLocation(t *testing.T) {
nv, err := FromTyped(src, ref) nv, err := FromTyped(src, ref)
require.NoError(t, err) require.NoError(t, err)
// Assert foo and bar have retained their location. // Assert foo has retained its location.
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv.Get("foo")) assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv.Get("foo"))
assert.Equal(t, dyn.NewValue("qux", dyn.Location{File: "bar"}), nv.Get("bar"))
// Assert bar lost its location (because it was overwritten).
assert.Equal(t, dyn.NewValue("qux", dyn.Location{}), nv.Get("bar"))
} }
func TestFromTypedStringMapWithZeroValue(t *testing.T) { func TestFromTypedStringMapWithZeroValue(t *testing.T) {
@ -352,7 +354,7 @@ func TestFromTypedMapNonEmpty(t *testing.T) {
}), nv) }), nv)
} }
func TestFromTypedMapNonEmptyRetainLocation(t *testing.T) { func TestFromTypedMapNonEmptyRetainLocationIfUnchanged(t *testing.T) {
var src = map[string]string{ var src = map[string]string{
"foo": "bar", "foo": "bar",
"bar": "qux", "bar": "qux",
@ -366,9 +368,11 @@ func TestFromTypedMapNonEmptyRetainLocation(t *testing.T) {
nv, err := FromTyped(src, ref) nv, err := FromTyped(src, ref)
require.NoError(t, err) require.NoError(t, err)
// Assert foo and bar have retained their locations. // Assert foo has retained its location.
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv.Get("foo")) assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv.Get("foo"))
assert.Equal(t, dyn.NewValue("qux", dyn.Location{File: "bar"}), nv.Get("bar"))
// Assert bar lost its location (because it was overwritten).
assert.Equal(t, dyn.NewValue("qux", dyn.Location{}), nv.Get("bar"))
} }
func TestFromTypedMapFieldWithZeroValue(t *testing.T) { func TestFromTypedMapFieldWithZeroValue(t *testing.T) {
@ -425,7 +429,7 @@ func TestFromTypedSliceNonEmpty(t *testing.T) {
}), nv) }), nv)
} }
func TestFromTypedSliceNonEmptyRetainLocation(t *testing.T) { func TestFromTypedSliceNonEmptyRetainLocationIfUnchanged(t *testing.T) {
var src = []string{ var src = []string{
"foo", "foo",
"bar", "bar",
@ -433,15 +437,17 @@ func TestFromTypedSliceNonEmptyRetainLocation(t *testing.T) {
ref := dyn.V([]dyn.Value{ ref := dyn.V([]dyn.Value{
dyn.NewValue("foo", dyn.Location{File: "foo"}), dyn.NewValue("foo", dyn.Location{File: "foo"}),
dyn.NewValue("bar", dyn.Location{File: "bar"}), dyn.NewValue("baz", dyn.Location{File: "baz"}),
}) })
nv, err := FromTyped(src, ref) nv, err := FromTyped(src, ref)
require.NoError(t, err) require.NoError(t, err)
// Assert foo and bar have retained their locations. // Assert foo has retained its location.
assert.Equal(t, dyn.NewValue("foo", dyn.Location{File: "foo"}), nv.Index(0)) assert.Equal(t, dyn.NewValue("foo", dyn.Location{File: "foo"}), nv.Index(0))
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "bar"}), nv.Index(1))
// Assert bar lost its location (because it was overwritten).
assert.Equal(t, dyn.NewValue("bar", dyn.Location{}), nv.Index(1))
} }
func TestFromTypedStringEmpty(t *testing.T) { func TestFromTypedStringEmpty(t *testing.T) {
@ -476,20 +482,12 @@ func TestFromTypedStringNonEmptyOverwrite(t *testing.T) {
assert.Equal(t, dyn.V("new"), nv) assert.Equal(t, dyn.V("new"), nv)
} }
func TestFromTypedStringRetainsLocations(t *testing.T) { func TestFromTypedStringRetainsLocationsIfUnchanged(t *testing.T) {
var ref = dyn.NewValue("foo", dyn.Location{File: "foo"})
// case: value has not been changed
var src string = "foo" var src string = "foo"
var ref = dyn.NewValue("foo", dyn.Location{File: "foo"})
nv, err := FromTyped(src, ref) nv, err := FromTyped(src, ref)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, dyn.NewValue("foo", dyn.Location{File: "foo"}), nv) assert.Equal(t, dyn.NewValue("foo", dyn.Location{File: "foo"}), nv)
// case: value has been changed
src = "bar"
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv)
} }
func TestFromTypedStringTypeError(t *testing.T) { func TestFromTypedStringTypeError(t *testing.T) {
@ -531,20 +529,12 @@ func TestFromTypedBoolNonEmptyOverwrite(t *testing.T) {
assert.Equal(t, dyn.V(true), nv) assert.Equal(t, dyn.V(true), nv)
} }
func TestFromTypedBoolRetainsLocations(t *testing.T) { func TestFromTypedBoolRetainsLocationsIfUnchanged(t *testing.T) {
var ref = dyn.NewValue(true, dyn.Location{File: "foo"})
// case: value has not been changed
var src bool = true var src bool = true
var ref = dyn.NewValue(true, dyn.Location{File: "foo"})
nv, err := FromTyped(src, ref) nv, err := FromTyped(src, ref)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, dyn.NewValue(true, dyn.Location{File: "foo"}), nv) assert.Equal(t, dyn.NewValue(true, dyn.Location{File: "foo"}), nv)
// case: value has been changed
src = false
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(false, dyn.Location{File: "foo"}), nv)
} }
func TestFromTypedBoolVariableReference(t *testing.T) { func TestFromTypedBoolVariableReference(t *testing.T) {
@ -594,20 +584,12 @@ func TestFromTypedIntNonEmptyOverwrite(t *testing.T) {
assert.Equal(t, dyn.V(int64(1234)), nv) assert.Equal(t, dyn.V(int64(1234)), nv)
} }
func TestFromTypedIntRetainsLocations(t *testing.T) { func TestFromTypedIntRetainsLocationsIfUnchanged(t *testing.T) {
var ref = dyn.NewValue(1234, dyn.Location{File: "foo"})
// case: value has not been changed
var src int = 1234 var src int = 1234
var ref = dyn.NewValue(1234, dyn.Location{File: "foo"})
nv, err := FromTyped(src, ref) nv, err := FromTyped(src, ref)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, dyn.NewValue(1234, dyn.Location{File: "foo"}), nv) assert.Equal(t, dyn.NewValue(1234, dyn.Location{File: "foo"}), nv)
// case: value has been changed
src = 1235
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(int64(1235), dyn.Location{File: "foo"}), nv)
} }
func TestFromTypedIntVariableReference(t *testing.T) { func TestFromTypedIntVariableReference(t *testing.T) {
@ -657,21 +639,12 @@ func TestFromTypedFloatNonEmptyOverwrite(t *testing.T) {
assert.Equal(t, dyn.V(1.23), nv) assert.Equal(t, dyn.V(1.23), nv)
} }
func TestFromTypedFloatRetainsLocations(t *testing.T) { func TestFromTypedFloatRetainsLocationsIfUnchanged(t *testing.T) {
var src float64 var src float64 = 1.23
var ref = dyn.NewValue(1.23, dyn.Location{File: "foo"}) var ref = dyn.NewValue(1.23, dyn.Location{File: "foo"})
// case: value has not been changed
src = 1.23
nv, err := FromTyped(src, ref) nv, err := FromTyped(src, ref)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, dyn.NewValue(1.23, dyn.Location{File: "foo"}), nv) assert.Equal(t, dyn.NewValue(1.23, dyn.Location{File: "foo"}), nv)
// case: value has been changed
src = 1.24
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(1.24, dyn.Location{File: "foo"}), nv)
} }
func TestFromTypedFloatVariableReference(t *testing.T) { func TestFromTypedFloatVariableReference(t *testing.T) {
@ -696,35 +669,3 @@ func TestFromTypedAnyNil(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, dyn.NilValue, nv) assert.Equal(t, dyn.NilValue, nv)
} }
func TestFromTypedNilPointerRetainsLocations(t *testing.T) {
type Tmp struct {
Foo string `json:"foo"`
Bar string `json:"bar"`
}
var src *Tmp
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
}
func TestFromTypedNilMapRetainsLocation(t *testing.T) {
var src map[string]string
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
}
func TestFromTypedNilSliceRetainsLocation(t *testing.T) {
var src []string
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
}