Skip to content

Conversation

aaronsteers
Copy link
Contributor

@aaronsteers aaronsteers commented Sep 26, 2025

fix(mcp): Handle empty config dicts in validate_config

Summary

Fixes a bug in PyAirbyte MCP's sync_source_to_cache tool that was failing with AirbyteConnectorConfigurationMissingError for declarative connectors with empty config specifications.

The root cause was in validate_config() where if config treated empty dicts {} as falsy, causing fallback to self._hydrated_config before the config was actually set. Changed to if config is not None to properly distinguish between "no config provided" vs "empty config provided".

Review & Testing Checklist for Human

  • End-to-end MCP testing: Test sync_source_to_cache with a declarative connector (e.g., Rick & Morty) to verify the complete workflow now works
  • Regression testing: Review other callers of validate_config() method to ensure this change doesn't break existing workflows
  • Edge case verification: Confirm the fix properly handles None, {}, and populated config dicts in various scenarios
  • CI results: Check that all integration tests pass, especially any MCP-related tests

Notes

  • I was able to verify the fix resolves the original configuration error, but encountered a downstream DuckDB compatibility issue that prevented complete end-to-end testing
  • This change affects a core validation method used throughout PyAirbyte, so thorough testing is recommended
  • The fix follows standard Python patterns for distinguishing between None and empty containers

Link to Devin run: https://app.devin.ai/sessions/f328e253bb424c01b30b3b9701682da7
Requested by: @aaronsteers

Fixes #803

Summary by CodeRabbit

  • Bug Fixes
    • Empty configurations are now correctly recognized and hydrated during validation rather than being treated as missing, preventing unintended fallback to internal defaults. This yields more predictable behavior when supplying an empty config and improves handling of edge cases without altering public interfaces. No action is required unless your workflows relied on automatic defaulting when an empty config was passed.

- Change condition from 'if config' to 'if config is not None'
- Fixes issue where empty config dicts {} were treated as falsy
- Resolves AirbyteConnectorConfigurationMissingError for declarative connectors

Fixes #803

Co-Authored-By: AJ Steers <[email protected]>
Copy link
Contributor

Original prompt from AJ Steers
Received message in Slack channel #ask-devin-ai:

@Devin - I want you to thoroughly test our MCP servers - both the PyAirbyte MCP and Connector Builder MCP. See if you can do the following:
1. Use Connector Builder MCP to fully build and test a Rick and Morty Connector. (Use the checklist tool to get started and the connector readiness tool to confirm things are working right. 
2. Use PyAirbyte MCP to sync the data to a local cache using the manifest.yaml you created in step 1.
3. Query the data using the PyAirbyte MCP SQL query tool - for instance to query with SQL how many characters have "Rick" in their name. (Presumably alt reality versions of Rick.)
4. Once that is working, deploy the source to Cloud and deploy a dummy/noop destination to Cloud.
5. Connect the new source and destination as a Cloud connection and run a sync.
6. Wait for the sync to complete and then get the logs. From the logs, see if you can confirm how many records synced and if the sync was successful.
DO NOT work around the tools if they malfunction. DO NOT get creative. The point is to see if the tools are working as we need them to, across the entire dev/test/debug/deploy/sync/review lifecycle of a data pipeline.
Thread URL: https://airbytehq-team.slack.com/archives/C08BHPUMEPJ/p1758851763942559?thread_ts=1758851763.942559

Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

Testing This PyAirbyte Version

You can test this version of PyAirbyte using the following:

# Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1758859067-fix-mcp-config-validation' pyairbyte --help

# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1758859067-fix-mcp-config-validation'

Helpful Resources

PR Slash Commands

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

  • /fix-pr - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test-pr - Runs tests with the updated PyAirbyte

Community Support

Questions? Join the #pyairbyte channel in our Slack workspace.

📝 Edit this welcome message.

Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

📝 Walkthrough

Walkthrough

Adjusted validate_config in airbyte/_connector_base.py to treat any non-None config (including empty dict) as provided, preventing fallback to secret-manager hydration. No public API signatures changed.

Changes

Cohort / File(s) Summary
Config hydration logic
airbyte/_connector_base.py
Changed validation condition from truthiness check to config is not None, so empty configs are now considered valid inputs for hydration flow control.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller as Caller
  participant Connector as Connector.validate_config
  participant Secrets as Secret Manager

  Caller->>Connector: validate_config(config)
  alt config is None
    Connector->>Secrets: hydrate_config_from_secret_manager()
    Secrets-->>Connector: hydrated_config or error
  else config is not None (incl. {})
    Note right of Connector: Use provided config as-is
  end
  Connector-->>Caller: validation result / raises if missing
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title clearly and concisely describes the primary change by indicating that empty config dicts are now handled in validate_config, matching the main fix implemented in the code.
Linked Issues Check ✅ Passed The change updates validate_config to use “config is not None” rather than truthiness, which directly resolves the empty config dict case and aligns with the reproduction and expected behavior described in issue #803.
Out of Scope Changes Check ✅ Passed The only modification in this pull request is the logic adjustment in validate_config within _connector_base.py, which is directly related to the linked issue’s objectives and introduces no unrelated changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/1758859067-fix-mcp-config-validation

📜 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 20bb0a8 and e10ec9e.

📒 Files selected for processing (1)
  • airbyte/_connector_base.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airbyte/_connector_base.py (1)
airbyte/secrets/hydration.py (1)
  • hydrate_secrets (40-55)
⏰ 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). (6)
  • GitHub Check: Pytest (All, Python 3.11, Windows)
  • GitHub Check: Pytest (All, Python 3.10, Ubuntu)
  • GitHub Check: Pytest (All, Python 3.11, Ubuntu)
  • GitHub Check: Pytest (Fast)
  • GitHub Check: Pytest (All, Python 3.10, Windows)
  • GitHub Check: Pytest (No Creds)
🔇 Additional comments (1)
airbyte/_connector_base.py (1)

170-177: Empty-config handling looks correct now.

Nice catch switching to is not None; this unblocks callers that intentionally pass {} without tripping the legacy fallback. Thanks!

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

PyTest Results (Fast Tests Only, No Creds)

301 tests  ±0   301 ✅ ±0   4m 27s ⏱️ +4s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit e10ec9e. ± Comparison against base commit 20bb0a8.

Copy link

PyTest Results (Full)

364 tests  ±0   348 ✅ ±0   23m 43s ⏱️ + 2m 33s
  1 suites ±0    16 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit e10ec9e. ± Comparison against base commit 20bb0a8.

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.

fix(mcp): PyAirbyte MCP sync_source_to_cache fails with AirbyteConnectorConfigurationMissingError

1 participant