From 5ca30d6279ba554fc83a31634d0ce0a9470997bc Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Fri, 11 Oct 2024 21:03:07 +0800 Subject: [PATCH] [flytectl] Use Protobuf Struct as dataclass Input for backward compatibility Signed-off-by: Future-Outlier --- flyteadmin/go.mod | 1 - flytecopilot/go.mod | 1 - flytecopilot/go.sum | 2 - .../go/coreutils/extract_literal_test.go | 28 +++++++ flyteidl/clients/go/coreutils/literals.go | 34 ++------ .../clients/go/coreutils/literals_test.go | 77 +++++-------------- flyteidl/go.mod | 1 - flyteidl/go.sum | 2 - flyteplugins/go.mod | 1 - flyteplugins/go.sum | 2 - 10 files changed, 54 insertions(+), 95 deletions(-) diff --git a/flyteadmin/go.mod b/flyteadmin/go.mod index 25340ec208..852082add9 100644 --- a/flyteadmin/go.mod +++ b/flyteadmin/go.mod @@ -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 diff --git a/flytecopilot/go.mod b/flytecopilot/go.mod index f14e7fdef1..ba5f8fa6d5 100644 --- a/flytecopilot/go.mod +++ b/flytecopilot/go.mod @@ -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 diff --git a/flytecopilot/go.sum b/flytecopilot/go.sum index 6294fe32f3..9fc4e35767 100644 --- a/flytecopilot/go.sum +++ b/flytecopilot/go.sum @@ -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= diff --git a/flyteidl/clients/go/coreutils/extract_literal_test.go b/flyteidl/clients/go/coreutils/extract_literal_test.go index 67e27fb74f..66b20439c2 100644 --- a/flyteidl/clients/go/coreutils/extract_literal_test.go +++ b/flyteidl/clients/go/coreutils/extract_literal_test.go @@ -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}} diff --git a/flyteidl/clients/go/coreutils/literals.go b/flyteidl/clients/go/coreutils/literals.go index c0244c5190..2bb789b423 100644 --- a/flyteidl/clients/go/coreutils/literals.go +++ b/flyteidl/clients/go/coreutils/literals.go @@ -2,6 +2,7 @@ package coreutils import ( + "encoding/json" "fmt" "math" "reflect" @@ -9,14 +10,12 @@ import ( "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" @@ -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) diff --git a/flyteidl/clients/go/coreutils/literals_test.go b/flyteidl/clients/go/coreutils/literals_test.go index 009703bc1c..3b5daf4b27 100644 --- a/flyteidl/clients/go/coreutils/literals_test.go +++ b/flyteidl/clients/go/coreutils/literals_test.go @@ -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" @@ -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) { diff --git a/flyteidl/go.mod b/flyteidl/go.mod index fb8a99f912..5aa9ba2b15 100644 --- a/flyteidl/go.mod +++ b/flyteidl/go.mod @@ -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 diff --git a/flyteidl/go.sum b/flyteidl/go.sum index 740ca66bab..bf47d92a09 100644 --- a/flyteidl/go.sum +++ b/flyteidl/go.sum @@ -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= diff --git a/flyteplugins/go.mod b/flyteplugins/go.mod index c2eb504e04..e62eda562d 100644 --- a/flyteplugins/go.mod +++ b/flyteplugins/go.mod @@ -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 diff --git a/flyteplugins/go.sum b/flyteplugins/go.sum index ac78be7844..c8aa6c1254 100644 --- a/flyteplugins/go.sum +++ b/flyteplugins/go.sum @@ -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=