feat(json-schema-check): add new action #240
Conversation
The goal is to check the compliance of configuration files against their Schema template. Leveraging the check-jsonschema CLI tool. Issue-Id: SCM-6366 Signed-off-by: Ambroise Vincent <[email protected]> Change-Id: I3ca864e58d408e85ed049266c3e703b8a19220b9
|
@ambroise-arm Thank you for the pull request. |
|
@kenji-miyake We want to have per package:
This patch is to allow to check that the param.yaml files match their package template. Separately, each schema.json file refers to a meta-schema that it adheres to (https://github.com/autowarefoundation/autoware.universe/blob/e70df1239c67d8ae86a7a8bf8d6c83bc20e7ece7/perception/lidar_apollo_segmentation_tvm_nodes/schema/lidar_apollo_segmentation_tvm_nodes.schema.json#L2). That's what the pre-commit is checking. I couldn't make the check of this patch fit within the already-existing pre-commit hook (or already-existing github actions). But the opposite is possible, we could drop the pre-commit check and have it as part of this action. |
|
|
||
| - name: Check configuration files | ||
| run: | | ||
| find -wholename '*/schema/*.schema.json' -execdir bash -c 'check-jsonschema --schemafile "$1" ../config/*.param.yaml' bash '{}' + |
There was a problem hiding this comment.
1 schema/{pkg_name}.schema.json that acts the template for the configuration files
1 (or more) config/*.param.yaml configuration that need to match the template
It seems these behaviors aren't implemented yet.
Is this pull request still under draft or do you plan to implement them in another pull request?
If it's the former, please mark this pull request as a draft.
If it's the latter, creating an issue would be good.
(Or am I misunderstanding something?)
There was a problem hiding this comment.
The design for the structure has been published here: https://github.com/orgs/autowarefoundation/discussions/3371
It has been implemented as an example for lidar_apollo_segmentation_tvm_nodes. A second one was added recently with autoware_auto_msgs_adapter. The rest is still to be done.
The idea is that in order for people to add those files, it is valuable to have the format checked in CI before they start to create patches. Otherwise we risk having incorrect files in the repositories and it will take time later to fix.
There was a problem hiding this comment.
Ah, I misunderstood one thing. The correspondence between param.yaml and schema.json was checked in the command.
@ambroise-arm So do you want to merge this as it is?
I mean, it seems that this action succeeds if a package doesn't have any schema.json. Is that okay for you?
There was a problem hiding this comment.
I will update the spelling with ROS 2, otherwise yes.
Should I update .github/workflows/test-composite-actions.yaml and the main README to add this? Or anything else that I missed?
There was a problem hiding this comment.
Should I update .github/workflows/test-composite-actions.yaml
It's not mandatory, but it's preferable if you can test this action.
the main README to add this?
Ah, yes. It would be nice.
Or anything else that I missed?
No, looks good! Thank you for your work.
I'll make a new release after merging it.
There was a problem hiding this comment.
I've fixed them, so I'll merge this.
Description
Create a new action, where the goal is to check the compliance of configuration files against their Schema template.
Leveraging the check-jsonschema CLI tool.
It can then be used in autoware.universe and autoware.core.
Related links
Implements #232
Tests performed
I haven't tested the deployment as a dedicated Github action in a separate repository, but I tested the specific commands in CI:
Checked that it passed on correct configuration files at https://github.com/ambroise-arm/autoware.universe/actions/runs/5354465514
Checked that it failed on wrong configuration files at https://github.com/ambroise-arm/autoware.universe/actions/runs/5322522827
Notes for reviewers
I will have a follow up PR to autoware.universe to make use of this.
Interface changes
Effects on system behavior
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.