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

Fix top-level is_valid flag for recursive jsonschema exceptions #208

Merged
merged 2 commits into from
Apr 22, 2022
Merged

Fix top-level is_valid flag for recursive jsonschema exceptions #208

merged 2 commits into from
Apr 22, 2022

Conversation

gadomski
Copy link
Member

The recursive validator catches jsonschema exceptions and sets the appropriate message. However, because it catches and doesn't re-raise the exception, the higher-level run method was still setting is_valid to True. This PR fixes the problem by returning False from the recursive validator if a jsonschema exception was caught, and True otherwise. Includes a unit test demonstrating the issue.

Related issues

The recursive validator catches jsonschema exceptions and sets the appropriate
message. However, because it catches and doesn't re-raise the exception, the
higher-level `run` method was still setting `is_valid` to `True`. This PR fixes
the problem by returning `False` from the recursive validator if a jsonschema
exception was caught, and True otherwise. Includes a unit test demonstrating the
issue.
@gadomski gadomski requested a review from jonhealy1 April 22, 2022 13:14
@gadomski gadomski changed the title fix: is_valid = false for recursive jsonschema Fix is_valid for recursive jsonschema exceptions Apr 22, 2022
@gadomski gadomski changed the title Fix is_valid for recursive jsonschema exceptions Fix top-level is_valid flag for recursive jsonschema exceptions Apr 22, 2022
Copy link
Collaborator

@jonhealy1 jonhealy1 left a comment

Choose a reason for hiding this comment

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

Looks great!

@gadomski gadomski merged commit 48baf27 into stac-utils:main Apr 22, 2022
@gadomski gadomski deleted the fix-recusive-jsonschema-retval branch April 22, 2022 18:28
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.

2 participants