Skip to content

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

Draft
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1791499901-yaml-csafeloader
Draft

devin-ai-integration[bot] wants to merge 2 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 get_safe_loader():          # looked up on every call, so tests can simulate missing libyaml
    return getattr(yaml, "CSafeLoader", yaml.SafeLoader)

def safe_load_yaml(stream):     # drop-in replacement for yaml.safe_load
    loader = get_safe_loader()
    if loader is yaml.SafeLoader:
        return yaml.load(stream, Loader=yaml.SafeLoader)
    rewind_to = _tell_if_seekable(stream)        # None for str/bytes or streams that can't seek
    try:
        return yaml.load(stream, Loader=loader)
    except yaml.YAMLError:
        if isinstance(stream, (str, bytes)):
            return yaml.load(stream, Loader=yaml.SafeLoader)   # raises today's error message
        if rewind_to is None:
            raise                                # stream can't be rewound: keep the C loader's error
        stream.seek(rewind_to)
        return yaml.load(stream, Loader=yaml.SafeLoader)

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

  • CSafeLoader is not a subclass of SafeLoader, and nothing here depends on isinstance/issubclass.
  • Error messages for invalid YAML are unchanged. When the C loader raises a yaml.YAMLError, safe_load_yaml parses the same input again with yaml.SafeLoader and raises that error. The error type and message are therefore the same as before this PR, including the source-line snippet that the C loader leaves out for string input (which is how YamlDeclarativeSource._parse reads manifests). File objects are rewound to where they started before the second parse. If a stream can't be rewound, the C loader's error is raised. The second parse only runs after the first one fails, so parsing valid YAML never touches the pure-Python loader.

Test Coverage

New unit_tests/utils/test_yaml_loader.py:

  • test_declarative_call_sites_use_c_loader: spies on yaml.CSafeLoader and checks that all five call sites use it. Confirmed it fails with the old call sites (assert 0 == 1) and passes with the fix.
  • test_syntax_error_in_string_matches_pure_python_message: str(error) matches the error from a direct SafeLoader parse, both through safe_load_yaml and through YamlDeclarativeSource._parse, and the ^ snippet is present. Confirmed it fails without the second parse.
  • test_syntax_error_in_file_matches_pure_python_message: same check for an open file object; the message includes the file path. Confirmed it fails without the second parse.
  • test_syntax_error_in_non_seekable_stream_keeps_c_loader_message: a stream that can't seek gets the C loader's error, not an error from the fallback.
  • test_successful_parse_never_uses_pure_python_loader: with a spy on yaml.SafeLoader, valid str, bytes and file input never create a SafeLoader.
  • test_pure_python_result_returned_if_only_c_loader_fails.
  • test_falls_back_to_pure_python_loader_without_libyaml: with monkeypatch.delattr(yaml, "CSafeLoader"), SafeLoader is chosen and parsing (including YamlDeclarativeSource._parse) still works.
  • test_component_schema_parses_identically_with_both_loaders.
  • test_malformed_yaml_raises_parser_error_with_both_loaders.
  • test_get_safe_loader_uses_c_loader_when_available.

Run locally: ruff check, ruff format and mypy are clean. pytest unit_tests/sources/declarative gave 2345 passed, 1 skipped (including test_yaml_declarative_source.py::test_source_fails_for_invalid_yaml). The test_yaml_loader.py, test_yaml_declarative_source.py and source_declarative_manifest CLI tests pass with the error-message change.

Declarative-First Evaluation

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

Breaking change evaluation

Not breaking: parsed objects, exception classes and error messages are the same as before, and no schema, spec, stream or state changes. semantic-release sets the CDK version from the PR title.

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

…eLoader when available

Co-Authored-By: bot_apk <apk@cognition.ai>
@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 074 tests  +40   5 062 ✅ +40   5m 49s ⏱️ - 4m 52s
    1 suites ± 0      12 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit 4864013. ± Comparison against base commit 916d6f6.

♻️ 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 077 tests  +40   5 065 ✅ +40   10m 45s ⏱️ - 4m 47s
    1 suites ± 0      12 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit 4864013. ± Comparison against base commit 916d6f6.

♻️ This comment has been updated with latest results.

@pnilan

Copy link
Copy Markdown
Collaborator

🤖 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.

…r fails

Co-Authored-By: bot_apk <apk@cognition.ai>
@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

This branch has not been deployed

No deployments
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.

1 participant