perf: Validate records with fastjsonschema

[BuzzCutNorman]

Add dependency of fastjsonshema and refactor of Sink and Sink._validate_and_parse utilize fastjsonschema.

Closes #2045

---

📚 Documentation preview 📚: https://meltano-sdk--2066.org.readthedocs.build/en/2066/

[BuzzCutNorman]

I have run into a date-time and time problem. If a date-time or time does not end in with a time zone like +00:00 or Z it is seen as not valid.

>>>
>>>
>>> import fastjsonschema
>>>
>>> validator = fastjsonschema.compile({
... "type": "object",
... "properties": {
... "id": {"type": "integer"},
... "datetime_col": {"type": "string", "format": "date-time"},
... "date_col": {"type": "string", "format": "date"},
... "time_col": {"type": "string", "format": "time"},
... },
... "required": ["id"],
... })
>>>
>>>
>>> validator({"id": 1, "datetime_col": "2023-01-03T08:10:00.000000+00:00", "date_col": "2023-01-03", "time_col": "08:10:00.000000+00:00"})
{'id': 1, 'datetime_col': '2023-01-03T08:10:00.000000+00:00', 'date_col': '2023-01-03', 'time_col': '08:10:00.000000+00:00'}
>>>
>>>
>>> validator({"id": 2, "datetime_col": "2023-01-03T08:10:00.000000Z", "date_col": "2023-01-03", "time_col": "08:10:00.000000Z"})
{'id': 2, 'datetime_col': '2023-01-03T08:10:00.000000Z', 'date_col': '2023-01-03', 'time_col': '08:10:00.000000Z'}
>>>
>>>
>>> validator({"id": 3, "datetime_col": "2023-01-03T08:10:00.000000", "date_col": "2023-01-03", "time_col": "08:10:00.000000"})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 24, in validate
fastjsonschema.exceptions.JsonSchemaValueException: data.datetime_col must be date-time
>>>

CC: @edgarrmondragon

[BuzzCutNorman]

Going back to the JSONSchema documentation I found that date-time, date, and time values should conform to RFC 3339, section 5.6. Reading through the specification I found a that RFC 3339, section 4.3 states how to deal with unknown local offset. In these instances, you represent the offset with -00:00. I have found that fastjsonschema finds date-time and time with the -00:00 offset as valid.

• https://json-schema.org/understanding-json-schema/reference/string#dates-and-times
• https://datatracker.ietf.org/doc/html/rfc3339#section-5.6
• https://datatracker.ietf.org/doc/html/rfc3339#section-4.3

>>> import fastjsonschema
>>>
>>>
>>> validator = fastjsonschema.compile({
... "type": "object",
... "properties": {
... "id": {"type": "integer"},
... "datetime_col": {"type": "string", "format": "date-time"},
... "date_col": {"type": "string", "format": "date"},
... "time_col": {"type": "string", "format": "time"},
... },
... "required": ["id"],
... })
>>>
>>> validator({"id": 1, "datetime_col": "2023-01-03T08:10:00.000000+00:00", "date_col": "2023-01-03", "time_col": "08:10:00.000000+00:00"})
{'id': 1, 'datetime_col': '2023-01-03T08:10:00.000000+00:00', 'date_col': '2023-01-03', 'time_col': '08:10:00.000000+00:00'}
>>>
>>>
>>> validator({"id": 2, "datetime_col": "2023-01-03T08:10:00.000000Z", "date_col": "2023-01-03", "time_col": "08:10:00.000000Z"})
{'id': 2, 'datetime_col': '2023-01-03T08:10:00.000000Z', 'date_col': '2023-01-03', 'time_col': '08:10:00.000000Z'}
>>>
>>>
>>> validator({"id": 1, "datetime_col": "2023-01-03T08:10:00.000000-00:00", "date_col": "2023-01-03", "time_col": "08:10:00.000000-00:00"})
{'id': 1, 'datetime_col': '2023-01-03T08:10:00.000000-00:00', 'date_col': '2023-01-03', 'time_col': '08:10:00.000000-00:00'}
>>>

