[low code connectors] perform schema validation of the input config against the declarative language schema #15543

brianjlai · 2022-08-11T09:36:23Z

What

This PR adds schema validation of the declarative language schema against a connector's YAML file implementation. At a high level, we are perform the schema validation at each level of component creation. So the creation of the schema for a given class and validation integrated into the creation flow which is then controlled with a boolean instantiate. For validation we pass False and for the actual existing creation flow, we pass True.

How

Because we're leveraging the existing create() flow, we do not have to perform a complex traversal of object relationships. Instead we traverse the object like we already do during creation, however, we have an additional flow in factory.py where instantiate is false, to perform the validation. We do this for each component of the YAML as well as at the top level which doesn't use the factory.

The general flow is made up of expanding a components interface into concrete classes, generating the json schema, performing the validation against the component definition arguments.

The more interesting bit is how we perform this expansion of interfaces which is critical to validating correctness

For a given dataclass, get all of its fields
Unpack the field which expands all interfaces. This can be complex because we need to retain the same object structure of any parent generic structures.

If a class is not contained in a generic and is declarative, we attempt to get it's subclasses
If it has no subclasses or is a non-declarative type we simply return
For classes contained within a generic (or multiple ones) we get the underlying type and recursive unpack
Depending on the generic type we either package it back up into it's respective type

Replace the annotations of the current dataclass with the expanded interface

note: This accounts for Optional (which is a union of type and none), lists, and Unions. So far in our schema i didn't see any cases for mappings that contained internal interfaces that needed expansion

Example Error Validations:

Wrong type than expected:

extractor:
  type: DpathExtractor
selector:
  class_name: airbyte_cdk.sources.declarative.extractors.record_selector.RecordSelector
  record_filter:
    class_name: airbyte_cdk.sources.declarative.extractors.record_filter.RecordFilter
    condition: "{{ record['id'] > stream_state['id'] }}"
  extractor:
    $ref: "*ref(extractor)"
    field_pointer: 408

Error:

instance = {'config': {'apikey': 'verysecrettoken', 'repos': ['airbyte', 'airbyte-cloud']}, 'limit_option': {'$options': {}, 'con...ions': {}, 'config': {'apikey': 'verysecrettoken', 'repos': ['airbyte', 'airbyte-cloud']}, 'inject_into': 'path'}, ...}
schema = {'$schema': 'http://json-schema.org/draft-06/schema#', 'definitions': {'CursorPaginationStrategy': {'description': '\n...$ref': '#/definitions/JsonDecoder', 'default': {}}, 'limit_option': {'$ref': '#/definitions/RequestOption'}, ...}, ...}
cls = <class 'jsonschema.validators.create.<locals>.Validator'>, args = ()
kwargs = {}
validator = <jsonschema.validators.create.<locals>.Validator object at 0x1049ef880>
error = <ValidationError: "408 is not of type 'array'">

Missing a required field:

stream_slicer:
  type: DatetimeStreamSlicer
  $options:
    datetime_format: "%Y-%m-%dT%H:%M:%S.%f%z"
  start_datetime:
    type: MinMaxDatetime
    datetime: "{{ config['start_time'] }}"
    min_datetime: "{{ config['start_time'] + day_delta(2) }}"
  end_datetime: "{{ config['end_time'] }}"
  cursor_field: "created"
  lookback_window: "5d"
  start_time_option:
    inject_into: request_parameter
    field_name: created[gte]

Error:

instance = {'config': {'apikey': 'verysecrettoken', 'repos': ['airbyte', 'airbyte-cloud']}, 'limit_option': {'$options': {}, 'con...ions': {}, 'config': {'apikey': 'verysecrettoken', 'repos': ['airbyte', 'airbyte-cloud']}, 'inject_into': 'path'}, ...}
schema = {'$schema': 'http://json-schema.org/draft-06/schema#', 'definitions': {'CursorPaginationStrategy': {'description': '\n...$ref': '#/definitions/JsonDecoder', 'default': {}}, 'limit_option': {'$ref': '#/definitions/RequestOption'}, ...}, ...}
cls = <class 'jsonschema.validators.create.<locals>.Validator'>, args = ()
kwargs = {}
validator = <jsonschema.validators.create.<locals>.Validator object at 0x107ca6280>
error = <ValidationError: "'step' is a required property">

