feat: add more robust config parsing (str as json) #60
Conversation
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing This Branch via MCPTo test the changes in this specific branch with an MCP client like Claude Desktop, use the following configuration: {
"mcpServers": {
"connector-builder-mcp-dev": {
"command": "uvx",
"args": ["--from", "git+https://github.com/airbytehq/connector-builder-mcp.git@feat/robust-config-dict-parsing", "connector-builder-mcp"]
}
}
}Testing This Branch via CLIYou can test this version of the MCP Server using the following CLI snippet: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/connector-builder-mcp.git@feat/robust-config-dict-parsing#egg=airbyte-connector-builder-mcp' --helpPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances config parsing by adding support for JSON string input and improves the robustness of configuration handling. The changes move utility functions to a shared module and add new parsing capabilities.
- Refactored boolean and dictionary parsing functions to a centralized utility module
- Added support for parsing JSON strings as configuration dictionaries
- Enhanced type annotations and function signatures for better type safety
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| connector_builder_mcp/validation_testing.py | Removed local _as_bool function and updated to use centralized utilities with improved config parsing |
| connector_builder_mcp/_util.py | Added as_bool and as_dict utility functions with overloads for robust type conversion |
| connector_builder_mcp/_secrets.py | Minor parameter formatting improvement with keyword-only argument |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return default | ||
|
|
||
| if isinstance(val, str): | ||
| return cast("dict[str, Any]", json.loads(val)) |
There was a problem hiding this comment.
The json.loads() call can raise JSONDecodeError but this is not handled or documented. Consider wrapping in a try-catch block and raising a more descriptive error message that indicates the JSON parsing failed.
Copilot uses AI. Check for mistakes.
|
|
||
| raw_responses_data = None | ||
| if include_raw_responses_data is True and slices and isinstance(slices, list): | ||
| if include_raw_responses_data is True and slices: |
There was a problem hiding this comment.
The explicit is True comparison is redundant since include_raw_responses_data is already a boolean. The condition can be simplified to if include_raw_responses_data and slices:.
| if include_raw_responses_data is True and slices: | |
| if include_raw_responses_data and slices: |
Copilot uses AI. Check for mistakes.
1 participant
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.