CC @edgarrmondragon

[codspeed-hq]

CodSpeed Performance Report

Merging #2066 will improve performances by ×28

_{Comparing BuzzCutNorman:2045-validate-records-with-fastjsonschema (9d4fb94) with main (89d6afc)}

Summary

⚡ 2 improvements
✅ 4 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	BuzzCutNorman:2045-validate-records-with-fastjsonschema	Change
⚡	test_bench_validate_and_parse	673.1 ms	92.1 ms	×7.3
⚡	test_bench_validate_record_with_schema	601.8 ms	21.6 ms	×28

[edgarrmondragon]

Going back to the JSONSchema documentation I found that date-time, date, and time values should conform to RFC 3339, section 5.6. Reading through the specification I found a that RFC 3339, section 4.3 states how to deal with unknown local offset. In these instances, you represent the offset with -00:00. I have found that fastjsonschema finds date-time and time with the -00:00 offset as valid.

I see. This makes me a bit worried that this may be a breaking change. Targets may be currently doing some false negatives and letting values like 2023-01-03T00:10:00.0000 go through.

[BuzzCutNorman]

I see. This makes me a bit worried that this may be a breaking change. Targets may be currently doing some false negatives and letting values like 2023-01-03T00:10:00.0000 go through.

@edgarrmondragon , I do have a test table (StackOverflow postlinks table) in which that very scenario is happening. Dates with no time zone pass validation using jsonschema but are failing validation with fastjsonschema.

I did some testing yesterday and even if we could append -00:00 to date-time or time strings so they pass fastjsonschema validation, once the string is processed by .fromisoformat a datetime or time with the UTC time zone is generated instead of a datetime or time no time zone. That solution will not work.

This morning I found that you can pass a custom format to replace a built-in format. I tested a regex string for iso datetime with an optional time zone which resolved the before mention issue with my test table. I also tested a simple lamda function that passed date-time when value == value. There seems to be no performance impact by doing this. There is also a flag to disable format checking use_formats=False.

[BuzzCutNorman]

@edgarrmondragon I took a go at incorporating the changes you wrote in PR1471 into this PR. When you get a chance, please take a look and let me know what you think.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (89d6afc) 88.47% compared to head (9d4fb94) 88.46%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2066      +/-   ##
==========================================
- Coverage   88.47%   88.46%   -0.01%     
==========================================
  Files          54       54              
  Lines        4721     4717       -4     
  Branches      919      918       -1     
==========================================
- Hits         4177     4173       -4     
  Misses        383      383              
  Partials      161      161              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[edgarrmondragon]

@BuzzCutNorman I like the approach of defaulting to no format checks since are the most problematic. We probably want consider an update to #1711, but that's outside of the scope of this issue.

[edgarrmondragon]

I think you may wanna run poetry lock --no-update and commit the updated lock file

[BuzzCutNorman]

@edgarrmondragon I was wondering if you wanted me to break out the abstraction of the JSON Schema library into its own PR?

[edgarrmondragon]

@edgarrmondragon I was wondering if you wanted me to break out the abstraction of the JSON Schema library into its own PR?

@BuzzCutNorman That'd be amazing if you're interested in doing it!

[edgarrmondragon]

Switching the codspeed workflow to run in Python 3.12 lets us see a flamegraph diff!!!

https://codspeed.io/meltano/sdk/branches/BuzzCutNorman:2045-validate-records-with-fastjsonschema

[BuzzCutNorman]

@edgarrmondragon should we keep this PR or close it? Your two PRs have not gone anywhere and fastjsonschema doesn't have support for anything above draft7. I really like the speed boost and the ease to update format checks but since I can now in the base SDK turn off format checks and even all checks I don't know if switching is worth it. What do you think?

I guess the reverse argument could also be made that if we move to fastjsonschema and a target developer needs validation options beyond what it can provide there is a path for the developer to implement the JSONSchema validation library of their choice.

[edgarrmondragon]

Superseded by #2231