feat(AI-3112): support keboola.duckdb-transformation via generic config tools - #652
Conversation
|
@claude review |
There was a problem hiding this comment.
Pull request overview
Adds DuckDB transformation support (keboola.duckdb-transformation) by routing it through the existing generic configuration tools, aligning it with the existing Python/R transformation flow (and keeping the SQL-transformation tools Snowflake/BigQuery-only).
Changes:
- Added
DUCKDB_TRANSFORMATION_IDand enabled folder metadata handling for DuckDB transformations viaupdate_config. - Updated tool docstrings and the
update_sql_transformation404 guidance to explicitly point DuckDB users tocreate_config/update_config. - Extended unit tests to cover DuckDB folder behavior and updated error-message expectations; bumped project version to
1.75.0(plus regenerated docs/lock updates).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/keboola_mcp_server/tools/components/utils.py |
Adds DuckDB transformation component ID constant and includes it in folder-support set. |
src/keboola_mcp_server/tools/components/tools.py |
Updates docstrings and improves 404 guidance to steer DuckDB to generic config tools. |
tests/tools/components/test_tools.py |
Extends existing parametrized tests to cover DuckDB folder handling and error message content. |
TOOLS.md |
Regenerated tool documentation reflecting updated docstrings/guidance. |
feature_spec/duckdb_transformation_support/RFC.md |
Adds an RFC documenting the motivation, scope, and verification plan for DuckDB support. |
pyproject.toml |
Version bump to 1.75.0. |
uv.lock |
Lockfile updated to reflect the version bump. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cac143a to
5b18e80
Compare
|
Rebased onto the AI-3664 fix branch ( |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
integtests/tools/flow/test_tools.py:833
- The cleanup block swallows deletion exceptions but logs without traceback, which makes integration-test failures much harder to debug. Use structured logging with
exc_info=True(orLOG.exception) when catching and continuing.
except Exception as e:
LOG.warning(f'Failed to delete {flow_type} flow {flow_id}: {e}')
integtests/tools/flow/test_tools.py:424
get_flows()lists all flows in the project (it does not filter to only those created by these fixtures), so asserting exact equality of IDs will fail in projects that already have flows. This test should assert that the two fixture-created flow IDs are present (subset), not that they are the only results.
assert {f.configuration_id for f in flows.flows} == {
initial_cf.configuration_id,
initial_cf2.configuration_id,
}
Add keboola.duckdb-transformation alongside Python/R as a generic-config-managed transformation type (create_sql_transformation/update_sql_transformation remain Snowflake/BigQuery-only, since their component ID is derived from the workspace SQL dialect and DuckDB has no workspace backend). - utils.py: add DUCKDB_TRANSFORMATION_ID, include it in FOLDER_SUPPORTING_COMPONENT_IDS so folder metadata works the same as Python/R. - tools.py: update create_config/create_sql_transformation/update_sql_transformation docstrings and the update_sql_transformation 404 error message to mention DuckDB. - Extend existing parametrized folder and 404-error tests with DuckDB cases. RFC: feature_spec/duckdb_transformation_support/RFC.md
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: auto_approve (risk 2/5) · profile keboola-mcp-server
Additive DuckDB component-ID support via existing generic config tools, with linked RFC, version bump, and extended tests — safe to auto-approve.
Impact flags: scope creep — see Check Run summary.
Concerns:
integtests/tools/flow/test_tools.py: Unrelated flow-test refactor bundled into a DuckDB PR; drops legacy Orchestrator coverage.
5b18e80 to
4fab217
Compare
Dismissing prior approval — a new commit was pushed and this review was for an earlier SHA. Run @keboola-pr-reviewer-bot review to get a fresh verdict.
|
New commit on |
|
👀 |
tomasfejfar
left a comment
There was a problem hiding this comment.
Doesn't do the same for update.
Also, while it works, it's not great. DuckDB transformation is essentially a SQL transformation and should be treated as such. Limitations of our tool shouldn't dictate that it goes to code-like transformations. But I wouldn't block on that alone. The missing update is the blocker.
|
Also:
|
Description
Linear: AI-3112
Change Type
Summary
Adds
keboola.duckdb-transformationas a supported transformation type, routed through theexisting generic
create_config/update_configtools — the same path Python/R transformationsalready use.
create_sql_transformation/update_sql_transformationstay Snowflake/BigQuery-only,since their component ID is derived from the project's workspace SQL dialect and DuckDB has no
cloud workspace backend.
Changes:
utils.py: newDUCKDB_TRANSFORMATION_IDconstant, added toFOLDER_SUPPORTING_COMPONENT_IDSso folder metadata management works the same as for Python/R.
tools.py: docstrings forcreate_config,create_sql_transformation, andupdate_sql_transformation, plus theupdate_sql_transformation404 error message, updated tomention DuckDB alongside Python/R so the model routes DuckDB requests correctly.
syntax_check,lineage_visualization,execution_plan_visualization,expected_input_tables) and configparameters (
backend_size,timeout,duckdb_version, etc.) require no new code — they'realready handled generically by
run_sync_actionand the opaque-JSONparametersfield.RFC:
feature_spec/duckdb_transformation_support/RFC.mdFollow-up (separate Linear issue, own RFC, not in this PR): AI-3662 —
consolidating the per-backend transformation tools and moving shared guidance into
project_system_prompt.md.Testing
Streamable-HTTPtransports)Optional testing
canary-orionMCP (Streamable-HTTP)canary-orioncanary-orionChecklist