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

SAT: perform extra validation on integer fields. #19800

Closed
alafanechere opened this issue Nov 24, 2022 · 0 comments · Fixed by #19820
Closed

SAT: perform extra validation on integer fields. #19800

alafanechere opened this issue Nov 24, 2022 · 0 comments · Fixed by #19820

Comments

@alafanechere
Copy link
Contributor

What

SAT perform schema validation of records against the catalog schema with the jsonschema library.
According to jsonschema a float value with a zero fractional part is a valid integer :

>>> from jsonschema import Draft7Validator
>>> schema = {'$schema': 'http://json-schema.org/draft-07/schema#', 'type': 'object', 'properties': {'metrics.conversion': {'type': ['null', 'integer']},}}
>>> record = {'metrics.conversion': 1.0}
>>> Draft7Validator(schema).is_valid(record)
True
>>> record = {'metrics.conversion': 1.1}
>>> Draft7Validator(schema).is_valid(record)
False

This relaxed definition is problematic because database destinations are more strict in terms of typing.
This situation prevents us from identifying schema inconsistencies in SAT when a field is declared as integers but the actual record values in our test account are 1.0, 2.0, 3.0 etc. It led this PR that introduced a schema inconsistency to be merged with successful acceptance tests.

How

A. Implement a custom check in the verify_records_schema function to make sure that a field defined as integer is an integer in python term's...
B. Find a way to force this validation with jsonschema

@alafanechere alafanechere changed the title SAT: perform custom validation on integer fields. SAT: perform extra validation on integer fields. Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant