Skip to content

fix(low-code): parse manifest and component schema with libyaml CSafeLoader when available - #1223

Merged
Patrick Nilan (pnilan) merged 3 commits into
mainfrom
devin/1791499901-yaml-csafeloader
Oct 22, 2026
Merged

Patrick Nilan (pnilan) merged 3 commits into
mainfrom
devin/1791499901-yaml-csafeloader

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Resolves https://github.com/airbytehq/airbyte-internal-issues/issues/17932:

Close on merge: yes — internally filed, no customer to confirm with.

Part of epic https://github.com/airbytehq/airbyte-internal-issues/issues/17920. Requested by Patrick Nilan (Patrick Nilan (@pnilan)).

At startup, every manifest-only connector parses its manifest.yaml and the CDK's ~5.9k-line declarative_component_schema.yaml with PyYAML's pure-Python SafeLoader. This PR parses both with libyaml's CSafeLoader when PyYAML was built with libyaml, and falls back to SafeLoader otherwise (without libyaml, accessing yaml.CSafeLoader raises AttributeError).

New helper in airbyte_cdk/utils/yaml_loader.py:

def safe_load_yaml(stream):     # replaces yaml.safe_load at the call sites below
    loader = _get_safe_loader()  # CSafeLoader if PyYAML has libyaml, else SafeLoader
    # exact str/bytes and seekable streams try libyaml first; str/bytes subclasses,
    # non-seekable streams and mocks go straight to SafeLoader (same behaviour as today)
    if loader is yaml.SafeLoader or (not exact_text and rewind_to is None):
        return yaml.load(stream, Loader=yaml.SafeLoader)
    try:
        return yaml.load(stream, Loader=loader)
    except yaml.YAMLError:
        pass                     # second parse runs outside the handler: no chained libyaml error
    if rewind_to is not None:
        stream.seek(rewind_to)
    return yaml.load(stream, Loader=yaml.SafeLoader)   # raises today's error message

Call sites switched to safe_load_yaml:

  • YamlDeclarativeSource._read_and_parse_yaml_file and YamlDeclarativeSource._parse
  • concurrent_declarative_source._get_declarative_component_schema
  • cli/source_declarative_manifest/_run._parse_manifest_from_file (--manifest-path)
  • legacy/.../manifest_declarative_source._get_declarative_component_schema. The issue doesn't name this one, but it is the same schema load, so I changed it too.

Left unchanged, because the issue puts them out of scope: the small-input yaml.safe_load calls in config_components_resolver, parametrized_components_resolver and migrations_registry, plus connector.py and the test utilities.

Measured locally (linux/amd64, CDK poetry env, PyYAML 6.0.2, yaml.__with_libyaml__ == True)

Component schema parse (min of 5) SafeLoader CSafeLoader
declarative_component_schema.yaml 394 ms 42.5 ms

