Skip to content

Conversation

brianjlai
Copy link
Contributor

@brianjlai brianjlai commented Aug 4, 2025

Summary by CodeRabbit

  • New Features
    • Added support for conditional manifest migration and normalization during source creation, based on configuration settings.

@brianjlai brianjlai requested a review from lmossman August 4, 2025 20:45
Copy link

github-actions bot commented Aug 4, 2025

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

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@brian/add_back_migrate_and_normalize_for_connector_builder#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 brian/add_back_migrate_and_normalize_for_connector_builder

Helpful Resources

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
  • /poe <command> - Runs any poe command in the CDK environment

📝 Edit this welcome message.

@github-actions github-actions bot added the bug Something isn't working label Aug 4, 2025
Copy link
Contributor

coderabbitai bot commented Aug 4, 2025

📝 Walkthrough

Walkthrough

Two new boolean flags, migrate_manifest and normalize_manifest, are introduced to control manifest migration and normalization in the Airbyte connector builder. These flags are determined from the configuration and passed through the source creation logic, ultimately being added to the constructor of ConcurrentDeclarativeSource.

Changes

Cohort / File(s) Change Summary
Connector Builder Handler
airbyte_cdk/connector_builder/connector_builder_handler.py
Adds logic to extract migrate_manifest and normalize_manifest flags from config and passes them as arguments to ConcurrentDeclarativeSource during source creation.
Concurrent Declarative Source
airbyte_cdk/sources/declarative/concurrent_declarative_source.py
Updates ConcurrentDeclarativeSource constructor to accept migrate_manifest and normalize_manifest boolean parameters (defaulting to False) and forwards them to the superclass constructor.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Handler as connector_builder_handler.py
    participant Source as ConcurrentDeclarativeSource
    participant Super as ManifestDeclarativeSource

    User->>Handler: create_source(config)
    Handler->>Handler: should_migrate_manifest(config)
    Handler->>Handler: should_normalize_manifest(config)
    Handler->>Source: Instantiate with migrate_manifest, normalize_manifest
    Source->>Super: Call __init__ with migrate_manifest, normalize_manifest
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Would you like to see a suggestion for adding a docstring or comment to clarify the new parameters in the constructor for future maintainers, wdyt?

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 950acc6 and f907337.

