Skip to content

fix: source asset should pass schema_path to catalog #67

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

Merged
merged 1 commit into from
Jul 4, 2025

Conversation

tomasvotava
Copy link
Owner

No description provided.

@tomasvotava tomasvotava requested a review from Copilot July 4, 2025 11:20
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new schema_path attribute to SourceAsset and updates the asset processing logic to forward this field to the catalog.

  • Introduce schema_path on SourceAsset
  • Update both async and sync loops in _run to pass schema_path to store_asset_async
Comments suppressed due to low confidence (3)

hyperion/sources/base.py:29

  • Consider updating the class docstring or adding a comment for SourceAsset to explain the purpose and expected format of the new schema_path field.
    schema_path: str | None = None

hyperion/sources/base.py:64

  • Add or update tests to verify that schema_path is correctly forwarded to store_asset_async, covering both non-null and None cases.
                            asset.asset, asset.data, notify=notify, schema_path=asset.schema_path

hyperion/sources/base.py:64

  • Ensure that catalog.store_asset_async has been updated to accept the schema_path keyword argument; otherwise this change will cause a TypeError at runtime.
                            asset.asset, asset.data, notify=notify, schema_path=asset.schema_path

@tomasvotava tomasvotava merged commit 4fbccac into master Jul 4, 2025
8 checks passed
@tomasvotava tomasvotava deleted the fix/source-asset-ignores-schema-path branch July 4, 2025 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant