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.

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

JSON IDL #5542

Closed
wants to merge 42 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
d26c228
proto
wild-endeavor May 8, 2024
a5d515d
make generate
Future-Outlier Aug 6, 2024
707a013
new swagger
Future-Outlier Aug 6, 2024
5d85d16
update test
Future-Outlier Jul 23, 2024
fa8bbd5
add all json scalar things need to be implemented, need to add tests …
Future-Outlier Jul 23, 2024
a503b8b
support upstream STRUCT downstream JSON case
Future-Outlier Aug 5, 2024
0b32845
make idl correct back
Future-Outlier Aug 6, 2024
1dcfbc4
support attribute access
Future-Outlier Aug 7, 2024
d5d7b79
fix attribute access for nested dict/dataclass
Future-Outlier Aug 8, 2024
c7c82ac
Create Execution Success, need to implement get Execution
Future-Outlier Aug 8, 2024
00d6e2b
remove print
Future-Outlier Aug 8, 2024
a005a2d
Use scalarValue.Json.GetValue()
Future-Outlier Aug 8, 2024
7ece699
fix flyteadmin tests
Future-Outlier Aug 9, 2024
8c0ab5c
Merge branch 'master' into idl/json-type-bytes-and-attribute-access
Future-Outlier Aug 26, 2024
b3543e5
regenerate proto
Future-Outlier Aug 26, 2024
cb2876c
propeller error msg example
Future-Outlier Aug 27, 2024
aa68c42
Merge branch 'master' into idl/json-type-bytes-and-attribute-access
Future-Outlier Aug 27, 2024
ded1f7c
solve conlfict
Future-Outlier Aug 27, 2024
62b7aa4
solve lint and depedency error
Future-Outlier Aug 27, 2024
e44d309
make go-tidy
Future-Outlier Aug 27, 2024
b099d06
add tests for flyteidl/clients/go/coreutils/literals.go
Future-Outlier Aug 28, 2024
df99f28
add tests for flytepropeller/pkg/compiler/validators
Future-Outlier Aug 28, 2024
9fe07af
add tests for attr_path_resolver
Future-Outlier Aug 28, 2024
5e2b35d
add tests for attr_path_resolver with nearly 100% coverage
Future-Outlier Aug 28, 2024
77ea3ed
lint
Future-Outlier Aug 28, 2024
e5af3a3
nit
Future-Outlier Aug 28, 2024
3f155d6
split convertLiteral to convertStructToLiteral and convertJSONToLiteral
Future-Outlier Aug 29, 2024
19bac6a
Merge branch 'master' into idl/json-type-bytes-and-attribute-access
Future-Outlier Aug 29, 2024
bade905
add comments for convertNumbers
Future-Outlier Aug 30, 2024
4e3c28f
Update pingsu's advice
Future-Outlier Aug 30, 2024
b523b9d
Merge branch 'master' into idl/json-type-bytes-and-attribute-access
Future-Outlier Aug 30, 2024
2cf54c3
update idl
Future-Outlier Aug 30, 2024
1b0edc8
update attribute resolver
Future-Outlier Sep 5, 2024
fa8cca2
remove func convertNumbers
Future-Outlier Sep 8, 2024
a56d56a
specify msgpack as JSON format
Future-Outlier Sep 8, 2024
5adafcc
add tests for attr_path_resolver
Future-Outlier Sep 8, 2024
cf95f40
add coreutils for flytectl
Future-Outlier Sep 8, 2024
1631a94
update the IDL
Future-Outlier Sep 9, 2024
6f07cdd
add serialization format in flytepropeller and flytectl
Future-Outlier Sep 9, 2024
44da0cd
support SerializationFormat: msgpack in coreutils/literals.go MakeDef…
Future-Outlier Sep 9, 2024
2a12ce2
fix propeller's test
Future-Outlier Sep 9, 2024
27dbc63
fix coreutils test
Future-Outlier Sep 9, 2024
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
2 changes: 2 additions & 0 deletions flyteadmin/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ require (
github.com/tidwall/match v1.1.1 // indirect
github.com/tidwall/pretty v1.2.0 // indirect
github.com/tidwall/sjson v1.2.5 // indirect
github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.46.1 // indirect
go.opentelemetry.io/otel/exporters/jaeger v1.17.0 // indirect
Expand Down
4 changes: 4 additions & 0 deletions flyteadmin/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,10 @@ github.com/unrolled/secure v0.0.0-20181005190816-ff9db2ff917f/go.mod h1:mnPT77IA
github.com/urfave/negroni v1.0.0/go.mod h1:Meg73S6kFm/4PpbYdq35yYWoCZ9mS/YSx+lKnmiohz4=
github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw=
github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc=
github.com/vmihailenco/msgpack/v5 v5.4.1 h1:cQriyiUvjTwOHg8QZaPihLWeRAAVoCpE00IUPn0Bjt8=
github.com/vmihailenco/msgpack/v5 v5.4.1/go.mod h1:GaZTsDaehaPpQVyxrf5mtQlH+pc21PIudVV/E3rRQok=
github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g=
github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds=
github.com/wI2L/jsondiff v0.5.0 h1:RRMTi/mH+R2aXcPe1VYyvGINJqQfC3R+KSEakuU1Ikw=
github.com/wI2L/jsondiff v0.5.0/go.mod h1:qqG6hnK0Lsrz2BpIVCxWiK9ItsBCpIZQiv0izJjOZ9s=
github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c/go.mod h1:lB8K/P019DLNhemzwFU4jHLhdvlE6uDZjXFejJXr49I=
Expand Down
2 changes: 2 additions & 0 deletions flytecopilot/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ require (
github.com/spf13/jwalterweatherman v1.1.0 // indirect
github.com/spf13/viper v1.11.0 // indirect
github.com/subosito/gotenv v1.2.0 // indirect
github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.46.1 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.46.1 // indirect
Expand Down
4 changes: 4 additions & 0 deletions flytecopilot/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,10 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s=
github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
github.com/vmihailenco/msgpack/v5 v5.4.1 h1:cQriyiUvjTwOHg8QZaPihLWeRAAVoCpE00IUPn0Bjt8=
github.com/vmihailenco/msgpack/v5 v5.4.1/go.mod h1:GaZTsDaehaPpQVyxrf5mtQlH+pc21PIudVV/E3rRQok=
github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g=
github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds=
github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Expand Down
2 changes: 2 additions & 0 deletions flytectl/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ require (
github.com/spf13/viper v1.11.0 // indirect
github.com/stretchr/objx v0.5.2 // indirect
github.com/subosito/gotenv v1.2.0 // indirect
github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.47.0 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.46.1 // indirect
Expand Down
4 changes: 4 additions & 0 deletions flytectl/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,10 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s=
github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
github.com/vmihailenco/msgpack/v5 v5.4.1 h1:cQriyiUvjTwOHg8QZaPihLWeRAAVoCpE00IUPn0Bjt8=
github.com/vmihailenco/msgpack/v5 v5.4.1/go.mod h1:GaZTsDaehaPpQVyxrf5mtQlH+pc21PIudVV/E3rRQok=
github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g=
github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds=
github.com/yalp/jsonpath v0.0.0-20180802001716-5cc68e5049a0 h1:6fRhSjgLCkTD3JnJxvaJ4Sj+TYblw757bqYgZaOq5ZY=
github.com/yalp/jsonpath v0.0.0-20180802001716-5cc68e5049a0/go.mod h1:/LWChgwKmvncFJFHJ7Gvn9wZArjbV5/FppcK2fKk/tI=
github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Expand Down
53 changes: 43 additions & 10 deletions flyteidl/clients/go/assets/admin.swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions flyteidl/clients/go/coreutils/extract_literal.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ func ExtractFromLiteral(literal *core.Literal) (interface{}, error) {
return scalarValue.Generic, nil
case *core.Scalar_StructuredDataset:
return scalarValue.StructuredDataset.Uri, nil
case *core.Scalar_Json:
return scalarValue.Json.GetValue(), nil
Comment on lines +65 to +66
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is covered by flyteidl/clients/go/coreutils/literals.go

case *core.Scalar_Union:
// extract the value of the union but not the actual union object
extractedVal, err := ExtractFromLiteral(scalarValue.Union.Value)
Expand Down
47 changes: 44 additions & 3 deletions flyteidl/clients/go/coreutils/literals.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
"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/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
"github.com/flyteorg/flyte/flytestdlib/storage"
"github.com/vmihailenco/msgpack/v5"
)

func MakePrimitive(v interface{}) (*core.Primitive, error) {
Expand Down Expand Up @@ -250,6 +250,19 @@
},
},
}, nil
case core.SimpleType_JSON:
return &core.Literal{
Value: &core.Literal_Scalar{
Scalar: &core.Scalar{
Value: &core.Scalar_Json{
Json: &core.Json{
Value: []byte(""),
SerializationFormat: "msgpack",
},
},
},
},
}, nil
}
return nil, errors.Errorf("Not yet implemented. Default creation is not yet implemented for [%s] ", t.Simple.String())
case *core.LiteralType_Blob:
Expand Down Expand Up @@ -385,6 +398,13 @@
scalar.Value = &core.Scalar_Generic{
Generic: st,
}
case core.SimpleType_JSON:
scalar.Value = &core.Scalar_Json{
Json: &core.Json{
Value: []byte(s),
SerializationFormat: "msgpack",
},

Check warning on line 406 in flyteidl/clients/go/coreutils/literals.go

View check run for this annotation

Codecov / codecov/patch

flyteidl/clients/go/coreutils/literals.go#L401-L406

Added lines #L401 - L406 were not covered by tests
}
case core.SimpleType_BINARY:
scalar.Value = &core.Scalar_Binary{
Binary: &core.Binary{
Expand Down Expand Up @@ -567,10 +587,31 @@
strValue = string(byteValue)
}
}
if newT.Simple == core.SimpleType_JSON {
// Return literal value here because this is the most efficient way to serialize json type
if _, isValueStringType := v.(string); !isValueStringType {
msgpackBytes, err := msgpack.Marshal(v)
if err != nil {
return nil, fmt.Errorf("unable to marshal to msgpack bytes for value %v: %w", v, err)

Check warning on line 595 in flyteidl/clients/go/coreutils/literals.go

View check run for this annotation

Codecov / codecov/patch

flyteidl/clients/go/coreutils/literals.go#L595

Added line #L595 was not covered by tests
}
l.Value = &core.Literal_Scalar{
Scalar: &core.Scalar{
Value: &core.Scalar_Json{
Json: &core.Json{
Value: msgpackBytes,
SerializationFormat: "msgpack",
},
},
},
}
return l, nil
}
}
lv, err := MakeLiteralForSimpleType(newT.Simple, strValue)
if err != nil {
return nil, err
}