📒 Files selected for processing (2)
  • airbyte_cdk/connector_builder/connector_builder_handler.py (1 hunks)
  • airbyte_cdk/sources/declarative/concurrent_declarative_source.py (2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: aaronsteers
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in `airbyte_cdk/cli/source_declarative_manifest/`, including `_run.py`, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
Learnt from: aaronsteers
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/cli/source_declarative_manifest/spec.json:9-15
Timestamp: 2024-11-15T00:59:08.154Z
Learning: When code in `airbyte_cdk/cli/source_declarative_manifest/` is being imported from another repository, avoid suggesting modifications to it during the import process.
Learnt from: ChristoGrab
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the `YamlDeclarativeSource` class in `airbyte_cdk/sources/declarative/yaml_declarative_source.py`, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
📚 Learning: when modifying the `yamldeclarativesource` class in `airbyte_cdk/sources/declarative/yaml_declarativ...
Learnt from: ChristoGrab
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the `YamlDeclarativeSource` class in `airbyte_cdk/sources/declarative/yaml_declarative_source.py`, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.

Applied to files:

  • airbyte_cdk/sources/declarative/concurrent_declarative_source.py
  • airbyte_cdk/connector_builder/connector_builder_handler.py
📚 Learning: the files in `airbyte_cdk/cli/source_declarative_manifest/`, including `_run.py`, are imported from ...
Learnt from: aaronsteers
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in `airbyte_cdk/cli/source_declarative_manifest/`, including `_run.py`, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.

Applied to files:

  • airbyte_cdk/sources/declarative/concurrent_declarative_source.py
  • airbyte_cdk/connector_builder/connector_builder_handler.py
📚 Learning: when code in `airbyte_cdk/cli/source_declarative_manifest/` is being imported from another repositor...
Learnt from: aaronsteers
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/cli/source_declarative_manifest/spec.json:9-15
Timestamp: 2024-11-15T00:59:08.154Z
Learning: When code in `airbyte_cdk/cli/source_declarative_manifest/` is being imported from another repository, avoid suggesting modifications to it during the import process.

Applied to files:

  • airbyte_cdk/sources/declarative/concurrent_declarative_source.py
  • airbyte_cdk/connector_builder/connector_builder_handler.py
📚 Learning: copying files from `site-packages` in the dockerfile maintains compatibility with both the old file ...
Learnt from: ChristoGrab
PR: airbytehq/airbyte-python-cdk#90
File: Dockerfile:16-21
Timestamp: 2024-12-02T18:36:04.346Z
Learning: Copying files from `site-packages` in the Dockerfile maintains compatibility with both the old file structure that manifest-only connectors expect and the new package-based structure where SDM is part of the CDK.

Applied to files:

  • airbyte_cdk/connector_builder/connector_builder_handler.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Check: source-intercom
  • GitHub Check: Check: destination-motherduck
  • GitHub Check: Check: source-pokeapi
  • GitHub Check: Check: source-hardcoded-records
  • GitHub Check: Check: source-shopify
  • GitHub Check: SDM Docker Image Build
  • GitHub Check: Pytest (All, Python 3.10, Ubuntu)
  • GitHub Check: Pytest (All, Python 3.11, Ubuntu)
  • GitHub Check: Pytest (Fast)
  • GitHub Check: Analyze (python)
🔇 Additional comments (5)
airbyte_cdk/connector_builder/connector_builder_handler.py (3)

46-53: LGTM! Clean helper function implementation.

The function correctly extracts the migration flag from the config with a sensible default. The docstring clearly explains the purpose and that it's set by the UI. Nice work on the clear implementation!


56-62: LGTM! Consistent implementation pattern.

This follows the same pattern as should_migrate_manifest which is great for consistency. The function name and logic are clear and appropriate.


86-87: LGTM! Proper integration of the new flags.

The flags are correctly passed to the ConcurrentDeclarativeSource constructor using the helper functions. This maintains clean separation of concerns. The implementation looks solid!

airbyte_cdk/sources/declarative/concurrent_declarative_source.py (2)

98-99: LGTM! Backward-compatible parameter addition.

The new optional parameters with False defaults maintain backward compatibility while enabling the new functionality. The parameter names are clear and consistent with the usage in the connector builder handler. Nice approach!


140-141: LGTM! Proper parameter forwarding.

The parameters are correctly passed through to the superclass constructor, maintaining the chain of configuration. This ensures the manifest migration and normalization flags reach the appropriate base class implementation.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch brian/add_back_migrate_and_normalize_for_connector_builder

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@lmossman lmossman left a comment

Choose a reason for hiding this comment

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

Changes LGTM and works as expected when testing locally with the Builder

Copy link

github-actions bot commented Aug 4, 2025

PyTest Results (Fast)

3 699 tests  ±0   3 688 ✅ ±0   6m 41s ⏱️ +5s
    1 suites ±0      11 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit f907337. ± Comparison against base commit 950acc6.

Copy link

github-actions bot commented Aug 4, 2025

PyTest Results (Full)

3 702 tests  ±0   3 691 ✅ ±0   11m 43s ⏱️ ±0s
    1 suites ±0      11 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit f907337. ± Comparison against base commit 950acc6.

♻️ This comment has been updated with latest results.

@brianjlai brianjlai merged commit db1e4c0 into main Aug 4, 2025
28 of 31 checks passed
@brianjlai brianjlai deleted the brian/add_back_migrate_and_normalize_for_connector_builder branch August 4, 2025 21:47
lmossman added a commit that referenced this pull request Aug 5, 2025
brianjlai added a commit that referenced this pull request Aug 5, 2025
@coderabbitai coderabbitai bot mentioned this pull request Aug 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants