fix: override configure method in `ManifestDeclarativeSource` to set transformed config in entrypoint #611

pnilan · 2025-06-25T17:56:00Z

What

The entrypoint was passing the untransformed config to each operation methods (check/discover/read)
- Note: The migrated config was still being properly handled. This issue is isolated to the transformed config.
This fix overrides the Source method configure in the ManifestDeclarativeSource ultimately returning the transformed config to the entrypoint, where the config can then be validated against the spec and passed to each operation method.

Summary by CodeRabbit

New Features
- Added a configuration method to enable easier setup of sources using custom settings.
Bug Fixes
- Improved configuration transformations to support adding and remapping fields, ensuring accurate and enriched source setup.

Copilot

Pull Request Overview

This PR fixes an issue by overriding the configure method in ManifestDeclarativeSource so that the migrated/transformed configuration stored in self._config is used when available.

Adds an override for the configure method that selects between self._config and the provided config.
Ensures the transformed configuration is passed to the parent class implementation for validation and further processing.

Comments suppressed due to low confidence (1)

airbyte_cdk/sources/declarative/manifest_declarative_source.py:241

Consider adding a docstring for the configure method to explain its behavior and how it chooses between self._config and the passed config.

    def configure(self, config: Mapping[str, Any], temp_dir: str

coderabbitai · 2025-06-25T17:59:17Z

📝 Walkthrough

Walkthrough

A configure method was introduced to the ManifestDeclarativeSource class, allowing it to resolve and assign configuration data before invoking the superclass's configure method. The related test was enhanced to verify configuration transformations including adding and remapping fields, and to assert that the configure method returns the expected config. No other logic or error handling was modified.

Changes

File(s)	Change Summary
airbyte_cdk/sources/declarative/manifest_declarative_source.py	Added a `configure` method to `ManifestDeclarativeSource` for handling configuration assignment and delegation.
unit_tests/sources/declarative/test_manifest_declarative_source.py	Expanded test to verify configuration transformations and the behavior of the new `configure` method.

Suggested labels

airbyte-python-cdk

Suggested reviewers

pnilan
brianjlai

Would you like to add a dedicated test specifically for edge cases or error handling of the new configure method, or do you feel the current test coverage is sufficient for now? Wdyt?

📜 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 2729b18 and f0d328f.

📒 Files selected for processing (1)

unit_tests/sources/declarative/test_manifest_declarative_source.py (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

unit_tests/sources/declarative/test_manifest_declarative_source.py

⏰ Context from checks skipped due to timeout of 90000ms (9)

GitHub Check: Check: source-hardcoded-records
GitHub Check: Check: source-pokeapi
GitHub Check: Check: source-intercom
GitHub Check: Check: source-shopify
GitHub Check: Check: source-amplitude
GitHub Check: SDM Docker Image Build
GitHub Check: Pytest (Fast)
GitHub Check: Pytest (All, Python 3.11, Ubuntu)
GitHub Check: Pytest (All, Python 3.10, Ubuntu)

✨ Finishing Touches

📝 Generate Docstrings

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.
- @coderabbitai modularize this function.
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.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

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

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

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

coderabbitai

Actionable comments posted: 1

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 8e36382 and a816a8c.

📒 Files selected for processing (1)

airbyte_cdk/sources/declarative/manifest_declarative_source.py (1 hunks)

🧰 Additional context used

🪛 Flake8 (7.2.0)

airbyte_cdk/sources/declarative/manifest_declarative_source.py

[error] 242-242: closing bracket does not match visual indentation

(E124)

🪛 GitHub Actions: Linters

airbyte_cdk/sources/declarative/manifest_declarative_source.py

[error] 1-1: ruff formatting check failed. File would be reformatted. Run 'ruff format' to fix code style issues.

⏰ Context from checks skipped due to timeout of 90000ms (9)

GitHub Check: Publish SDM to DockerHub
GitHub Check: Check: source-amplitude
GitHub Check: Check: source-intercom
GitHub Check: Check: source-hardcoded-records
GitHub Check: Check: source-shopify
GitHub Check: Pytest (All, Python 3.11, Ubuntu)
GitHub Check: Pytest (All, Python 3.10, Ubuntu)
GitHub Check: Pytest (Fast)
GitHub Check: SDM Docker Image Build

airbyte_cdk/sources/declarative/manifest_declarative_source.py

brianjlai

change seems fine, but can we test this e2e w/ amazon ads to make sure this works.

Also how useful or easy would it be to unit test this. It's probably trivial or the result might be the same, but still feels like we should exercise the code path

github-actions · 2025-06-25T18:05:44Z

PyTest Results (Fast)

3 669 tests ±0 3 658 ✅ ±0 6m 8s ⏱️ -7s
1 suites ±0 11 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit f0d328f. ± Comparison against base commit 8e36382.

♻️ This comment has been updated with latest results.

github-actions · 2025-06-25T18:17:13Z

PyTest Results (Full)

3 672 tests ±0 3 661 ✅ ±0 18m 12s ⏱️ +13s
1 suites ±0 11 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit f0d328f. ± Comparison against base commit 8e36382.

♻️ This comment has been updated with latest results.

pnilan · 2025-06-25T20:13:26Z

Note:

I manually tested this change by creating a SDM dev image and then testing transformations on source-amazon-seller-partner and source-amplitude. I observed that w/o this change, the untransformed config was being used for check/discover/read operations. When updating the underlying SDM images to the dev image, the check/discover/read operations began using the transformed config as expected.

override configure method

a816a8c

github-actions bot added bug Something isn't working security labels Jun 25, 2025

pnilan requested a review from brianjlai June 25, 2025 17:56

pnilan marked this pull request as ready for review June 25, 2025 17:56

Copilot AI review requested due to automatic review settings June 25, 2025 17:56

Copilot AI reviewed Jun 25, 2025

View reviewed changes

pnilan temporarily deployed to DockerHub June 25, 2025 17:57 — with GitHub Actions Inactive

pnilan temporarily deployed to PyPi June 25, 2025 17:57 — with GitHub Actions Inactive

chore: format

b7689f0

pnilan changed the title ~~fix: override configure method in ManifestDeclarativeSource to use migrated/transformed config~~ fix: override configure method in ManifestDeclarativeSource to set migrated/transformed config in entrypoint Jun 25, 2025

coderabbitai bot requested changes Jun 25, 2025

View reviewed changes

airbyte_cdk/sources/declarative/manifest_declarative_source.py Outdated Show resolved Hide resolved

coderabbitai bot approved these changes Jun 25, 2025

View reviewed changes

brianjlai reviewed Jun 25, 2025

View reviewed changes

add test for configure

2729b18

pnilan changed the title ~~fix: override configure method in ManifestDeclarativeSource to set migrated/transformed config in entrypoint~~ fix: override configure method in ManifestDeclarativeSource to set transformed config in entrypoint Jun 25, 2025

pnilan requested a review from brianjlai June 25, 2025 19:57

update test

f0d328f

brianjlai approved these changes Jun 25, 2025

View reviewed changes

pnilan merged commit 0afea4a into main Jun 25, 2025
26 checks passed

pnilan deleted the pnilan/fix/update-entrypoint-to-pass-migrated-config-to-methods branch June 25, 2025 20:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: override configure method in `ManifestDeclarativeSource` to set transformed config in entrypoint #611

fix: override configure method in `ManifestDeclarativeSource` to set transformed config in entrypoint #611

Uh oh!

pnilan commented Jun 25, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

coderabbitai bot commented Jun 25, 2025 •

edited

Loading

Walkthrough

Changes

Suggested labels

Suggested reviewers

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

brianjlai left a comment

Uh oh!

github-actions bot commented Jun 25, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jun 25, 2025 •

edited

Loading

Uh oh!

pnilan commented Jun 25, 2025

Uh oh!

Uh oh!

Uh oh!

fix: override configure method in ManifestDeclarativeSource to set transformed config in entrypoint #611

fix: override configure method in ManifestDeclarativeSource to set transformed config in entrypoint #611

Uh oh!

Conversation

pnilan commented Jun 25, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Summary by CodeRabbit

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

coderabbitai bot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Suggested labels

Suggested reviewers

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brianjlai left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PyTest Results (Fast)

Uh oh!

github-actions bot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PyTest Results (Full)

Uh oh!

pnilan commented Jun 25, 2025

Uh oh!

Uh oh!

Uh oh!

fix: override configure method in `ManifestDeclarativeSource` to set transformed config in entrypoint #611

fix: override configure method in `ManifestDeclarativeSource` to set transformed config in entrypoint #611

pnilan commented Jun 25, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jun 25, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

github-actions bot commented Jun 25, 2025 •

edited

Loading

github-actions bot commented Jun 25, 2025 •

edited

Loading