Wrong type passed for a reference type (this one is interesting because validator doesn't know how to compare literal types, but it identifies that the passed in type is missing a field (cursor_value) that is expected on classes that implement pagination_strategy.

paginator:
  type: "LimitPaginator"
  page_size: 10
  url_base: "https://airbyte.io"
  limit_option:
    inject_into: request_parameter
    field_name: page_size
  page_token_option:
    inject_into: path
  pagination_strategy:
    type: "MinMaxDatetime"
    datetime: "{{ response._metadata.next }}"

Error:

instance = {'config': {'apikey': 'verysecrettoken', 'repos': ['airbyte', 'airbyte-cloud']}, 'limit_option': {'$options': {}, 'con...ions': {}, 'config': {'apikey': 'verysecrettoken', 'repos': ['airbyte', 'airbyte-cloud']}, 'inject_into': 'path'}, ...}
schema = {'$schema': 'http://json-schema.org/draft-06/schema#', 'definitions': {'CursorPaginationStrategy': {'description': '\n...$ref': '#/definitions/JsonDecoder', 'default': {}}, 'limit_option': {'$ref': '#/definitions/RequestOption'}, ...}, ...}
cls = <class 'jsonschema.validators.create.<locals>.Validator'>, args = ()
kwargs = {}
validator = <jsonschema.validators.create.<locals>.Validator object at 0x106768c40>
error = <ValidationError: "'cursor_value' is a required property">

The errors are bit more verbose

…validator

brianjlai · 2022-08-15T22:11:24Z

airbyte-cdk/python/unit_tests/sources/declarative/test_factory.py

+
+# Leaving this test here to document a limitation of the validator. Decoder has no meaningful fields to validate on so it accepts
+# the MinMaxDatetime despite being the wrong type
+def test_validation_wrong_object_type():


This is a limitation of the validator itself

brianjlai · 2022-08-15T22:12:05Z

airbyte-cdk/python/unit_tests/sources/declarative/test_factory.py

+
+# This test should fail because the extractor doesn't match the Array of resolved classes. However, despite the schema being correct
+# validation passes. Leaving this here to document it and revisit at another time. This is another validator limitation.
+def test_validate_types_nested_in_list():


I've actually tested this by explicitly replacing the Interface in our code with a union of relevant types, but interestingly the validator still couldn't pick up on it.

brianjlai · 2022-08-15T22:13:46Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/auth/declarative_authenticator.py

+
+
+@dataclass
+class DeclarativeAuthenticator(ABC):


This abstraction is needed so that we only attempt to expand certain authenticators during validation. It is a little bit of a concern that schema generation is coupled very closely to our exact object model which could get a little unwieldly in the future.

brianjlai · 2022-08-15T22:16:39Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/parsers/factory.py

+        else:
+            # Because the component's data fields definitions use interfaces, we need to resolve the underlying types into the
+            # concrete classes that implement the interface before generating the schema
+            DeclarativeComponentFactory._transform_interface_to_union(class_)


Interestingly, even though we make a copy of the class and replace those annotations, it does seem to modifying the incoming class argument. I tried returning the relevant class and the way it is currently shown, to the same effect of expansion. However, if I comment this line out, the expansion will not go through (which is expected)

this is almost certainly because the fields don't get copied.

Looking at dataclasses.fields, it seems like they are in the "dataclass_fields" field of the class' __dict__.

They don't get copied because we don't do a deepcopy of the dict here.

That being said, I don't know if there's an easy way to copy them because mappingproxys can't be serialized.

I doubt this matters much...

brianjlai · 2022-08-15T22:18:41Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/parsers/factory.py

+            copy_cls.__annotations__[field.name] = unpacked_field_types
+
+    @staticmethod
+    def unpack(field_type: type):


This is where it gets particularly fun. This whole recursive function is necessary to retain the existing structure of any declarative components that must be expanded, without losing the shape of the container they are in. We basically retrieve the underlying type and use that as the next recursive call. And once we have unpacked the underlying classes, we package it back into its original container. I added a handful of tests to verify this was where the subclass replacement became less than trivial.

Happy to discuss this part more since this was something I realized we needed midway through.

sherifnada · 2022-08-15T22:24:35Z

@brianjlai could you show some examples of the error message associated with an invalid YAML?

brianjlai · 2022-08-15T22:46:49Z

yup i'll add some sample test cases and the validation message that comes back!

girarda · 2022-08-15T22:57:07Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/http_requester.py

        if self.request_options_provider is None:
            self._request_options_provider = InterpolatedRequestOptionsProvider(config=self.config, options=options)
        elif isinstance(self.request_options_provider, dict):
            self._request_options_provider = InterpolatedRequestOptionsProvider(config=self.config, **self.request_options_provider)
        else:
            self._request_options_provider = self.request_options_provider
-        self.authenticator = self.authenticator or NoAuth()
+        self.authenticator = self.authenticator or BasicHttpAuthenticator("", config=self.config, options={})


why is BasicHttp the default?

yeah that's a good point. I think it makes sene to just have define a declarative NoAuth that implements the methods and returns empty strings. Will fix!

girarda · 2022-08-15T23:12:14Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/yaml_declarative_source.py


    def _read_and_parse_yaml_file(self, path_to_yaml_file):
        with open(path_to_yaml_file, "r") as f:
            config_content = f.read()
            return YamlParser().parse(config_content)
+
+    def _validate_source(self):
+        full_config = {


minor: can we also check the "version" field we added last week?

Confirming its presence is enough for now. no need for additional validation

ah yes will add!

girarda · 2022-08-16T00:22:55Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/parsers/factory.py

+        else:
+            # Because the component's data fields definitions use interfaces, we need to resolve the underlying types into the
+            # concrete classes that implement the interface before generating the schema
+            DeclarativeComponentFactory._transform_interface_to_union(class_)


this is almost certainly because the fields don't get copied.

Looking at dataclasses.fields, it seems like they are in the "dataclass_fields" field of the class' __dict__.

They don't get copied because we don't do a deepcopy of the dict here.

That being said, I don't know if there's an easy way to copy them because mappingproxys can't be serialized.

I doubt this matters much...

brianjlai · 2022-08-18T18:35:08Z

/publish-cdk dry-run=true

🕑 https://github.com/airbytehq/airbyte/actions/runs/2884546299
✅ https://github.com/airbytehq/airbyte/actions/runs/2884546299

brianjlai · 2022-08-18T18:45:44Z

/publish-cdk dry-run=false

🕑 https://github.com/airbytehq/airbyte/actions/runs/2884600096
❌ https://github.com/airbytehq/airbyte/actions/runs/2884600096

brianjlai · 2022-08-18T19:02:35Z

/publish-cdk dry-run=true

🕑 https://github.com/airbytehq/airbyte/actions/runs/2884697466
✅ https://github.com/airbytehq/airbyte/actions/runs/2884697466

brianjlai · 2022-08-18T19:17:55Z

/publish-cdk dry-run=false

🕑 https://github.com/airbytehq/airbyte/actions/runs/2884785622
✅ https://github.com/airbytehq/airbyte/actions/runs/2884785622

…gainst the declarative language schema (#15543) * draft: first pass at complete schema language generation and factory validator * actually a working validator and fixes to the schema that went uncaught * remove extra spike file * fix formatting file * pr feedback and a little bit of refactoring * fix some types that were erroneously marked as invalid schema * some comments * add jsonschemamixin to interfaces * update changelog * bump version

draft: first pass at complete schema language generation and factory …

1efe2f0

…validator

github-actions bot added the CDK Connector Development Kit label Aug 11, 2022

brianjlai added 3 commits August 13, 2022 17:35

Merge branch 'master' into brian/low_code_validate_schema

30a73ae

actually a working validator and fixes to the schema that went uncaught

5ca8e9e

remove extra spike file

385a593

brianjlai changed the title ~~draft: first pass at generating the complete schema and performing schema validation in factory~~ [low code connectors] perform schema validation of the input config against the declarative language schema Aug 14, 2022

fix formatting file

a29c048

brianjlai commented Aug 15, 2022

View reviewed changes

brianjlai marked this pull request as ready for review August 15, 2022 22:19

brianjlai requested review from a team and girarda August 15, 2022 22:19

brianjlai mentioned this pull request Aug 15, 2022

[low code connectors] generate complete json schema from classes #15647

Merged

girarda reviewed Aug 16, 2022

View reviewed changes

brianjlai added 2 commits August 15, 2022 22:29

pr feedback and a little bit of refactoring

41eea30

Merge branch 'master' into brian/low_code_validate_schema

e232f09

girarda approved these changes Aug 16, 2022

View reviewed changes

brianjlai added 8 commits August 16, 2022 10:22

Merge branch 'master' into brian/low_code_validate_schema

1111da5

fix some types that were erroneously marked as invalid schema

2d71d85

some comments

65a7bd0

Merge branch 'master' into brian/low_code_validate_schema

8d3d1fc

add jsonschemamixin to interfaces

561a285

Merge branch 'master' into brian/low_code_validate_schema

5d2e722

Merge branch 'master' into brian/low_code_validate_schema

2c3f720

update changelog

6ef9e10

bump version

3612789

brianjlai merged commit ca80d37 into master Aug 18, 2022

brianjlai deleted the brian/low_code_validate_schema branch August 18, 2022 19:29

octavia-squidington-iii mentioned this pull request Aug 19, 2022

Bump Airbyte version from 0.40.0-alpha to 0.40.1-alpha #15775

Closed

evantahler mentioned this pull request Aug 22, 2022

Bump Airbyte version from 0.40.0-alpha to 0.40.1 #15857

Merged

brianjlai mentioned this pull request Aug 25, 2022

[low-code connectors] Protect top-level namespace in yaml file #15467

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[low code connectors] perform schema validation of the input config against the declarative language schema #15543

[low code connectors] perform schema validation of the input config against the declarative language schema #15543

brianjlai commented Aug 11, 2022 •

edited

Loading

brianjlai Aug 15, 2022

brianjlai Aug 15, 2022

brianjlai Aug 15, 2022

brianjlai Aug 15, 2022

girarda Aug 16, 2022

brianjlai Aug 15, 2022 •

edited

Loading

sherifnada commented Aug 15, 2022

brianjlai commented Aug 15, 2022

girarda Aug 15, 2022

brianjlai Aug 16, 2022

girarda Aug 15, 2022

brianjlai Aug 16, 2022

girarda Aug 16, 2022

brianjlai commented Aug 18, 2022 •

edited by github-actions bot

Loading

brianjlai commented Aug 18, 2022 •

edited by github-actions bot

Loading

brianjlai commented Aug 18, 2022 •

edited by github-actions bot

Loading

brianjlai commented Aug 18, 2022 •

edited by github-actions bot

Loading

[low code connectors] perform schema validation of the input config against the declarative language schema #15543

[low code connectors] perform schema validation of the input config against the declarative language schema #15543

Conversation

brianjlai commented Aug 11, 2022 • edited Loading

What

How

Recommended reading order

Example Error Validations:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianjlai Aug 15, 2022 • edited Loading

Choose a reason for hiding this comment

sherifnada commented Aug 15, 2022

brianjlai commented Aug 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianjlai commented Aug 18, 2022 • edited by github-actions bot Loading

brianjlai commented Aug 18, 2022 • edited by github-actions bot Loading

brianjlai commented Aug 18, 2022 • edited by github-actions bot Loading

brianjlai commented Aug 18, 2022 • edited by github-actions bot Loading

brianjlai commented Aug 11, 2022 •

edited

Loading

brianjlai Aug 15, 2022 •

edited

Loading

brianjlai commented Aug 18, 2022 •

edited by github-actions bot

Loading

brianjlai commented Aug 18, 2022 •

edited by github-actions bot

Loading

brianjlai commented Aug 18, 2022 •

edited by github-actions bot

Loading

brianjlai commented Aug 18, 2022 •

edited by github-actions bot

Loading