SAT: check records to comply jsonschema format field #5661
Conversation
avida
force-pushed
the
drezchykov/5477-SAT-check-records-format
branch
from
220ff9a to
4d01696
Compare
| @pytest.mark.parametrize( | ||
| "record, configured_catalog, valid", | ||
| [ | ||
| # time |
There was a problem hiding this comment.
You have tests only for time and date-time. What about date? Could you please add tests for date format also?
| @@ -38,7 +38,7 @@ def verify_records_schema( | |||
| """ | |||
| validators = {} | |||
| for stream in catalog.streams: | |||
| validators[stream.stream.name] = Draft4Validator(stream.stream.json_schema) | |||
| validators[stream.stream.name] = Draft7Validator(stream.stream.json_schema, format_checker=FormatChecker()) | |||
There was a problem hiding this comment.
For sanity check: what happens you send null data to format_checker?
There was a problem hiding this comment.
It would generate an error, added this case to unittests.
|
@Phlair adding you as a reviewer since it's a CDK related |
| timestamp_regex = re.compile(("^\d{4}-\d?\d-\d?\d" # date | ||
| "(\s|T)" # separator | ||
| "\d?\d:\d?\d:\d?\d(.\d+)?" # time | ||
| ".*$" #timezone | ||
| )) |
There was a problem hiding this comment.
is there a standard (e.g. ISO 8601) that this regex is based on?
Whether based on a standard or custom, it would be useful to add a description in the docs (somewhere on this page perhaps) so that date/time format requirements of a connector are clear when building one.
There was a problem hiding this comment.
No, its based on bigquery timestamp format (https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#timestamp_type) as its most strict of sql timestamp formats we are supporting
There was a problem hiding this comment.
Gotcha, so we're lining it up to our most strict destination to ensure compatibility. Makes sense. Small piece in creating connector docs explaining that (with some examples) would be great.
github-actions
bot
added
the
area/documentation
What
Resolves #5477 issue
How
Describe the solution
Recommended reading order
x.javay.pythonPre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mddocs/SUMMARY.mddocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampledocs/integrations/README.mdairbyte-integrations/builds.mdAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing./publishcommand described hereUpdating a connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mddocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing./publishcommand described hereConnector Generator
-scaffoldin their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplatesthen checking in your changes