Skip to content
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

Generate JSON schema of Flogo app descriptor #20

Merged
merged 2 commits into from
Feb 28, 2019

Conversation

debovema
Copy link
Contributor

Completely inspired by work of @pointlander in https://github.com/project-flogo/microgateway project (see https://github.com/project-flogo/microgateway/tree/e0abd58fba70f73f8bb6e2c68d23f50277700826/internal/schema), I added the required files to generate the JSON schema of the Flogo app descriptor (flogo.json).

This will be very useful to validate against the schema at buildtime / runtime and output the errors to end-users and also to write unit tests.

@debovema
Copy link
Contributor Author

BTW, you can validate the default flogo.json against the generated JSON schema at https://www.jsonschemavalidator.net for instance.

@debovema
Copy link
Contributor Author

Is there something missing before merging ?

@fm-tibco
Copy link

Hi guys,

There is no doubt that we need to provide the schema, the question is do we need this schema generating code or should we just provide the file? I hope we aren't going to be changing the schema much more going forward... but having the code is nice. @mellistibco @vijaynalawade any thoughts?

@vijaynalawade
Copy link
Contributor

I tend to agree with @fm-tibco. A single schema definition in core repo should be good enough to validate any flogo app json. Things like object schema are under consideration that might need app descriptor update. So, it would be better to keep a file which can be updated easily.

@emilienthomas
Copy link

emilienthomas commented Jan 31, 2019

Franck, Vijay,

If the flogo app json changes often, you save time and effort by generating the json schema.
If the flogo app json changes only a few times, generating the schema in the build process ensures that you do not forget to impact the json schema file.

What advantage do you see in not generating the json schema ?

@debovema
Copy link
Contributor Author

debovema commented Feb 1, 2019

The idea behind this was also to facilitate the writing of unit tests with standard Flogo app descriptor.
If the structures evolve, we automatically ensure that older flogo.json (used by customers) still validate, load and run.

@mellistibco
Copy link
Member

I'm not sure why we should generate the schema during build? Why not generate it once and store it in git? We should then look at adding validation to the cli. I'm not sure I see any reason for the schema to generate code in core. If this is just to facilitate unit tests, perhaps the cli can generate whatever code is required via a plugin?

@fm-tibco
Copy link

fm-tibco commented Feb 1, 2019

Regardless, the flow and stream actions should also have schema's for their corresponding resources.

I don't think the schema will change much, but the nice thing about this, as part of the travis build, we can generate the schema and if by any chance the schema.json changed, we can catch it there. This assumes the schema generation code generates an identical schema.json if the code hasn't been altered.

@mellistibco
Copy link
Member

@debovema what specifically are you thinking? microgateway uses the schema for run time validation, are you thinking the same here?

@debovema
Copy link
Contributor Author

debovema commented Feb 1, 2019

Validation at runtime could be good. Especially if the validation outputs more readable messages to the end user in case of errors.
But the main advantage I see is to keep a single source of truth. Currently the truth is given by the Go structures (as they will be used to parse actually) and, with your option, generating the JSON separately will be another source of truth uncorrelated to the first one. Hence the 😕

@fm-tibco
Copy link

fm-tibco commented Feb 1, 2019

Thats fine, we should also incorporate the validation into the engine as part of startup. What do you mean by validate against the schema during build time? How do you envision this being used in unit tests... do we need to expose a public method given that currently resides in the internal directory?

@debovema debovema closed this Feb 3, 2019
@fm-tibco
Copy link

fm-tibco commented Feb 4, 2019

@debovema Any reason you closed this PR? I was hoping to incorporate your code in core and also in both flow and stream to validate their resources.

@debovema debovema reopened this Feb 4, 2019
@debovema
Copy link
Contributor Author

debovema commented Feb 4, 2019

Sorry it was a false manipulation with mobile phone.

@debovema
Copy link
Contributor Author

debovema commented Feb 4, 2019

I am not sure if we can avoid the public method. The package name could be changed though.

@debovema
Copy link
Contributor Author

debovema commented Feb 4, 2019

I created project-flogo/flow#4 for flow action definition.

@fm-tibco fm-tibco merged commit b3e6192 into project-flogo:master Feb 28, 2019
@debovema debovema deleted the generate-json-schema branch February 28, 2019 16:24
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.

None yet

5 participants