-
Notifications
You must be signed in to change notification settings - Fork 47
Fix #213 - Review DataInputSchema unmarshal function; review k8s annotations for Object type #214
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 #213 - Review DataInputSchema unmarshal function; review k8s annotations for Object type #214
Conversation
…n; review k8s annotations for Object type Signed-off-by: Ricardo Zanini <ricardozanini@gmail.com>
@ribeiromiranda can u please take a look? |
@@ -0,0 +1,53 @@ | |||
# Copyright 2024 The Serverless Workflow Specification Authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this file being used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is the new test case. There's a test case in which we go through each file in this folder and perform a parse.
@@ -581,17 +581,6 @@ func TestFromFile(t *testing.T) { | |||
assert.Equal(t, "SendTextForHighPriority", w.States[10].SwitchState.DefaultCondition.Transition.NextState) | |||
assert.Equal(t, true, w.States[10].End.Terminate) | |||
}, | |||
}, { | |||
"./testdata/workflows/dataInputSchemaValidation.yaml", func(t *testing.T, w *model.Workflow) { | |||
assert.NotNil(t, w.DataInputSchema) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't it better to keep it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No this validation is not valid anymore since Schema is not loading the external file to the attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comments.
@ribeiromiranda do you want to take another look before merging?
Many thanks for submitting your Pull Request ❤️!
What this PR does / why we need it:
Fix #213
Special notes for reviewers:
I also added the following annotations:
To
Metadata
andObject
attributes since we can't force the type object on Kubernetes context for this type. For example, in the schema use case, theDataInputSchema
can be a string.Additional information (if needed):