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

[flytectl] Use Protobuf Struct as dataclass Input for backward compatibility #5840

Merged
merged 1 commit into from
Oct 15, 2024
Merged

[flytectl] Use Protobuf Struct as dataclass Input for backward compatibility #5840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion flyteadmin/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ require (
github.com/prometheus/procfs v0.10.1 // indirect
github.com/rcrowley/go-metrics v0.0.0-20200313005456-10cdbea86bc0 // indirect
github.com/sendgrid/rest v2.6.9+incompatible // indirect
github.com/shamaton/msgpack/v2 v2.2.2 // indirect
github.com/sirupsen/logrus v1.9.3 // indirect
github.com/spf13/afero v1.8.2 // indirect
github.com/spf13/cast v1.4.1 // indirect
Expand Down
1 change: 0 additions & 1 deletion flytecopilot/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ require (
github.com/prometheus/client_model v0.4.0 // indirect
github.com/prometheus/common v0.44.0 // indirect
github.com/prometheus/procfs v0.10.1 // indirect
github.com/shamaton/msgpack/v2 v2.2.2 // indirect
github.com/sirupsen/logrus v1.9.3 // indirect
github.com/spf13/afero v1.8.2 // indirect
github.com/spf13/cast v1.4.1 // indirect
Expand Down
2 changes: 0 additions & 2 deletions flytecopilot/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,6 @@ github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFR
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/shamaton/msgpack/v2 v2.2.2 h1:GOIg0c9LV04VwzOOqZSrmsv/JzjNOOMxnS/HvOHGdgs=
github.com/shamaton/msgpack/v2 v2.2.2/go.mod h1:6khjYnkx73f7VQU7wjcFS9DFjs+59naVWJv1TB7qdOI=
github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72 h1:qLC7fQah7D6K1B0ujays3HV9gkFtllcxhzImRR7ArPQ=
Expand Down
28 changes: 28 additions & 0 deletions flyteidl/clients/go/coreutils/extract_literal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,34 @@ func TestFetchLiteral(t *testing.T) {
assert.Nil(t, err)
})

t.Run("Generic", func(t *testing.T) {
literalVal := map[string]interface{}{
"x": 1,
"y": "ystringvalue",
}
var literalType = &core.LiteralType{Type: &core.LiteralType_Simple{Simple: core.SimpleType_STRUCT}}
lit, err := MakeLiteralForType(literalType, literalVal)
assert.NoError(t, err)
extractedLiteralVal, err := ExtractFromLiteral(lit)
assert.NoError(t, err)
fieldsMap := map[string]*structpb.Value{
"x": {
Kind: &structpb.Value_NumberValue{NumberValue: 1},
},
"y": {
Kind: &structpb.Value_StringValue{StringValue: "ystringvalue"},
},
}
expectedStructVal := &structpb.Struct{
Fields: fieldsMap,
}
extractedStructValue := extractedLiteralVal.(*structpb.Struct)
assert.Equal(t, len(expectedStructVal.Fields), len(extractedStructValue.Fields))
for key, val := range expectedStructVal.Fields {
assert.Equal(t, val.Kind, extractedStructValue.Fields[key].Kind)
}
})

