Skip to content

Commit

Permalink
Add comments
Browse files Browse the repository at this point in the history
Signed-off-by: Future-Outlier <[email protected]>
  • Loading branch information
Future-Outlier committed Nov 20, 2024
1 parent 9b19f04 commit 30aa096
Showing 1 changed file with 24 additions and 2 deletions.
26 changes: 24 additions & 2 deletions flytepropeller/pkg/compiler/validators/typing.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package validators

import (
"bytes"
"context"
"encoding/json"
"github.com/flyteorg/flyte/flytestdlib/logger"
"strings"

structpb "github.com/golang/protobuf/ptypes/struct"
Expand Down Expand Up @@ -47,6 +49,7 @@ func isSuperTypeInJSON(sourceMetaData, targetMetaData *structpb.Struct) bool {
errs := jscmp.Compare(tgtSchema, srcSchema)

// Ignore not implemented errors in json-schema-compare (additionalProperties)
// TODO: Explain why we use for loop here to check error msg
for _, err := range errs {
if !strings.Contains(err.Error(), "not implemented") {
return false
Expand All @@ -57,13 +60,26 @@ func isSuperTypeInJSON(sourceMetaData, targetMetaData *structpb.Struct) bool {
}

func isSameTypeInJSON(sourceMetaData, targetMetaData *structpb.Struct) bool {
srcSchemaBytes, _ := json.Marshal(sourceMetaData.GetFields())
tgtSchemaBytes, _ := json.Marshal(targetMetaData.GetFields())
srcSchemaBytes, err := json.Marshal(sourceMetaData.GetFields())
if err != nil {
logger.Infof(context.Background(), "Failed to marshal source metadata: %v", err)
return false
}

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

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L65-L67

Added lines #L65 - L67 were not covered by tests

tgtSchemaBytes, err := json.Marshal(targetMetaData.GetFields())
if err != nil {
logger.Infof(context.Background(), "Failed to marshal target metadata: %v", err)
return false
}

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

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/compiler/validators/typing.go#L71-L73

Added lines #L71 - L73 were not covered by tests

// Use jsondiff to compare the two schemas
patch, err := jsondiff.CompareJSON(srcSchemaBytes, tgtSchemaBytes)
if err != nil {
logger.Infof(context.Background(), "Failed to compare JSON schemas: %v", err)
return false
}

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

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L78 - L80 were not covered by tests

// If the length of the patch is zero, the two JSON structures are identical
return len(patch) == 0
}

Expand All @@ -86,10 +102,16 @@ func (t trivialChecker) CastsFrom(upstreamType *flyte.LiteralType) bool {
return false
}

// Related Issue: https://github.com/flyteorg/flyte/issues/5489
// RFC: https://github.com/flyteorg/flyte/blob/master/rfc/system/5741-binary-idl-with-message-pack.md#flytepropeller
if upstreamType.GetSimple() == flyte.SimpleType_STRUCT && t.literalType.GetSimple() == flyte.SimpleType_STRUCT {
// Json Schema is stored in Metadata
upstreamMetaData := upstreamType.GetMetadata()
downstreamMetaData := t.literalType.GetMetadata()

// There's bug in flytekit's dataclass Transformer to generate JSON Scheam before,
// in some case, we the JSON Schema will be nil, so we can only pass it to support
// backward compatible. (reference task should be supported.)
if upstreamMetaData == nil || downstreamMetaData == nil {
return true
}
Expand Down

0 comments on commit 30aa096

Please sign in to comment.