return lv, nil

case *core.LiteralType_Blob:
Expand Down
71 changes: 71 additions & 0 deletions flyteidl/clients/go/coreutils/literals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"testing"
"time"

"github.com/vmihailenco/msgpack/v5"

"github.com/go-test/deep"
"github.com/golang/protobuf/ptypes"
structpb "github.com/golang/protobuf/ptypes/struct"
Expand Down Expand Up @@ -250,6 +252,16 @@ func TestMakeDefaultLiteralForType(t *testing.T) {
assert.NotNil(t, l.GetScalar().GetGeneric())
})

t.Run("json", func(t *testing.T) {
l, err := MakeDefaultLiteralForType(&core.LiteralType{Type: &core.LiteralType_Simple{
Simple: core.SimpleType_JSON,
}})
assert.NoError(t, err)
assert.NotNil(t, l.GetScalar().GetJson())
assert.NotNil(t, l.GetScalar().GetJson().GetValue())
assert.NotNil(t, l.GetScalar().GetJson().GetSerializationFormat())
})

t.Run("enum", func(t *testing.T) {
l, err := MakeDefaultLiteralForType(&core.LiteralType{Type: &core.LiteralType_EnumType{
EnumType: &core.EnumType{Values: []string{"x", "y", "z"}},
Expand Down Expand Up @@ -444,6 +456,65 @@ func TestMakeLiteralForType(t *testing.T) {
assert.Equal(t, expectedVal, actualVal)
})

t.Run("SimpleJson", 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_JSON}}
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,
},
pingsutw marked this conversation as resolved.
Show resolved Hide resolved
"f": []string{"a", "b", "c"},
}

val, err := MakeLiteralForType(literalType, v)
assert.NoError(t, err)

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

literalVal := &core.Literal{
Value: &core.Literal_Scalar{
Scalar: &core.Scalar{
Value: &core.Scalar_Json{
Json: &core.Json{
Value: msgpackBytes,
SerializationFormat: "msgpack",
},
},
},
},
}

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

expectedBytes, ok := expectedLiteralVal.([]byte)
assert.True(t, ok, "expectedLiteralVal is not of type []byte")
actualBytes, ok := actualLiteralVal.([]byte)
assert.True(t, ok, "actualLiteralVal is not of type []byte")

var expectedVal, actualVal map[string]interface{}
err = msgpack.Unmarshal(expectedBytes, &expectedVal)
assert.NoError(t, err)
err = msgpack.Unmarshal(actualBytes, &actualVal)
assert.NoError(t, err)

assert.Equal(t, expectedVal, actualVal)
})

t.Run("ArrayStrings", func(t *testing.T) {
var literalType = &core.LiteralType{Type: &core.LiteralType_CollectionType{
CollectionType: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: core.SimpleType_STRING}}}}
Expand Down
Loading
Loading