- 
                Notifications
    You must be signed in to change notification settings 
- Fork 30
do not merge: vector db work (split) #760
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
base: main
Are you sure you want to change the base?
Changes from all commits
987f454
              8c84b6d
              42e2d7f
              fc37c17
              3de3785
              cffad9f
              5ccaca9
              5e485bd
              def67f7
              1f55214
              f49761b
              b8fa937
              edc20d6
              164afb6
              d14cd84
              5d75886
              061de78
              8f047b9
              a84f50a
              3f19659
              96eabbf
              aa5f6a4
              bb13c89
              43ee692
              d2c7a51
              35dda44
              c137302
              cb69302
              93aeb9f
              f325a11
              98d3296
              c742f80
              5a6aea4
              7b284d5
              749106b
              51a5ddf
              09916a9
              5f0ac1d
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|  | @@ -7,12 +7,16 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import pkgutil | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import Any, ClassVar, Dict, List, Mapping, MutableMapping, Optional, Tuple | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from copy import deepcopy | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import TYPE_CHECKING, Any, ClassVar, Dict, List, Mapping, MutableMapping, Tuple, cast | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import jsonref | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from jsonschema import RefResolver, validate | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from jsonschema import validate | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from jsonschema.exceptions import ValidationError | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from pydantic.v1 import BaseModel, Field | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from referencing import Registry, Resource | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from referencing._core import Resolver # used for type hints | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from referencing.jsonschema import DRAFT7 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from airbyte_cdk.models import ConnectorSpecification, FailureType | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from airbyte_cdk.utils.traced_exception import AirbyteTracedException | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | @@ -63,18 +67,30 @@ def resolve_ref_links(obj: Any) -> Any: | |||||||||||||||||||||||||||||||||||||||||||||||||||
| return obj | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _expand_refs(schema: Any, ref_resolver: Optional[RefResolver] = None) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| def get_ref_resolver_registry(schema: dict[str, Any]) -> Registry: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Get a reference resolver registry for the given schema.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| resource: Resource = Resource.from_contents( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| contents=schema, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| default_specification=DRAFT7, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return cast( # Mypy has a hard time detecting this return type. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Registry", | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Registry().with_resource( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| uri="", | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| resource=resource, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _expand_refs(schema: Any, ref_resolver: Resolver) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Internal function to iterate over schema and replace all occurrences of $ref with their definitions. Recursive. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param schema: schema that will be patched | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param ref_resolver: resolver to get definition from $ref, if None pass it will be instantiated | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ref_resolver = ref_resolver or RefResolver.from_schema(schema) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if isinstance(schema, MutableMapping): | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if "$ref" in schema: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ref_url = schema.pop("$ref") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| _, definition = ref_resolver.resolve(ref_url) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| definition = ref_resolver.lookup(ref_url).contents | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| _expand_refs( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| definition, ref_resolver=ref_resolver | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) # expand refs in definitions as well | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | @@ -90,10 +106,14 @@ def _expand_refs(schema: Any, ref_resolver: Optional[RefResolver] = None) -> Non | |||||||||||||||||||||||||||||||||||||||||||||||||||
| def expand_refs(schema: Any) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Iterate over schema and replace all occurrences of $ref with their definitions. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| If a "definitions" section is present at the root of the schema, it will be removed | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| after $ref resolution is complete. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param schema: schema that will be patched | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| _expand_refs(schema) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| schema.pop("definitions", None) # remove definitions created by $ref | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ref_resolver = get_ref_resolver_registry(schema).resolver() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| _expand_refs(schema, ref_resolver) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| schema.pop("definitions", None) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| 
      Comment on lines
    
      106
     to 
      117
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainReturn the schema for ergonomic use and back-compat Some call sites (tests, etc.) assign  -def expand_refs(schema: Any) -> None:
+def expand_refs(schema: Any) -> Any:
@@
-    schema.pop("definitions", None)
+    schema.pop("definitions", None)
+    return schemaRun to find assignment-style usages that would otherwise break: 🏁 Script executed: #!/bin/bash
# Find assignment-style expand_refs usage
rg -nP --type=py '^\s*\w+\s*=\s*expand_refs\s*\(' -C1
# Find all expand_refs call sites for a quick audit
rg -nP --type=py '\bexpand_refs\s*\(' -C2Length of output: 3461 Return schema from expand_refs for ergonomic use and back-compat? One assignment-style caller was found at unit_tests/destinations/vector_db_based/config_test.py:67 — change the signature to return the schema so both assignment and in-place usages work. -def expand_refs(schema: Any) -> None:
+def expand_refs(schema: Any) -> Any:
@@
-    schema.pop("definitions", None)
+    schema.pop("definitions", None)
+    return schemawdyt? 📝 Committable suggestion
 
        Suggested change
       
 🤖 Prompt for AI Agents | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| def rename_key(schema: Any, old_key: str, new_key: str) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|  | @@ -3,10 +3,25 @@ | |||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||||||||||
| from copy import deepcopy | ||||||||||||||||||||||||||||||||||||||||
| from enum import Flag, auto | ||||||||||||||||||||||||||||||||||||||||
| from typing import Any, Callable, Dict, Generator, Mapping, Optional, cast | ||||||||||||||||||||||||||||||||||||||||
| from typing import TYPE_CHECKING, Any, Callable, Dict, Generator, Mapping, Optional, cast | ||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||
| from jsonschema import Draft7Validator, ValidationError, validators | ||||||||||||||||||||||||||||||||||||||||
| from referencing import Registry, Resource | ||||||||||||||||||||||||||||||||||||||||
| from referencing._core import Resolver | ||||||||||||||||||||||||||||||||||||||||
| from referencing.exceptions import Unresolvable | ||||||||||||||||||||||||||||||||||||||||
| from referencing.jsonschema import DRAFT7 | ||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||
| from airbyte_cdk.sources.utils.schema_helpers import expand_refs | ||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||
| from .schema_helpers import get_ref_resolver_registry | ||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||
| from jsonschema.validators import Validator | ||||||||||||||||||||||||||||||||||||||||
| except: | ||||||||||||||||||||||||||||||||||||||||
| 
     | ||||||||||||||||||||||||||||||||||||||||
| except: | |
| except ImportError: | 
    
      
    
      Copilot
AI
    
    
    
      Sep 11, 2025 
    
  
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expand_refs function is called multiple times and mutates the schema in-place. Consider creating a deep copy of the schema before the first call to avoid unintended side effects on the original schema object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t expand refs on every validator callback (perf + possible Unresolvable)
Calling expand_refs(schema) per-visit mutates the schema repeatedly and can raise Unresolvable when expanding dict-valued fragments without root context. Can we remove in-callback expansion and do a single pre-expansion before validator creation, wdyt?
Apply:
-            # Very first step is to expand $refs in the schema itself:
-            expand_refs(schema)
-
-            # Now we can expand $refs in the property value:
-            if isinstance(property_value, dict):
-                expand_refs(property_value)
+            # $refs are pre-expanded once before validator construction.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Very first step is to expand $refs in the schema itself: | |
| expand_refs(schema) | |
| # Now we can expand $refs in the property value: | |
| if isinstance(property_value, dict): | |
| expand_refs(property_value) | |
| def resolve(subschema: dict[str, Any]) -> dict[str, Any]: | |
| if "$ref" in subschema: | |
| _, resolved = cast( | |
| RefResolver, | |
| validator_instance.resolver, | |
| ).resolve(subschema["$ref"]) | |
| return cast(dict[str, Any], resolved) | |
| return subschema | |
| # Now we can validate and normalize the values: | |
| # $refs are pre-expanded once before validator construction. | |
| # Now we can validate and normalize the values: | 
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function call should match the pattern used elsewhere in the codebase. The original code assigned the result back to
schema, butexpand_refsmodifies the schema in-place and returns None. Consider documenting this behavior or maintaining consistency with the assignment pattern for clarity.