Skip to content

Comments

validate_json_schema() function#23

Merged
SarahJohnsonONS merged 4 commits intomainfrom
#17-validate-json
Feb 9, 2024
Merged

validate_json_schema() function#23
SarahJohnsonONS merged 4 commits intomainfrom
#17-validate-json

Conversation

@SarahJohnsonONS
Copy link
Contributor

validate_json_schema() added with tests. Function accepts schema as str, and data to be validated against schema as str or dict. Users can also include an optional msg (str) and specify the integer value to indent json output.

TODO: Write README once function is approved.

Copy link
Contributor

@mikeAdamss mikeAdamss left a comment

Choose a reason for hiding this comment

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

few comments but this is mostly there

Copy link
Contributor

@mikeAdamss mikeAdamss left a comment

Choose a reason for hiding this comment

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

minor comments

else:
error_location = "JSON data"
# Create formatted message to be output on ValidationError
if error_msg or indent:
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this work as if error_msg is None or indent is None: to allow default kwargs of None?

My thinking is, I think this all makes sense and won't interfere with things, but we don't 100% know until we get into structured logs and the like, so I'm keen for a default behaviour of exactly what jsonschema usually does (so its obvious to a DE how to turn off the prettiness if they need to).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, this works - json.dumps default for indent is None.

Copy link
Contributor

@mikeAdamss mikeAdamss left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@SarahJohnsonONS SarahJohnsonONS merged commit f265ae8 into main Feb 9, 2024
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