Skip to content

Improve JSON Schema compatibility #1099

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JBBianchi
Copy link
Member

Notes
The schema in JSON format has been temporarily added. This allows some tools to use the JSON version as it's not accessible elsewhere otherwise.

Please specify parts of this PR update:

  • Specification
  • Schema
  • Examples
  • Extensions
  • Use Cases
  • Community
  • CTK
  • Other

Discussion or Issue link:

#1093

What this PR does:

  • use allOf instead of a combination of $ref and properties for inheritance.
  • bump to version 1.0.1

Additional information:

- use `allOf` instead of a combination of `$ref` and `properties` for inheritance.
- bump to version 1.0.1

**Notes**
The schema in JSON format has been temporarily added. This allows some tools to use the JSON version as it's not accessible elsewhere otherwise.

Signed-off-by: Jean-Baptiste Bianchi <[email protected]>
Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fjtirado I think you can start testing the Java SDK with this new schema.
@JBBianchi can we have a script to translate from YAML to JSON as part of this PR? We can create one under the hack or scripts directory. This way we keep the JSON generation from YAML idempotent.

@@ -24,7 +24,7 @@ export module SWSchemaValidator {
const ajv = new Ajv({ strict: false, allowUnionTypes: true });
addFormats(ajv);

const workflowSchemaId = "https://serverlessworkflow.io/schemas/1.0.0/workflow.yaml";
const workflowSchemaId = "https://serverlessworkflow.io/schemas/1.0.1/workflow.yaml";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we merging this as part of the 1.0.1 release?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what else is planned for v1.0.1, but since this introduces a schema change, I think we should bump the version. Otherwise, anyone relying on v1.0.0 might be caught off guard when this gets merged.
For example, if @fjtirado wasn’t aware of the change and we published an override of v1.0.0, it could break the Java SDK.

@fjtirado
Copy link
Collaborator

@ricardozanini The new schema is already incorporated to the latest version of the SDK serverlessworkflow/sdk-java#567

@JBBianchi
Copy link
Member Author

@JBBianchi can we have a script to translate from YAML to JSON as part of this PR? We can create one under the hack or scripts directory. This way we keep the JSON generation from YAML idempotent.

I forgot to remove the JSON file altogether. I would rather keep only the YAML file in this repo, and build a pipeline to update the website with both YAML and JSON. WDYT ?

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.

3 participants