Skip to content

Commit

Permalink
Improve literal type string representation handling
Browse files Browse the repository at this point in the history
Signed-off-by: Kevin Su <[email protected]>
  • Loading branch information
pingsutw committed Oct 28, 2024
1 parent 13b3d82 commit ed3072b
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 12 deletions.
3 changes: 2 additions & 1 deletion flytepropeller/pkg/compiler/transformers/k8s/inputs.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package k8s

import (
"github.com/flyteorg/flyte/flytestdlib/utils"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
Expand Down Expand Up @@ -42,7 +43,7 @@ func validateInputs(nodeID common.NodeID, iface *core.TypedInterface, inputs cor
continue
}
if !validators.AreTypesCastable(inputType, v.Type) {
errs.Collect(errors.NewMismatchingTypesErr(nodeID, inputVar, v.Type.String(), inputType.String()))
errs.Collect(errors.NewMismatchingTypesErr(nodeID, inputVar, utils.LiteralTypeToStr(v.Type), utils.LiteralTypeToStr(inputType)))
continue
}

Expand Down
7 changes: 4 additions & 3 deletions flytepropeller/pkg/compiler/validators/bindings.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package validators

import (
"fmt"
"github.com/flyteorg/flyte/flytestdlib/utils"
"reflect"

"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -131,7 +132,7 @@ func validateBinding(w c.WorkflowBuilder, node c.Node, nodeParam string, binding
// If the variable has an index. We expect param to be a collection.
if v.Index != nil {
if cType := param.GetType().GetCollectionType(); cType == nil {
errs.Collect(errors.NewMismatchingVariablesErr(nodeID, outputVar, param.Type.String(), inputVar, expectedType.String()))
errs.Collect(errors.NewMismatchingVariablesErr(nodeID, outputVar, utils.LiteralTypeToStr(param.Type), inputVar, utils.LiteralTypeToStr(expectedType)))
} else {
sourceType = cType
}
Expand Down Expand Up @@ -164,7 +165,7 @@ func validateBinding(w c.WorkflowBuilder, node c.Node, nodeParam string, binding
return param.GetType(), []c.NodeID{val.Promise.NodeId}, true
}

errs.Collect(errors.NewMismatchingVariablesErr(node.GetId(), outputVar, sourceType.String(), inputVar, expectedType.String()))
errs.Collect(errors.NewMismatchingVariablesErr(node.GetId(), outputVar, utils.LiteralTypeToStr(sourceType), inputVar, utils.LiteralTypeToStr(expectedType)))
return nil, nil, !errs.HasErrors()
}
}
Expand All @@ -180,7 +181,7 @@ func validateBinding(w c.WorkflowBuilder, node c.Node, nodeParam string, binding
if literalType == nil {
errs.Collect(errors.NewUnrecognizedValueErr(nodeID, reflect.TypeOf(val.Scalar.GetValue()).String()))
} else if validateParamTypes && !AreTypesCastable(literalType, expectedType) {
errs.Collect(errors.NewMismatchingTypesErr(nodeID, nodeParam, literalType.String(), expectedType.String()))
errs.Collect(errors.NewMismatchingTypesErr(nodeID, nodeParam, utils.LiteralTypeToStr(literalType), utils.LiteralTypeToStr(expectedType)))
}

if expectedType.GetEnumType() != nil {
Expand Down
3 changes: 2 additions & 1 deletion flytepropeller/pkg/compiler/validators/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package validators

import (
"fmt"
"github.com/flyteorg/flyte/flytestdlib/utils"

flyte "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
c "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/common"
Expand Down Expand Up @@ -44,7 +45,7 @@ func ValidateBooleanExpression(w c.WorkflowBuilder, node c.NodeBuilder, expr *fl
if op1Valid && op2Valid && op1Type != nil && op2Type != nil {
if op1Type.String() != op2Type.String() {
errs.Collect(errors.NewMismatchingTypesErr(node.GetId(), "RightValue",
op1Type.String(), op2Type.String()))
utils.LiteralTypeToStr(op1Type), utils.LiteralTypeToStr(op2Type)))
}
}
} else if expr.GetConjunction() != nil {
Expand Down
3 changes: 2 additions & 1 deletion flytepropeller/pkg/compiler/validators/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
flyte "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
c "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/common"
"github.com/flyteorg/flyte/flytepropeller/pkg/compiler/errors"
"github.com/flyteorg/flyte/flytestdlib/utils"
)

func validateOutputVar(n c.NodeBuilder, paramName string, errs errors.CompileErrors) (
Expand Down Expand Up @@ -40,7 +41,7 @@ func validateInputVar(n c.NodeBuilder, paramName string, requireParamType bool,
func validateVarType(nodeID c.NodeID, paramName string, param *flyte.Variable,
expectedType *flyte.LiteralType, errs errors.CompileErrors) (ok bool) {
if param.GetType().String() != expectedType.String() {
errs.Collect(errors.NewMismatchingTypesErr(nodeID, paramName, param.GetType().String(), expectedType.String()))
errs.Collect(errors.NewMismatchingTypesErr(nodeID, paramName, utils.LiteralTypeToStr(param.GetType()), utils.LiteralTypeToStr(expectedType)))
}

return !errs.HasErrors()
Expand Down
5 changes: 3 additions & 2 deletions flytestdlib/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/ernesto-jimenez/gogen v0.0.0-20180125220232-d7d4131e6607
github.com/fatih/color v1.13.0
github.com/fatih/structtag v1.2.0
github.com/flyteorg/flyte/flyteidl v0.0.0-00010101000000-000000000000
github.com/flyteorg/stow v0.3.10
github.com/fsnotify/fsnotify v1.6.0
github.com/ghodss/yaml v1.0.0
Expand All @@ -21,7 +22,7 @@ require (
github.com/magiconair/properties v1.8.6
github.com/mitchellh/mapstructure v1.5.0
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.19.1-0.20240620110541-bccd68204bf4
github.com/prometheus/client_golang v1.19.1
github.com/prometheus/common v0.53.0
github.com/sirupsen/logrus v1.9.3
github.com/spf13/cobra v1.7.0
Expand Down Expand Up @@ -99,7 +100,6 @@ require (
github.com/jmespath/go-jmespath v0.4.0 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/klauspost/compress v1.17.8 // indirect
github.com/kylelemons/godebug v1.1.0 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
github.com/mattn/go-colorable v0.1.12 // indirect
Expand Down Expand Up @@ -154,6 +154,7 @@ require (
replace (
github.com/flyteorg/flyte/datacatalog => ../datacatalog
github.com/flyteorg/flyte/flyteadmin => ../flyteadmin
github.com/flyteorg/flyte/flyteidl => ../flyteidl
github.com/flyteorg/flyte/flyteplugins => ../flyteplugins
github.com/flyteorg/flyte/flytepropeller => ../flytepropeller
github.com/flyteorg/flyte/flytestdlib => ../flytestdlib
Expand Down
6 changes: 2 additions & 4 deletions flytestdlib/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,6 @@ github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1
github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk=
github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
github.com/klauspost/compress v1.17.8 h1:YcnTYrq7MikUT7k0Yb5eceMmALQPYBW/Xltxn0NAMnU=
github.com/klauspost/compress v1.17.8/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw=
github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg=
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
Expand Down Expand Up @@ -332,8 +330,8 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE
github.com/pkg/sftp v1.13.1/go.mod h1:3HaPG6Dq1ILlpPZRO0HVMrsydcdLt6HRDccSgb87qRg=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/prometheus/client_golang v1.19.1-0.20240620110541-bccd68204bf4 h1:GCiMVi+gRj5QaXuw8Gkz71k8US0ilrLJmoG/mp5+8dI=
github.com/prometheus/client_golang v1.19.1-0.20240620110541-bccd68204bf4/go.mod h1:JJCmTHsrwjUPYl5HyuWSzf8ZNGQzncCeuj37Rby0GzI=
github.com/prometheus/client_golang v1.19.1 h1:wZWJDwK+NameRJuPGDhlnFgx8e8HN3XHQeLaYJFJBOE=
github.com/prometheus/client_golang v1.19.1/go.mod h1:mP78NwGzrVks5S2H6ab8+ZZGJLZUq1hoULYBAYBw1Ho=
github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/prometheus/client_model v0.6.1 h1:ZKSh/rekM+n3CeS952MLRAdFwIKqeY8b62p8ais2e9E=
github.com/prometheus/client_model v0.6.1/go.mod h1:OrxVMOVHjw3lKMa8+x6HeMGkHMQyHDk9E3jmP2AmGiY=
Expand Down
22 changes: 22 additions & 0 deletions flytestdlib/utils/pretty_print.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package utils

import (
"fmt"
"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
"strings"
)

func LiteralTypeToStr(lt *core.LiteralType) string {
if lt == nil {
return "None"
}
if lt.GetSimple() == core.SimpleType_STRUCT {
var structure string
for k, v := range lt.GetStructure().GetDataclassType() {
structure += fmt.Sprintf("dataclass_type:{key:%v value:{%v}, ", k, LiteralTypeToStr(v))
}
structure = strings.TrimSuffix(structure, ", ")
return fmt.Sprintf("Simple: STRUCT structure{%v}", structure)
}
return lt.String()
}
21 changes: 21 additions & 0 deletions flytestdlib/utils/pretty_print_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package utils

import (
"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
"github.com/stretchr/testify/assert"
"testing"
)

func TestLiteralTypeToStr(t *testing.T) {
assert.Equal(t, LiteralTypeToStr(nil), "None")
assert.Equal(t, LiteralTypeToStr(&core.LiteralType{
Type: &core.LiteralType_Simple{Simple: core.SimpleType_STRUCT},
Structure: &core.TypeStructure{
DataclassType: map[string]*core.LiteralType{
"a": {
Type: &core.LiteralType_Simple{Simple: core.SimpleType_INTEGER},
},
},
},
}), "Simple: STRUCT structure{dataclass_type:{key:a value:{simple:INTEGER}}")
}

0 comments on commit ed3072b

Please sign in to comment.