t.Run("Generic Passed As String", func(t *testing.T) {
literalVal := "{\"x\": 1,\"y\": \"ystringvalue\"}"
var literalType = &core.LiteralType{Type: &core.LiteralType_Simple{Simple: core.SimpleType_STRUCT}}
Expand Down
34 changes: 5 additions & 29 deletions flyteidl/clients/go/coreutils/literals.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,20 @@
package coreutils

import (
"encoding/json"
"fmt"
"math"
"reflect"
"strconv"
"strings"
"time"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
"github.com/flyteorg/flyte/flytestdlib/storage"
"github.com/golang/protobuf/jsonpb"
"github.com/golang/protobuf/ptypes"
structpb "github.com/golang/protobuf/ptypes/struct"
"github.com/pkg/errors"
"github.com/shamaton/msgpack/v2"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
"github.com/flyteorg/flyte/flytestdlib/storage"
)

const MESSAGEPACK = "msgpack"
Expand Down Expand Up @@ -562,35 +561,12 @@ func MakeLiteralForType(t *core.LiteralType, v interface{}) (*core.Literal, erro
strValue = fmt.Sprintf("%.0f", math.Trunc(f))
}
if newT.Simple == core.SimpleType_STRUCT {
// If the type is a STRUCT, we expect the input to be a complex object
// like the following example:
// inputs:
// dc:
// a: 1
// b: 3.14
// c: "example_string"
// Instead of storing it directly as a structured value, we will serialize
// the input object using MsgPack and return it as a binary IDL object.

// If the value is not already a string (meaning it's not already serialized),
// proceed with serialization.
if _, isValueStringType := v.(string); !isValueStringType {
byteValue, err := msgpack.Marshal(v)
byteValue, err := json.Marshal(v)
if err != nil {
return nil, fmt.Errorf("unable to marshal to json string for struct value %v", v)
}
return &core.Literal{
Value: &core.Literal_Scalar{
Scalar: &core.Scalar{
Value: &core.Scalar_Binary{
Binary: &core.Binary{
Value: byteValue,
Tag: MESSAGEPACK,
},
},
},
},
}, nil
strValue = string(byteValue)
}
}
lv, err := MakeLiteralForSimpleType(newT.Simple, strValue)
Expand Down
77 changes: 21 additions & 56 deletions flyteidl/clients/go/coreutils/literals_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/golang/protobuf/ptypes"
structpb "github.com/golang/protobuf/ptypes/struct"
"github.com/pkg/errors"
"github.com/shamaton/msgpack/v2"
"github.com/stretchr/testify/assert"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
Expand Down Expand Up @@ -455,66 +454,32 @@ func TestMakeLiteralForType(t *testing.T) {
assert.Equal(t, expectedVal, actualVal)
})

t.Run("SimpleBinary", func(t *testing.T) {
// We compare the deserialized values instead of the raw msgpack bytes because Go does not guarantee the order
// of map keys during serialization. This means that while the serialized bytes may differ, the deserialized
// values should be logically equivalent.

var literalType = &core.LiteralType{Type: &core.LiteralType_Simple{Simple: core.SimpleType_STRUCT}}
v := map[string]interface{}{
"a": int64(1),
"b": 3.14,
"c": "example_string",
"d": map[string]interface{}{
"1": int64(100),
"2": int64(200),
},
"e": map[string]interface{}{
"a": int64(1),
"b": 3.14,
},
"f": []string{"a", "b", "c"},
t.Run("Generic", func(t *testing.T) {
literalVal := map[string]interface{}{
"x": 1,
"y": "ystringvalue",
}

val, err := MakeLiteralForType(literalType, v)
var literalType = &core.LiteralType{Type: &core.LiteralType_Simple{Simple: core.SimpleType_STRUCT}}
lit, err := MakeLiteralForType(literalType, literalVal)
assert.NoError(t, err)

msgpackBytes, err := msgpack.Marshal(v)
extractedLiteralVal, err := ExtractFromLiteral(lit)
assert.NoError(t, err)

literalVal := &core.Literal{
Value: &core.Literal_Scalar{
Scalar: &core.Scalar{
Value: &core.Scalar_Binary{
Binary: &core.Binary{
Value: msgpackBytes,
Tag: MESSAGEPACK,
},
},
},
fieldsMap := map[string]*structpb.Value{
"x": {
Kind: &structpb.Value_NumberValue{NumberValue: 1},
},
"y": {
Kind: &structpb.Value_StringValue{StringValue: "ystringvalue"},
},
}

expectedLiteralVal, err := ExtractFromLiteral(literalVal)
assert.NoError(t, err)
actualLiteralVal, err := ExtractFromLiteral(val)
assert.NoError(t, err)

// Check if the extracted value is of type *core.Binary (not []byte)
expectedBinary, ok := expectedLiteralVal.(*core.Binary)
assert.True(t, ok, "expectedLiteralVal is not of type *core.Binary")
actualBinary, ok := actualLiteralVal.(*core.Binary)
assert.True(t, ok, "actualLiteralVal is not of type *core.Binary")

// Now check if the Binary values match
var expectedVal, actualVal map[string]interface{}
err = msgpack.Unmarshal(expectedBinary.Value, &expectedVal)
assert.NoError(t, err)
err = msgpack.Unmarshal(actualBinary.Value, &actualVal)
assert.NoError(t, err)

// Finally, assert that the deserialized values are equal
assert.Equal(t, expectedVal, actualVal)
expectedStructVal := &structpb.Struct{
Fields: fieldsMap,
}
extractedStructValue := extractedLiteralVal.(*structpb.Struct)
assert.Equal(t, len(expectedStructVal.Fields), len(extractedStructValue.Fields))
for key, val := range expectedStructVal.Fields {
assert.Equal(t, val.Kind, extractedStructValue.Fields[key].Kind)
}
})

t.Run("ArrayStrings", func(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion flyteidl/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ require (
github.com/mitchellh/mapstructure v1.5.0
github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c
github.com/pkg/errors v0.9.1
github.com/shamaton/msgpack/v2 v2.2.2
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.9.0
golang.org/x/net v0.27.0
Expand Down
2 changes: 0 additions & 2 deletions flyteidl/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,6 @@ github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPH
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/shamaton/msgpack/v2 v2.2.2 h1:GOIg0c9LV04VwzOOqZSrmsv/JzjNOOMxnS/HvOHGdgs=
github.com/shamaton/msgpack/v2 v2.2.2/go.mod h1:6khjYnkx73f7VQU7wjcFS9DFjs+59naVWJv1TB7qdOI=
github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
Expand Down
1 change: 0 additions & 1 deletion flyteplugins/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ require (
github.com/prometheus/client_model v0.4.0 // indirect
github.com/prometheus/common v0.44.0 // indirect
github.com/prometheus/procfs v0.10.1 // indirect
github.com/shamaton/msgpack/v2 v2.2.2 // indirect
github.com/sirupsen/logrus v1.9.3 // indirect
github.com/spf13/afero v1.8.2 // indirect
github.com/spf13/cast v1.4.1 // indirect
Expand Down
2 changes: 0 additions & 2 deletions flyteplugins/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,6 @@ github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFR
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/shamaton/msgpack/v2 v2.2.2 h1:GOIg0c9LV04VwzOOqZSrmsv/JzjNOOMxnS/HvOHGdgs=
github.com/shamaton/msgpack/v2 v2.2.2/go.mod h1:6khjYnkx73f7VQU7wjcFS9DFjs+59naVWJv1TB7qdOI=
github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72 h1:qLC7fQah7D6K1B0ujays3HV9gkFtllcxhzImRR7ArPQ=
Expand Down
Loading