Both loaders produce equal objects. This PR does not add the before/after row to the benchmark table in the epic (https://github.com/airbytehq/airbyte-internal-issues/issues/17920). Patrick Nilan (@pnilan), that row still needs to be measured and added.

Behavior notes for reviewers

  • Parity on real manifests. In Patrick Nilan (@pnilan)'s review, all 539 manifest.yaml files in airbytehq/airbyte, plus the component schema, parsed to identical objects with SafeLoader and CSafeLoader. The comparison checked types, key order and anchor sharing across str, bytes and file input, on PyYAML 6.0.2/6.0.3 and Python 3.11/3.13/3.14.

  • Syntax errors. If libyaml raises a yaml.YAMLError, the input is parsed again with SafeLoader and that error is raised, without libyaml's error chained as __context__. Error type, message (including the source-line snippet for string input) and traceback match what callers saw before this PR. Seekable streams are rewound first.

  • Streams that can't seek, mock handles and str/bytes subclasses go straight to SafeLoader. The C parser accepts only exact str/bytes, and these streams couldn't be parsed a second time.

  • Not fully equivalent to yaml.safe_load. When libyaml succeeds, a few inputs give results that differ from SafeLoader's. None of them occur in a shipped manifest:

    • Input libyaml now accepts that SafeLoader rejected:
      • tab separators (a:\tb → {'a': 'b'}, [a,\tb], a: 1\t# c)
      • the |# block header and [a?b]
      • nesting deeper than about 500 levels
    • Input that now parses to a different value:
      • a: ! gives '' instead of None
      • a U+FEFF at the start of a later line is dropped
      • constructors and resolvers registered on yaml.SafeLoader are ignored, because CSafeLoader doesn't inherit from it
    • Input that now raises a different exception:
      • a lone surrogate in a str raises UnicodeEncodeError instead of ReaderError
      • nesting of about 25,000 levels or more crashes the process instead of raising RecursionError

    The docstring says to use the helper only for trusted input. For that reason these stay on yaml.safe_load:

    • config_components_resolver and parametrized_components_resolver: they rely on SafeLoader's tab error.
    • test/standard_tests/declarative_sources.py: it's the strict check that keeps libyaml-only manifests out of the monorepo.
  • _get_safe_loader is private, so pdoc doesn't publish it.

Test Coverage

unit_tests/utils/test_yaml_loader.py:

Tests confirmed to fail on the old code:

  • test_declarative_call_sites_use_c_loader: checks that all five call sites use yaml.CSafeLoader. It fails with the old call sites (assert 0 == 1).
  • These fail without the second parse, and match a direct SafeLoader parse (str(error), type, __context__ is None):
    • test_syntax_error_in_string_matches_pure_python_message (also runs through YamlDeclarativeSource._parse)
    • test_syntax_error_in_file_matches_pure_python_message
  • These four fail on the previous revision (4864013):
    • test_syntax_error_does_not_chain_c_loader_error
    • test_non_seekable_stream_parses_and_errors_like_safe_loader
    • test_mock_open_handle_with_invalid_yaml_raises
    • test_str_subclass_parses

Other tests:

  • test_successful_parse_never_uses_pure_python_loader: a spy on yaml.SafeLoader is never created for valid str, bytes or file input.
  • test_pure_python_result_returned_if_only_c_loader_fails
  • test_c_loader_accepts_tab_separators_that_safe_loader_rejects: pins one of the known differences.
  • test_component_schema_parses_identically_with_both_loaders: compares with == and repr (types, key order).
  • test_malformed_yaml_raises_parser_error_with_c_loader and ..._without_libyaml
  • test_falls_back_to_pure_python_loader_without_libyaml and test_get_safe_loader_uses_c_loader_when_available
  • The module collects without libyaml. I simulated this with del yaml.CSafeLoader: 3 passed, 12 skipped.

Run locally:

  • ruff check, ruff format and mypy: clean.
  • test_yaml_loader.py, test_yaml_declarative_source.py, test_concurrent_declarative_source.py, unit_tests/source_declarative_manifest and unit_tests/legacy: 266 passed, 1 skipped.
  • unit_tests/sources/declarative on the first revision: 2345 passed.

Declarative-First Evaluation

Not applicable: this changes the CDK framework, not a declarative connector's manifest or custom components.

Breaking change evaluation

Not breaking:

  • All 539 monorepo manifests and the component schema parse to identical objects.
  • Syntax errors keep the same exception and message.
  • No schema, spec, stream or state changes.

The differences listed above only affect input that no shipped manifest uses. The release drafter proposes a patch version from the fix title type, and a maintainer publishes the release (docs/RELEASES.md).

Link to Devin session: https://app.devin.ai/sessions/aa6c3fbe780246f99b7658fa21e285ac
Open in Devin Desktop: https://app.devin.ai/desktop/session/aa6c3fbe780246f99b7658fa21e285ac?variant=devin

Note

Auto-merge may have been disabled. Please check the PR status to confirm.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

Testing This CDK Version

You can test this version of the CDK using the following:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1791499901-yaml-csafeloader#egg=airbyte-python-cdk[dev]' --help

# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1791499901-yaml-csafeloader

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /autofix - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test - Runs connector tests with the updated CDK
  • /prerelease - Triggers a prerelease publish with default arguments
  • /poe build - Regenerate git-committed build artifacts, such as the pydantic models which are generated from the manifest JSON schema in YAML.
  • /poe <command> - Runs any poe command in the CDK environment
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

PyTest Results (Fast)

5 854 tests   5 823 ✅  5m 44s ⏱️
    1 suites     31 💤
    1 files        0 ❌

Results for commit 766f1d3.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

PyTest Results (Full)

5 857 tests   5 826 ✅  11m 3s ⏱️
    1 suites     31 💤
    1 files        0 ❌

Results for commit 766f1d3.

♻️ This comment has been updated with latest results.

@pnilan

Copy link
Copy Markdown
Member

🤖 This comment was generated by an AI Agent.

The issue this PR resolves has one addition, under "Proposed change" and "Acceptance criteria" of airbytehq/airbyte-internal-issues#17932. Please update the PR for it.

Keep today's message for a YAML syntax error

When the C loader raises a yaml.YAMLError, parse the same input again with yaml.SafeLoader and raise that error instead. The second parse runs only after the first has failed, so the success path is unchanged.

Why: the C loader's message keeps line and column but drops the source-line snippet when the YAML is read from a string, which is how a connector image reads its manifest (YamlDeclarativeSource._parse).

Example: a list item indented one space short

Pure-Python loader, today:

while parsing a block collection
  in "<unicode string>", line 7, column 3:
      - type: DeclarativeStream
      ^
expected <block end>, but found '<block mapping start>'
  in "<unicode string>", line 9, column 4:
       primary_key: id
       ^

C loader:

while parsing a block collection
  in "<unicode string>", line 7, column 3
did not find expected '-' indicator
  in "<unicode string>", line 9, column 4

Please cover:

  • safe_load_yaml given a string with a syntax error raises an error whose str() equals the one from yaml.load(text, Loader=yaml.SafeLoader).
  • The same for a file object: the message matches SafeLoader's for that file, which names the file and has no snippet. The stream has to be rewound, or read once, before the second parse. If it cannot be rewound, raise the C loader's error.
  • A parse that succeeds never calls the pure-Python loader.

The "Behavior notes for reviewers" line in the description, which says the snippet is omitted, will need updating too.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Patrick Nilan (@pnilan) re: Keep today's message for a YAML syntax error (#1223 (comment)). Done in 4864013.

When the C loader raises a yaml.YAMLError, safe_load_yaml now parses the same input again with yaml.SafeLoader and raises that error:

  • Strings and bytes: the second parse uses the input directly.
  • File objects: the stream is rewound to where it started before the second parse.
  • Streams that can't be rewound: the C loader's error is raised as-is.

Parsing valid YAML never touches the pure-Python loader.

New tests in unit_tests/utils/test_yaml_loader.py:

  • String input: str(error) matches a direct SafeLoader parse, including the ^ snippet. Checked through both safe_load_yaml and YamlDeclarativeSource._parse. This test fails without the second parse.
  • File object: the error matches a direct SafeLoader parse and names the file path. This test fails without the second parse.
  • Stream that can't seek: the C loader's error is kept.
  • Valid input: for str, bytes and file input, a spy on yaml.SafeLoader is never created.

I updated the "Behavior notes" section of the PR description to match.

Written by Devin

@pnilan Patrick Nilan (pnilan) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 This review was generated by an AI Agent.

Review at 4864013c: fix before merge (not breaking)

The parser change is safe for existing connectors. All 539 manifest.yaml files in airbytehq/airbyte, plus the component schema, parse to identical objects with SafeLoader and CSafeLoader. The comparison covered str, bytes and file input, and checked types, key order and anchor sharing, on PyYAML 6.0.2 and 6.0.3 with Python 3.11, 3.13 and 3.14.

The items below are the title, edge cases in the new helper, and claims in the docstring and the description that aren't accurate. Please address 1–6. Item 7 is lower priority.

1. Retitle the PR (P1, blocks merge)

perf isn't an allowed type in .github/workflows/semantic_pr_check.yml:27-41, and Validate PR title is a required check on main. It reports pass today only because both of its steps skip on drafts, so the PR will turn red when it's marked ready. fix gets the same patch bump from the release drafter.

fix(low-code): parse manifest and component schema with libyaml CSafeLoader when available

Also fix the description's "semantic-release sets the CDK version from the PR title". The release drafter proposes a version from the title type, and a maintainer publishes the release (docs/RELEASES.md).

2. Re-parse outside the except block (P2)

Where: airbyte_cdk/utils/yaml_loader.py:42-48

The SafeLoader re-parse runs inside except yaml.YAMLError:, so the error it raises carries libyaml's error as __context__. str(error) is unchanged. But traceback.format_exc() at cli/source_declarative_manifest/_run.py:121 and :235 now prints libyaml's error first, then "During handling of the above exception, another exception occurred". For the misindented example in the tests, the traceback grows from 46 to 70 lines. That puts libyaml's wording back into the trace's stack_trace, which the fallback exists to avoid.

Fix: exit the except block, then rewind and re-parse after it (see the module under item 3). Add assert exc_info.value.__context__ is None to the string and file message tests.

3. Send non-seekable streams, mock handles and str/bytes subclasses straight to SafeLoader (P2)

Where: airbyte_cdk/utils/yaml_loader.py:15-25 and :45-47

  • Non-seekable streams. A FIFO or /dev/stdin passed to --manifest-path gets libyaml's verdict, even on input SafeLoader accepts. description: "\ud83d\ude00 emoji" uses the escape json.dumps emits for emoji. It parsed before; it now raises ScannerError.
  • Mock handles. A MagicMock or mock_open handle passes the duck-typed seekable() check, and its seek does nothing. SafeLoader then re-reads an exhausted buffer, so a YAML syntax error returns None, and _parse_manifest_from_file reports "Manifest file at … is empty".
  • Subclasses. A str subclass raises TypeError, where yaml.safe_load parsed it. The C parser accepts only exact str and bytes, and TypeError isn't a YAMLError, so the fallback never runs.

This supersedes the earlier request in #1223 (comment) to raise the C loader's error for streams that can't be rewound. Parsing those streams with SafeLoader from the start gives today's message without a second parse.

Suggested airbyte_cdk/utils/yaml_loader.py (covers items 2, 3, 4 and 7)
#
# Copyright (c) 2025 Airbyte, Inc., all rights reserved.
#

from __future__ import annotations

from typing import IO, Any, Optional, Union, cast

import yaml


def _get_safe_loader() -> Union[type[yaml.SafeLoader], type[yaml.CSafeLoader]]:
    """Return libyaml's CSafeLoader when PyYAML was built with it, else the pure-Python SafeLoader."""
    # The annotations stay unevaluated: yaml.CSafeLoader does not exist without libyaml.
    return cast(
        "Union[type[yaml.SafeLoader], type[yaml.CSafeLoader]]",
        getattr(yaml, "CSafeLoader", yaml.SafeLoader),
    )


def _tell_if_seekable(stream: Union[IO[str], IO[bytes]]) -> Optional[int]:
    """Return the stream's current position if it can be rewound, else None."""
    try:
        if stream.seekable() is not True:
            return None
        position = stream.tell()
    except (AttributeError, OSError, ValueError):
        return None
    return position if isinstance(position, int) else None


def safe_load_yaml(stream: Union[str, bytes, IO[str], IO[bytes]]) -> Any:
    """Parse trusted YAML like yaml.safe_load, using libyaml's CSafeLoader when available.

    Results match yaml.safe_load for documents both loaders accept. libyaml accepts
    some input SafeLoader rejects (for example a tab after `key:`), ignores constructors
    and resolvers registered on yaml.SafeLoader, and can crash the process on very deeply
    nested input, so do not use this for untrusted input. When libyaml raises a
    YAMLError, the input is parsed again with SafeLoader so callers see SafeLoader's error.
    """
    loader = _get_safe_loader()
    exact_text = type(stream) in (str, bytes)
    rewind_to = None
    if not exact_text and not isinstance(stream, (str, bytes)):
        rewind_to = _tell_if_seekable(stream)
    # str/bytes subclasses, non-seekable streams and mocks keep today's behaviour.
    if loader is yaml.SafeLoader or (not exact_text and rewind_to is None):
        return yaml.load(stream, Loader=yaml.SafeLoader)
    try:
        return yaml.load(stream, Loader=loader)
    except yaml.YAMLError:
        pass  # Re-parse outside the handler so libyaml's error is not chained.
    if rewind_to is not None:
        cast(Union[IO[str], IO[bytes]], stream).seek(rewind_to)
    return yaml.load(stream, Loader=yaml.SafeLoader)

Checked against the current helper:

  • It gives the same results and error messages as yaml.safe_load for str, bytes, file, BytesIO, str-subclass and non-seekable input.
  • With a mock_open handle it raises ParserError, as yaml.safe_load does; the current helper returns None.
  • No error carries a __context__.
  • A successful parse never creates a SafeLoader.
  • It imports and parses with yaml.CSafeLoader removed.
  • It passes mypy with the repo's mypy.ini.

Tests: replace test_syntax_error_in_non_seekable_stream_keeps_c_loader_message (unit_tests/utils/test_yaml_loader.py:164). On the current head, the first four tests below fail; with the suggested module, all five pass. They reuse the file's MISINDENTED_YAML and requires_libyaml, and need from unittest import mock.

Suggested tests
ESCAPED_EMOJI_YAML = 'description: "\\ud83d\\ude00 emoji"\n'


class _NonSeekable(io.StringIO):
    def seekable(self) -> bool:
        return False


@requires_libyaml
def test_syntax_error_does_not_chain_c_loader_error() -> None:
    with pytest.raises(yaml.YAMLError) as exc_info:
        safe_load_yaml(MISINDENTED_YAML)
    assert exc_info.value.__context__ is None


@requires_libyaml
def test_non_seekable_stream_parses_and_errors_like_safe_loader() -> None:
    assert safe_load_yaml(_NonSeekable(ESCAPED_EMOJI_YAML)) == yaml.load(
        _NonSeekable(ESCAPED_EMOJI_YAML), Loader=yaml.SafeLoader
    )
    with pytest.raises(yaml.YAMLError) as expected:
        yaml.load(_NonSeekable(MISINDENTED_YAML), Loader=yaml.SafeLoader)
    with pytest.raises(yaml.YAMLError) as actual:
        safe_load_yaml(_NonSeekable(MISINDENTED_YAML))
    assert str(actual.value) == str(expected.value)


@requires_libyaml
def test_mock_open_handle_with_invalid_yaml_raises() -> None:
    with pytest.raises(yaml.YAMLError):
        safe_load_yaml(mock.mock_open(read_data=MISINDENTED_YAML)())


def test_str_subclass_parses() -> None:
    class _Str(str):
        pass

    assert safe_load_yaml(_Str("a: 1")) == {"a": 1}


@requires_libyaml
def test_c_loader_accepts_tab_separators_that_safe_loader_rejects() -> None:
    with pytest.raises(yaml.scanner.ScannerError):
        yaml.load("a:\tb", Loader=yaml.SafeLoader)
    assert safe_load_yaml("a:\tb") == {"a": "b"}

The last test belongs to item 4. It pins current behaviour, so it passes on both versions.

4. Correct the equivalence claims (P2)

Where: airbyte_cdk/utils/yaml_loader.py:29 and the PR description

"Equivalent to yaml.safe_load" isn't true, and the description repeats it: "parsed objects, exception classes and error messages are the same as before". The fallback runs only when libyaml fails, so wherever libyaml succeeds, the result can differ:

  • Accepted now, rejected before:
    • tab separators: a:\tb → {'a': 'b'}, plus [a,\tb] and a: 1\t# c
    • the |# block header and [a?b]
    • nesting from about 500 to about 25,000 levels
  • Different value:
    • a: ! gives '' instead of None.
    • A U+FEFF at the start of a later line is dropped.
    • Constructors and resolvers registered on yaml.SafeLoader are ignored, because CSafeLoader doesn't inherit from it.
  • Different exception:
    • A lone surrogate in a str raises UnicodeEncodeError instead of ReaderError.
    • Nesting of about 25,000 levels or more segfaults (exit 139, no trace message) instead of raising RecursionError.

None of these appear in any shipped manifest, and none of the five call sites passes such input, so this isn't breaking. It matters for the next caller. resolvers/config_components_resolver.py:210 returns tab-containing config values unparsed by matching SafeLoader's tab error. Moving that call to this helper would turn "key:\tvalue" into {'key': 'value'}.

Fix:

  • Use the docstring in the suggested module above.
  • Add the tab test from the suggested tests.
  • Update "Behavior notes for reviewers" and "Breaking change evaluation" in the description. List these differences and the 539/539 parity result.

5. Make the test module collectable without libyaml (P2)

Where: unit_tests/utils/test_yaml_loader.py:74

yaml.CSafeLoader in the module-level parametrize is evaluated at import. Without libyaml, collection fails with AttributeError before any skipif applies. All 10 tests are lost, including test_falls_back_to_pure_python_loader_without_libyaml, the one test written for that environment. CI doesn't catch it, because every leg installs a libyaml wheel.

pytest.param(
    getattr(yaml, "CSafeLoader", None),
    marks=pytest.mark.skipif(not yaml.__with_libyaml__, reason="PyYAML was built without libyaml"),
),

6. Tighten the parity and malformed-YAML tests (P2)

Where: unit_tests/utils/test_yaml_loader.py:65 and :82

  • assert pure_python_schema == c_loader_schema treats True == 1 == 1.0 as equal and ignores key order. Compare repr() of both results, or walk them and check types.
  • test_malformed_yaml_raises_parser_error_with_both_loaders calls yaml.load directly, so no change to safe_load_yaml can make it fail. Send the malformed input through safe_load_yaml.

7. Make get_safe_loader private and type it (P3)

Where: airbyte_cdk/utils/yaml_loader.py:10, :15 and :23

docs/generate.py publishes every non-underscore module in the pdoc reference, so get_safe_loader becomes public API, although it exists for tests.

  • Rename it to _get_safe_loader and give it a real return type instead of Any.
  • Type _tell_if_seekable's parameter, so its cast goes away.
  • Keep the new annotation lazy. yaml.CSafeLoader doesn't exist without libyaml, so type[yaml.CSafeLoader] in an evaluated annotation, or in a cast whose type isn't a string, raises at import. The suggested module uses from __future__ import annotations and a string cast.
  • Update the two tests that import get_safe_loader.

Leave unchanged

  • airbyte_cdk/test/standard_tests/declarative_sources.py:76 stays on yaml.safe_load. It's the strict check that keeps manifests that only parse under libyaml out of the monorepo.
  • config_components_resolver.py:204 and parametrized_components_resolver.py:122 stay on yaml.safe_load, for the reason in item 4.

CI notes

  • Check: destination-motherduck fails because the MotherDuck token was rejected ("Your request is not authenticated"). It's also red on #1233 and #1229, so it isn't caused by this PR.
  • Skipped Check: source-google-drive didn't run. Validate PR title hasn't evaluated the title yet, because the PR is a draft.
  • Of the six Test Connectors entries, only source-intercom and source-pokeapi reach the changed parse path.
  • Still open from the issue: the measured before/after row for the table in airbytehq/airbyte-internal-issues#17920.

@devin-ai-integration devin-ai-integration Bot changed the title perf(low-code): parse manifest and component schema with libyaml CSafeLoader when available fix(low-code): parse manifest and component schema with libyaml CSafeLoader when available Oct 21, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Patrick Nilan (@pnilan), about your review at 4864013c (review 5604923506): all items are done in c24a48b.

  1. Title. The PR is now titled fix(low-code): …. I also corrected the release wording in the description: the release drafter proposes the version from the title type, and a maintainer publishes it.
  2. Chained libyaml error. SafeLoader now parses the input a second time outside the except block, so its error carries no libyaml __context__. The string and file message tests now assert __context__ is None, and test_syntax_error_does_not_chain_c_loader_error checks it directly.
  3. Streams that can't seek, mocks and str/bytes subclasses go straight to SafeLoader. I took your module verbatim; ruff and mypy needed no changes. I replaced the old non-seekable test with your four tests. Each of them failed on 4864013 and passes now.
  4. Equivalence claims. I used your docstring and added the tab test. The description's "Behavior notes" and "Breaking change evaluation" sections now list the known differences, the 539/539 parity result, and which callers stay on yaml.safe_load and why.
  5. Collecting without libyaml. No module-level yaml.CSafeLoader reference remains. With del yaml.CSafeLoader; yaml.__with_libyaml__=False the module collects and gives 3 passed, 12 skipped.
  6. Parity and malformed-YAML tests. The parity test now also compares repr() of both results. The malformed-YAML tests go through safe_load_yaml, once with libyaml and once with CSafeLoader removed.
  7. Private, typed helper. get_safe_loader is now _get_safe_loader, with a lazily evaluated Union[type[SafeLoader], type[CSafeLoader]] return type. _tell_if_seekable's parameter is typed.

I left the callers you listed under "Leave unchanged" alone.

Checks run locally:

  • ruff and mypy: clean.
  • test_yaml_loader.py, test_yaml_declarative_source.py, test_concurrent_declarative_source.py, unit_tests/source_declarative_manifest and unit_tests/legacy: 266 passed, 1 skipped.

The benchmark row for https://github.com/airbytehq/airbyte-internal-issues/issues/17920 is still open.

Written by Devin

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Documented parsing differences and deep-nesting crashes require maintainer acceptance and confirmation that updated entry points receive trusted input.

No open issues 🎉

What changed in this PR

Speeds up declarative connector startup by using libyaml for manifest and component-schema parsing when available.

Changes:

  • Adds a shared loader with pure-Python fallback and error-message preservation.
  • Updates five parsing call sites.
  • Adds tests for loader selection, compatibility, fallback behavior, and integrations.
File Description
unit_tests/​utils/​test_yaml_loader.py Tests loader behavior and integrations.
airbyte_cdk/​utils/​yaml_loader.py Implements the shared YAML loader.
airbyte_cdk/​sources/​declarative/​yaml_declarative_source.py Updates manifest parsing.
airbyte_cdk/​sources/​declarative/​concurrent_declarative_source.py Updates component-schema parsing.
airbyte_cdk/​legacy/​sources/​declarative/​manifest_declarative_source.py Updates legacy component-schema parsing.
airbyte_cdk/​cli/​source_declarative_manifest/​_run.py Updates CLI manifest-file parsing.

Review effort: Balanced

@pnilan
Patrick Nilan (pnilan) changed the base branch from main to devin/1791497733-sdm-precompile-bytecode October 21, 2026 21:57
@pnilan
Patrick Nilan (pnilan) added this pull request to stack #1245 October 21, 2026 21:57
Base automatically changed from devin/1791497733-sdm-precompile-bytecode to main October 22, 2026 02:25
devin-ai-integration Bot and others added 3 commits October 21, 2026 19:25
…eLoader when available

Co-Authored-By: bot_apk <apk@cognition.ai>
…r fails

Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
@pnilan
Patrick Nilan (pnilan) force-pushed the devin/1791499901-yaml-csafeloader branch from 2023244 to 766f1d3 Compare October 22, 2026 02:25
@pnilan
Patrick Nilan (pnilan) enabled stacked auto-merge (squash) October 22, 2026 02:25
@pnilan
Patrick Nilan (pnilan) disabled stacked auto-merge October 22, 2026 02:28
@pnilan
Patrick Nilan (pnilan) enabled stacked auto-merge (squash) October 22, 2026 02:31
@pnilan
Patrick Nilan (pnilan) merged commit f11816e into main Oct 22, 2026
28 of 29 checks passed
@pnilan
Patrick Nilan (pnilan) deleted the devin/1791499901-yaml-csafeloader branch October 22, 2026 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants