sdk-go icon indicating copy to clipboard operation
sdk-go copied to clipboard

adding validate to flows against schema

Open brlala opened this issue 2 years ago • 5 comments

Signed-off-by: LiHeng [email protected]

Many thanks for submitting your Pull Request :heart:!

What this PR does / why we need it: need a way to validate the schemas, with this in place the remaining items would just be to validate whether the functionref/eventref/error ref is listed on top in the same workflow

Special notes for reviewers:

Additional information (if needed):

brlala avatar Oct 04 '22 19:10 brlala

CI will fail for now as quite alot of our specs are still in v0.7, raising this to collect feedback

brlala avatar Oct 04 '22 19:10 brlala

Hi, thanks for sending this. However I have two questions.

Why have you added vendor and, is there a use case to use this validation?

Vendor was added by mistake, will remove it in the next commit.

As for the use case, the first one is to ensure that the workflows in the repos are up to date to the latest JSON schema using that as the source of truth(can be added to CI), the second one being a utility for people who uses this package to validate their existing workflow e.g. when updating from v7 to v8 or to check the workflow they write are correct

brlala avatar Oct 05 '22 02:10 brlala

Thanks, @brlala, but I don't think we should copy the schemas to this repo. Let's try to create our tooling (maybe in Makefile at first) to download the schemas from the website/specificationn repo (using the correct tag), then run the tests.

Thanks!

Sure, the reason to add them into the repo is prevent the files being downloaded everytime for CI, so should I continue adding the test feature for this PR?

brlala avatar Oct 07 '22 09:10 brlala

Yes! It would be nice to have these tests for sure, but the schemas we should download them from the spec. Won't take that long. We are trying to avoid duplicating the schemas in the SDKs.

ricardozanini avatar Oct 07 '22 11:10 ricardozanini

this PR could address this issue https://github.com/serverlessworkflow/sdk-go/issues/28 :)

spolti avatar Oct 14 '22 18:10 spolti

dependencies against https://github.com/serverlessworkflow/specification/pull/707 conflicting state

brlala avatar Nov 01 '22 22:11 brlala

Changes looks good. one thing missing, maybe you can also add the GHA that will run this test.

sure! Will work on it, is it better to run it against the latest scheme(main) branch or the latest major release(0.8)?

brlala avatar Nov 03 '22 14:11 brlala

Changes looks good. one thing missing, maybe you can also add the GHA that will run this test.

sure! Will work on it, is it better to run it against the latest scheme(main) branch or the latest major release(0.8)?

That's a good question, maybe LATEST, PREVIOUS and main branch?

spolti avatar Nov 03 '22 14:11 spolti

Changes looks good. one thing missing, maybe you can also add the GHA that will run this test.

sure! Will work on it, is it better to run it against the latest scheme(main) branch or the latest major release(0.8)?

That's a good question, maybe LATEST, PREVIOUS and main branch?

Just to double confirm, are you referring to LATEST - v0.8 MAIN - main PREVIOUS - v0.7

Wouldn't the CI against v0.7 always fail since some of the keywords(e.g. start) became optional in v0.8?

brlala avatar Nov 03 '22 14:11 brlala

Changes looks good. one thing missing, maybe you can also add the GHA that will run this test.

sure! Will work on it, is it better to run it against the latest scheme(main) branch or the latest major release(0.8)?

That's a good question, maybe LATEST, PREVIOUS and main branch?

Just to double confirm, are you referring to LATEST - v0.8 MAIN - main PREVIOUS - v0.7

Wouldn't the CI against v0.7 always fail since some of the keywords(e.g. start) became optional in v0.8?

Something like this. the v08 should work with v07 and v08 workflow definitions. Maybe, instead have three checks we can reorganize the examples and have them segregated by spec version. and for any new spec that is released, we check it with all the previous ones.

spolti avatar Nov 04 '22 18:11 spolti

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Dec 20 '22 00:12 github-actions[bot]