Skip to content

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

Merged

Conversation

ricardozanini
Copy link
Member

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:

	// +kubebuilder:validation:Schemaless
	// +kubebuilder:pruning:PreserveUnknownFields

To Metadata and Object attributes since we can't force the type object on Kubernetes context for this type. For example, in the schema use case, the DataInputSchema can be a string.

Additional information (if needed):

…n; review k8s annotations for Object type

Signed-off-by: Ricardo Zanini <ricardozanini@gmail.com>
@ricardozanini
Copy link
Member Author

@ribeiromiranda can u please take a look?

@@ -0,0 +1,53 @@
# Copyright 2024 The Serverless Workflow Specification Authors
Copy link
Member

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?

Copy link
Member Author

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)
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

@spolti spolti left a 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?

@ricardozanini ricardozanini merged commit 0b585d7 into serverlessworkflow:main Oct 11, 2024
5 checks passed
@ricardozanini ricardozanini deleted the issue-schema-string branch October 11, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parser should not load DataInputSchema external files
2 participants