From b04cba32ace5f69e1497443c2d876141544743d5 Mon Sep 17 00:00:00 2001 From: Vincent Chen <62143443+mao3267@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:17:03 +0800 Subject: [PATCH] [Fix] Add logger for compiler and marshal while comparing union (#6034) * feat: fix Union type with dataclass ambiguous error Signed-off-by: mao3267 * fix: direct json comparison for superset Signed-off-by: mao3267 * fix: go.mod missing entry for error Signed-off-by: mao3267 * fix: update go module and sum Signed-off-by: mao3267 * refactor: gci format Signed-off-by: mao3267 * test: add dataset casting tests for same (one/two levels) and superset (one level) dataclass Signed-off-by: mao3267 * fix: support Pydantic BaseModel comparison Signed-off-by: mao3267 * fix: handle nested pydantic basemodel Signed-off-by: mao3267 * Reviews from Eduardo Signed-off-by: Future-Outlier * fix: support strict subset match Signed-off-by: mao3267 * test: update strict subset match test Signed-off-by: mao3267 * fix: missing go mod entry Signed-off-by: mao3267 * fix: missing go mod entry Signed-off-by: mao3267 * fix: go mod entry Signed-off-by: mao3267 * make go-tidy Signed-off-by: Future-Outlier * comments Signed-off-by: Future-Outlier * fix: strict subset match with draft 2020-12 mashumaro Signed-off-by: mao3267 * refactor: make go-tidy Signed-off-by: mao3267 * fix: support strict subset match with ambiguity Signed-off-by: mao3267 * fix: change test name and fix err Signed-off-by: mao3267 * Add comments Signed-off-by: Future-Outlier * nit Signed-off-by: Future-Outlier * add flytectl go-tidy in makefile Signed-off-by: Future-Outlier * nit Signed-off-by: Future-Outlier * fix: add comment for error checking Signed-off-by: mao3267 * test: basemodel castable test, two level dataclass and ParentToChild failure Signed-off-by: mao3267 * fix: add logger for jsonschema compiler Signed-off-by: mao3267 * fix: add logger for marshal and compiler Signed-off-by: mao3267 * better error msg format Signed-off-by: Future-Outlier --------- Signed-off-by: mao3267 Signed-off-by: Future-Outlier Co-authored-by: Future-Outlier Signed-off-by: SZL741023 --- .../pkg/compiler/validators/typing.go | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/flytepropeller/pkg/compiler/validators/typing.go b/flytepropeller/pkg/compiler/validators/typing.go index 5f8812a602..ca1ca03148 100644 --- a/flytepropeller/pkg/compiler/validators/typing.go +++ b/flytepropeller/pkg/compiler/validators/typing.go @@ -28,22 +28,40 @@ func isSuperTypeInJSON(sourceMetaData, targetMetaData *structpb.Struct) bool { // For custom types, we expect the JSON schemas in the metadata to come from the same JSON schema package, // specifically draft 2020-12 from Mashumaro. - 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 + } + tgtSchemaBytes, err := json.Marshal(targetMetaData.GetFields()) + if err != nil { + logger.Infof(context.Background(), "Failed to marshal target metadata: [%v]", err) + return false + } compiler := jsonschema.NewCompiler() - err := compiler.AddResource("src", bytes.NewReader(srcSchemaBytes)) + err = compiler.AddResource("src", bytes.NewReader(srcSchemaBytes)) if err != nil { + logger.Infof(context.Background(), "Failed to add resource to compiler: [%v]", err) return false } err = compiler.AddResource("tgt", bytes.NewReader(tgtSchemaBytes)) if err != nil { + logger.Infof(context.Background(), "Failed to add resource to compiler: [%v]", err) return false } - srcSchema, _ := compiler.Compile("src") - tgtSchema, _ := compiler.Compile("tgt") + srcSchema, err := compiler.Compile("src") + if err != nil { + logger.Infof(context.Background(), "Failed to compile source schema: [%v]", err) + return false + } + tgtSchema, err := compiler.Compile("tgt") + if err != nil { + logger.Infof(context.Background(), "Failed to compile target schema: [%v]", err) + return false + } // Compare the two schemas errs := jscmp.Compare(tgtSchema, srcSchema) @@ -63,20 +81,20 @@ func isSuperTypeInJSON(sourceMetaData, targetMetaData *structpb.Struct) bool { func isSameTypeInJSON(sourceMetaData, targetMetaData *structpb.Struct) bool { srcSchemaBytes, err := json.Marshal(sourceMetaData.GetFields()) if err != nil { - logger.Infof(context.Background(), "Failed to marshal source metadata: %v", err) + logger.Infof(context.Background(), "Failed to marshal source metadata: [%v]", err) return false } tgtSchemaBytes, err := json.Marshal(targetMetaData.GetFields()) if err != nil { - logger.Infof(context.Background(), "Failed to marshal target metadata: %v", err) + logger.Infof(context.Background(), "Failed to marshal target metadata: [%v]", err) return false } // 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) + logger.Infof(context.Background(), "Failed to compare JSON schemas: [%v]", err) return false }