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

Fix Union type with dataclass ambiguous error and support superset comparison #5858

Merged
merged 30 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
f06cdc6
feat: fix Union type with dataclass ambiguous error
mao3267 Oct 18, 2024
8660db5
Merge branch 'master' of https://github.com/mao3267/flyte into fix/#5…
mao3267 Nov 1, 2024
47ccbd1
fix: direct json comparison for superset
mao3267 Nov 1, 2024
85489dc
fix: go.mod missing entry for error
mao3267 Nov 1, 2024
cc685bb
fix: update go module and sum
mao3267 Nov 1, 2024
3a629e1
refactor: gci format
mao3267 Nov 1, 2024
aa4d98e
test: add dataset casting tests for same (one/two levels) and supers…
mao3267 Nov 1, 2024
b282e5f
Merge branch 'master' of https://github.com/mao3267/flyte into fix/#5…
mao3267 Nov 8, 2024
818afb7
fix: support Pydantic BaseModel comparison
mao3267 Nov 8, 2024
d6468b6
fix: handle nested pydantic basemodel
mao3267 Nov 8, 2024
ada05ed
Reviews from Eduardo
Future-Outlier Nov 11, 2024
56623e3
fix: support strict subset match
mao3267 Nov 15, 2024
b8f38a7
test: update strict subset match test
mao3267 Nov 15, 2024
b698769
fix: missing go mod entry
mao3267 Nov 15, 2024
70ad767
fix: missing go mod entry
mao3267 Nov 15, 2024
9dc3fa6
fix: go mod entry
mao3267 Nov 15, 2024
b224a02
make go-tidy
Future-Outlier Nov 15, 2024
7ed9be2
comments
Future-Outlier Nov 15, 2024
6fe8871
Merge branch 'fix/#5489-dataclass-mismatch' of https://github.com/mao…
mao3267 Nov 15, 2024
64343c8
fix: strict subset match with draft 2020-12 mashumaro
mao3267 Nov 18, 2024
8ccced5
Merge branch 'master' of https://github.com/mao3267/flyte into fix/#5…
mao3267 Nov 18, 2024
0def0ad
refactor: make go-tidy
mao3267 Nov 18, 2024
81445a7
fix: support strict subset match with ambiguity
mao3267 Nov 19, 2024
9b19f04
fix: change test name and fix err
mao3267 Nov 19, 2024
30aa096
Add comments
Future-Outlier Nov 20, 2024
e62ba6e
nit
Future-Outlier Nov 20, 2024
c6ac729
add flytectl go-tidy in makefile
Future-Outlier Nov 20, 2024
ba4d6f1
nit
Future-Outlier Nov 20, 2024
86a395e
fix: add comment for error checking
mao3267 Nov 20, 2024
7f28c35
test: basemodel castable test, two level dataclass and ParentToChild …
mao3267 Nov 20, 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
4 changes: 3 additions & 1 deletion .run/single-binary.run.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
<package value="github.com/flyteorg/flyte/cmd" />
<directory value="$PROJECT_DIR$" />
<filePath value="$PROJECT_DIR$" />
<method v="2" />
<method v="2">
<option name="RunConfigurationTask" enabled="true" run_configuration_name="make cmd/single/dist" run_configuration_type="MAKEFILE_TARGET_RUN_CONFIGURATION" />
</method>
</configuration>
</component>
6 changes: 3 additions & 3 deletions flyteadmin/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ require (
github.com/spf13/cobra v1.7.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.9.0
github.com/wI2L/jsondiff v0.5.0
github.com/wI2L/jsondiff v0.6.0
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.47.0
go.opentelemetry.io/otel v1.24.0
golang.org/x/net v0.27.0
Expand Down Expand Up @@ -176,9 +176,9 @@ 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/tidwall/gjson v1.17.0 // indirect
github.com/tidwall/gjson v1.17.1 // indirect
github.com/tidwall/match v1.1.1 // indirect
github.com/tidwall/pretty v1.2.0 // indirect
github.com/tidwall/pretty v1.2.1 // indirect
github.com/tidwall/sjson v1.2.5 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.46.1 // indirect
Expand Down
11 changes: 6 additions & 5 deletions flyteadmin/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1271,17 +1271,18 @@ github.com/tidwall/gjson v1.3.2/go.mod h1:P256ACg0Mn+j1RXIDXoss50DeIABTYK1PULOJH
github.com/tidwall/gjson v1.6.8/go.mod h1:zeFuBCIqD4sN/gmqBzZ4j7Jd6UcA2Fc56x7QFsv+8fI=
github.com/tidwall/gjson v1.7.1/go.mod h1:5/xDoumyyDNerp2U36lyolv46b3uF/9Bu6OfyQ9GImk=
github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.17.0 h1:/Jocvlh98kcTfpN2+JzGQWQcqrPQwDrVEMApx/M5ZwM=
github.com/tidwall/gjson v1.17.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.17.1 h1:wlYEnwqAHgzmhNUFfw7Xalt2JzQvsMx2Se4PcoFCT/U=
github.com/tidwall/gjson v1.17.1/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/match v1.0.1/go.mod h1:LujAq0jyVjBy028G1WhWfIzbpQfMO8bBZ6Tyb0+pL9E=
github.com/tidwall/match v1.0.3/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
github.com/tidwall/pretty v1.0.2/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
github.com/tidwall/pretty v1.1.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs=
github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4=
github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
github.com/tidwall/sjson v1.0.4/go.mod h1:bURseu1nuBkFpIES5cz6zBtjmYeOQmEESshn7VpF15Y=
github.com/tidwall/sjson v1.1.5/go.mod h1:VuJzsZnTowhSxWdOgsAnb886i4AjEyTkk7tNtsL7EYE=
github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY=
Expand All @@ -1302,8 +1303,8 @@ 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/wI2L/jsondiff v0.5.0 h1:RRMTi/mH+R2aXcPe1VYyvGINJqQfC3R+KSEakuU1Ikw=
github.com/wI2L/jsondiff v0.5.0/go.mod h1:qqG6hnK0Lsrz2BpIVCxWiK9ItsBCpIZQiv0izJjOZ9s=
github.com/wI2L/jsondiff v0.6.0 h1:zrsH3FbfVa3JO9llxrcDy/XLkYPLgoMX6Mz3T2PP2AI=
github.com/wI2L/jsondiff v0.6.0/go.mod h1:D6aQ5gKgPF9g17j+E9N7aasmU1O+XvfmWm1y8UMmNpw=
github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c/go.mod h1:lB8K/P019DLNhemzwFU4jHLhdvlE6uDZjXFejJXr49I=
github.com/xdg/stringprep v0.0.0-20180714160509-73f8eece6fdc/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y=
github.com/xdg/stringprep v1.0.0/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y=
Expand Down
6 changes: 5 additions & 1 deletion flytectl/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,17 @@ require (
github.com/prometheus/procfs v0.15.1 // indirect
github.com/rivo/uniseg v0.4.7 // indirect
github.com/russross/blackfriday/v2 v2.1.0 // indirect
github.com/shamaton/msgpack/v2 v2.2.2 // indirect
github.com/spf13/afero v1.9.2 // indirect
github.com/spf13/cast v1.4.1 // indirect
github.com/spf13/jwalterweatherman v1.1.0 // indirect
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/tidwall/gjson v1.17.1 // indirect
github.com/tidwall/match v1.1.1 // indirect
github.com/tidwall/pretty v1.2.1 // indirect
github.com/tidwall/sjson v1.2.5 // indirect
github.com/wI2L/jsondiff v0.6.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
12 changes: 12 additions & 0 deletions flytectl/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,18 @@ 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/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.17.1 h1:wlYEnwqAHgzmhNUFfw7Xalt2JzQvsMx2Se4PcoFCT/U=
github.com/tidwall/gjson v1.17.1/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4=
github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY=
github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28=
github.com/wI2L/jsondiff v0.6.0 h1:zrsH3FbfVa3JO9llxrcDy/XLkYPLgoMX6Mz3T2PP2AI=
github.com/wI2L/jsondiff v0.6.0/go.mod h1:D6aQ5gKgPF9g17j+E9N7aasmU1O+XvfmWm1y8UMmNpw=
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
5 changes: 5 additions & 0 deletions flytepropeller/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ require (
github.com/spf13/cobra v1.7.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.9.0
github.com/wI2L/jsondiff v0.6.0
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.47.0
go.opentelemetry.io/otel v1.24.0
go.opentelemetry.io/otel/trace v1.24.0
Expand Down Expand Up @@ -124,6 +125,10 @@ 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/tidwall/gjson v1.17.1 // indirect
github.com/tidwall/match v1.1.1 // indirect
github.com/tidwall/pretty v1.2.1 // indirect
github.com/tidwall/sjson v1.2.5 // 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
12 changes: 12 additions & 0 deletions flytepropeller/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,18 @@ 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/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.17.1 h1:wlYEnwqAHgzmhNUFfw7Xalt2JzQvsMx2Se4PcoFCT/U=
github.com/tidwall/gjson v1.17.1/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4=
github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY=
github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28=
github.com/wI2L/jsondiff v0.6.0 h1:zrsH3FbfVa3JO9llxrcDy/XLkYPLgoMX6Mz3T2PP2AI=
github.com/wI2L/jsondiff v0.6.0/go.mod h1:D6aQ5gKgPF9g17j+E9N7aasmU1O+XvfmWm1y8UMmNpw=
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
135 changes: 135 additions & 0 deletions flytepropeller/pkg/compiler/validators/typing.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package validators

import (
"encoding/json"
"strings"

structpb "github.com/golang/protobuf/ptypes/struct"
"github.com/wI2L/jsondiff"

flyte "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
)
Expand All @@ -16,6 +18,128 @@
literalType *flyte.LiteralType
}

func removeTitleFieldFromProperties(schema map[string]*structpb.Value) {
// TODO: Explain why we need this
// TODO: givse me example about dataclass vs. Pydantic BaseModel
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an example comparing dataclass and Pydantic BaseModel. As shown, the schema for dataclass includes a title field that records the name of the class. Additionally, the additionalProperties field is absent from the Pydantic BaseModel schema because its value is false. cc @eapolinario

dataclass Pydantic.BaseModel
image image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To add the comment, writing the entire schema would make it too lengthy. Would it be acceptable to use something like this instead?

class A:
	a: int

Pydantic.BaseModel: 	{"properties": {"a": {"title": "A", "type": "integer"}}}
dataclass: 			{"properties": {"a": {"type": "integer"}}, "additionalProperties": false}

Copy link
Member

@fg91 fg91 Nov 12, 2024

Choose a reason for hiding this comment

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

Are you proposing to preprocess the schemas so that one can mix and match dataclasses and base models given their schemas are aligned? I.e. task expects a dataclass with schema "A" and I pass a base model that has the same schema.

I personally feel this is not necessary and think it would be totally acceptable to consider a dataclass and a base model not a match by default. Especially if this makes things a lot more complicated in the backend otherwise because the schemas need to be aligned. What do you think about this?

If you are confident in the logic I'm of course not opposing the feature but if you feel this makes things complicated and brittle, I'd rather keep it simple and more robust.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it actually make things more complicated, will remove related logic.

properties, ok := schema["properties"]
if !ok {
return
}

Check warning on line 27 in flytepropeller/pkg/compiler/validators/typing.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L26-L27

Added lines #L26 - L27 were not covered by tests

for _, p := range properties.GetStructValue().Fields {
if _, ok := p.GetStructValue().Fields["properties"]; ok {
removeTitleFieldFromProperties(p.GetStructValue().Fields)
}
delete(p.GetStructValue().Fields, "title")
}
}

func resolveRef(schema, defs map[string]*structpb.Value) {
// Schema from Pydantic BaseModel includes a $def field, which is a reference to the actual schema.
// We need to resolve the reference to compare the schema with those from marshumaro.
// https://github.com/flyteorg/flytekit/blob/3475ddc41f2ba31d23dd072362be704d7c2470a0/flytekit/core/type_engine.py#L632-L641
for _, p := range schema["properties"].GetStructValue().Fields {
if _, ok := p.GetStructValue().Fields["$ref"]; ok {
propName := strings.TrimPrefix(p.GetStructValue().Fields["$ref"].GetStringValue(), "#/$defs/")
p.GetStructValue().Fields = defs[propName].GetStructValue().Fields
resolveRef(p.GetStructValue().Fields, defs)
delete(p.GetStructValue().Fields, "$ref")
}

Check warning on line 47 in flytepropeller/pkg/compiler/validators/typing.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L37-L47

Added lines #L37 - L47 were not covered by tests
}

delete(schema, "$defs")

Check warning on line 50 in flytepropeller/pkg/compiler/validators/typing.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L50

Added line #L50 was not covered by tests
}

func isSuperTypeInJSON(sourceMetaData, targetMetaData *structpb.Struct) bool {
// Since there are lots of field differences between draft-07 and draft 2020-12,
// we only support json schema with 2020-12 draft, which is generated here: https://github.com/flyteorg/flytekit/blob/ff2d0da686c82266db4dbf764a009896cf062349/flytekit/core/type_engine.py#L630-L639
_, upstreamIsDraft7 := sourceMetaData.Fields["$schema"]
_, downstreamIsDraft7 := targetMetaData.Fields["$schema"]

// We only support super type check for draft 2020-12
if upstreamIsDraft7 || downstreamIsDraft7 {
return false
}

Check warning on line 62 in flytepropeller/pkg/compiler/validators/typing.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L61-L62

Added lines #L61 - L62 were not covered by tests

copySrcSchema := make(map[string]*structpb.Value)
copyTgtSchema := make(map[string]*structpb.Value)

for k, v := range sourceMetaData.Fields {
copySrcSchema[k] = v
}

for k, v := range targetMetaData.Fields {
copyTgtSchema[k] = v
}

// For nested Pydantic BaseModel, we need to resolve the reference to compare the schema.
if _, ok := copySrcSchema["$defs"]; ok {
resolveRef(copySrcSchema, copySrcSchema["$defs"].GetStructValue().Fields)
}

Check warning on line 78 in flytepropeller/pkg/compiler/validators/typing.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L77-L78

Added lines #L77 - L78 were not covered by tests
if _, ok := copyTgtSchema["$defs"]; ok {
resolveRef(copyTgtSchema, copyTgtSchema["$defs"].GetStructValue().Fields)
}

Check warning on line 81 in flytepropeller/pkg/compiler/validators/typing.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L80-L81

Added lines #L80 - L81 were not covered by tests
// The JSON schema generated by Pydantic.BaseModel includes a title field in its properties, repeatedly recording the property name.
// Since this title field is absent in the JSON schema generated for dataclass, we need to remove the title field from the properties to ensure equivalence.
removeTitleFieldFromProperties(copySrcSchema)
removeTitleFieldFromProperties(copyTgtSchema)

srcSchemaBytes, _ := json.Marshal(copySrcSchema)
tgtSchemaBytes, _ := json.Marshal(copyTgtSchema)

patch, _ := jsondiff.CompareJSON(srcSchemaBytes, tgtSchemaBytes)
for _, p := range patch {
// If additionalProperties is false, the field is not present in the schema from Pydantic.BaseModel.
// We handle this case by checking the relationships by ourselves.
if p.Type != jsondiff.OperationAdd && strings.Contains(p.Path, "additionalProperties") {
if p.Type == jsondiff.OperationRemove || p.Type == jsondiff.OperationReplace {
if p.OldValue != false {
return false
}

Check warning on line 98 in flytepropeller/pkg/compiler/validators/typing.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L95-L98

Added lines #L95 - L98 were not covered by tests
}
} else if p.Type != jsondiff.OperationAdd {
return false

Check warning on line 101 in flytepropeller/pkg/compiler/validators/typing.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L101

Added line #L101 was not covered by tests
} else if strings.Contains(p.Path, "required") {
return false
}

Check warning on line 104 in flytepropeller/pkg/compiler/validators/typing.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L103-L104

Added lines #L103 - L104 were not covered by tests
}
return true
}

func isSameTypeInJSON(sourceMetaData, targetMetaData *structpb.Struct) bool {
// Since there are lots of field differences between draft-07 and draft 2020-12,
// we only support json schema with 2020-12 draft, which is generated here: https://github.com/flyteorg/flytekit/blob/ff2d0da686c82266db4dbf764a009896cf062349/flytekit/core/type_engine.py#L630-L639
_, upstreamIsDraft7 := sourceMetaData.Fields["$schema"]
_, downstreamIsDraft7 := targetMetaData.Fields["$schema"]

// If the schema version is different, we can't compare them.
if upstreamIsDraft7 != downstreamIsDraft7 {
return false
}

Check warning on line 118 in flytepropeller/pkg/compiler/validators/typing.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L117-L118

Added lines #L117 - L118 were not covered by tests

copySrcSchema := make(map[string]*structpb.Value)
copyTgtSchema := make(map[string]*structpb.Value)

for k, v := range sourceMetaData.Fields {
copySrcSchema[k] = v
}

for k, v := range targetMetaData.Fields {
copyTgtSchema[k] = v
}

// The JSON schema generated by Pydantic.BaseModel includes a title field in its properties, repeatedly recording the property name.
// Since this title field is absent in the JSON schema generated for dataclass, we need to remove the title field from the properties to ensure equivalence.
removeTitleFieldFromProperties(copySrcSchema)
removeTitleFieldFromProperties(copyTgtSchema)

srcSchemaBytes, _ := json.Marshal(copySrcSchema)
tgtSchemaBytes, _ := json.Marshal(copyTgtSchema)

patch, _ := jsondiff.CompareJSON(srcSchemaBytes, tgtSchemaBytes)
return len(patch) == 0
}

// CastsFrom is a trivial type checker merely checks if types match exactly.
func (t trivialChecker) CastsFrom(upstreamType *flyte.LiteralType) bool {
// If upstream is an enum, it can be consumed as a string downstream
Expand All @@ -35,6 +159,17 @@
return false
}

if upstreamType.GetSimple() == flyte.SimpleType_STRUCT && t.literalType.GetSimple() == flyte.SimpleType_STRUCT {
upstreamMetaData := upstreamType.GetMetadata()
downstreamMetaData := t.literalType.GetMetadata()

if upstreamMetaData == nil || downstreamMetaData == nil {
return true
}

return isSameTypeInJSON(upstreamMetaData, downstreamMetaData) || isSuperTypeInJSON(upstreamMetaData, downstreamMetaData)
}

// Ignore metadata when comparing types.
upstreamTypeCopy := *upstreamType
downstreamTypeCopy := *t.literalType
Expand Down
Loading
Loading