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

Switch to using fastjsonschema for schema validation. #809

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

clalancette
Copy link
Contributor

The main reason to do this is because jsonschema (what we currently use) depends on referencing, which depends on rpds-py, which has C libraries. That means that when using the python debug interpreter on Windows, we would have to build a custom pip package for it to work.

In contrast, fastjsonschema is pure python and has no dependencies. It is also available on all of our default platforms. So switch to using it here so things work on Windows debug.

Note that this depends on both ros2/ci#782 and ros/rosdistro#41538 to go in first.

The main reason to do this is because jsonschema (what
we currently use) depends on referencing, which depends
on rpds-py, which has C libraries.  That means that when
using the python debug interpreter on Windows, we would
have to build a custom pip package for it to work.

In contrast, fastjsonschema is pure python and has no
dependencies.  It is also available on all of our default
platforms.  So switch to using it here so things work on
Windows debug.

Signed-off-by: Chris Lalancette <[email protected]>
@clalancette
Copy link
Contributor Author

Rpr job is expected to fail until we actually merge this in and do a release.

@clalancette
Copy link
Contributor Author

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status
  • Windows Debug Build Status

Copy link

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

sounds much better approach for me.

@clalancette
Copy link
Contributor Author

@ros-pull-request-builder retest this please

@clalancette
Copy link
Contributor Author

Here's new CI with the new changes in ros2/ci#782 and ros-infrastructure/ros2-cookbooks#71:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status
  • Windows Debug Build Status

@clalancette
Copy link
Contributor Author

All right, all dependent PRs are merged, and CI is green here. Going ahead and merging this one in, thanks for the reviews.

@clalancette clalancette merged commit f25110b into rolling Jun 13, 2024
4 checks passed
@clalancette clalancette deleted the clalancette/fastjsonschema branch June 13, 2024 18:40
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.

4 participants