feat(workspace): BigQuery support for workspace query (0.58.0)#398
Conversation
3a75c1a to
67abdf6
Compare
workspace query (0.56.0)workspace query (0.58.0)
padak
left a comment
There was a problem hiding this comment.
Review of #398 — feat(workspace): BigQuery support for workspace query (0.58.0)
Generated by
kbagent-pr-reviewersubagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed viamake check, not duplicated here.
Summary
This PR extends workspace query / workspace list / workspace detail to
correctly classify BigQuery workspaces as Query-Service-compatible, adds
backend-aware error unwrapping for BigQuery query failures, and aligns
workspace create on BigQuery projects with keboola-mcp-server. The
implementation is clean (3-layer compliant, well-tested, good constant
hygiene), the regression guard for the Snowflake-default-stays-false case
is in place, and all prerequisite sync surfaces have been updated (gotchas.md,
workspace-workflow.md, changelog, version files). Two NON-BLOCKING gaps were
found: (1) commands-reference.md and keboola-expert.md §2 / §3 do not
note the 0.58.0 semantics change to qs_compatible classification for BigQuery
callers; (2) client.py crossed the 2000-LOC hard ceiling already on main
and this PR adds 25 more lines without splitting — the split obligation existed
before this PR but this PR is additive in a file above hard budget.
Verdict: APPROVE — no blocking findings, two non-blocking findings, one nit.
Verdict
- Verdict: APPROVE
- Blocking findings: 0
- Non-blocking findings: 2
- Nits: 1
Blocking findings
(none)
Non-blocking findings
[NB-1] plugins/kbagent/skills/kbagent/references/commands-reference.md:167-168 — workspace list / workspace detail entries carry no (updated v0.58.0) tag for the BigQuery qs_compatible change
The entries at lines 167-168 document the qs_compatible field with a bare
"Since v0.42.0 (#304)" tag. The semantics changed in 0.58.0: qs_compatible
is now keyed by (backend, loginType), not loginType alone, so BigQuery
workspaces (previously always false) are now true. An agent running on an
older kbagent install would follow the same --qs-compatible filter and get
zero results for a BigQuery project — identical symptom to pre-0.42.0 behavior.
Per CONTRIBUTING.md > references/gotchas.md, behavior changes that agents
would get wrong on older installs need a version tag.
Fix: append (updated v0.58.0 -- BigQuery workspaces carry loginType "default" and are now qs_compatible: true; before 0.58.0 they were always false) to the
workspace list and workspace detail bullet descriptions.
[NB-2] src/keboola_agent_cli/client.py:3194 — file is above the 2000-LOC hard ceiling
client.py is 3194 LOC after this PR (+25 from main's 3169). The hard
ceiling per CONTRIBUTING.md > "File-size budgets" is 2000 LOC for clients.
The split obligation predates this PR (the file was 3169 LOC on main), but
this PR adds material to a file that is already 60% over hard budget. Per the
guideline: "When a file crosses the hard ceiling, splitting is required before
merging more functionality into it." The split itself is likely a multi-PR
undertaking; flagging here so the next PR author does not inherit the rule
silently.
Fix: open a follow-up issue (or do it before merge if straightforward) to
extract the Query Service helpers (wait_for_query_job, _extract_query_job_error,
_unwrap_bigquery_error) plus the workspace query helpers into a sibling
client_query.py or client/workspace_query.py.
Nits
[NIT-1]plugins/kbagent/agents/keboola-expert.md:103— the "Ad-hoc SQL / row-count / type audit" row in §2 referencesworkspace list --qs-compatible (0.42.0+, #304)without noting that BigQuery workspaces are now included in the compatible set since 0.58.0. An agent working against a BigQuery project on 0.58.0+ could still be surprised by thedefaultlogin_type appearing inqs_compatible: trueoutput when the §2 row implies Snowflake-only semantics. One-liner addition to the "First choice" cell: append(since 0.58.0 also covers BigQuery workspaces; loginType "default" is QS-compatible on BigQuery but rejected on Snowflake).
Verification log
gh pr view 398 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state→ 13 files changed, +311/-30, state OPEN, headRefNameclaude/exciting-ride-fbf6e2matching working tree ✓git rev-parse --abbrev-ref HEAD→claude/exciting-ride-fbf6e2(matches PR branch) ✓- Read
CONTRIBUTING.md,CLAUDE.md,plugins/kbagent/agents/keboola-expert.md,plugins/kbagent/skills/kbagent/references/gotchas.md✓ gh pr diff 398→ 602 diff lines, no new@app.command(...)decorators → no new CLI command surface; noOPERATION_REGISTRY/context.py/CLAUDE.mdcommand-list update required ✓- Layer check:
grep -E 'from typer|import typer' /tmp/kbagent-pr-398.diff | grep services/→ empty ✓;grep -E 'from httpx|import httpx' diff | grep commands/→ empty ✓;grep -E 'formatter\.|console\.print' diff | grep client\.py→ empty ✓ - Convention checks: magic numbers → none; raw
error_code="..."strings → none; bareexcept:→ none;print()in src/ → none; token in logged output → none ✓ _classify_qs_compatibilitycallers: all three call sites inworkspace_service.py(lines 518, 684) pass the newbackendargument; no missed call sites ✓QUERY_SERVICE_COMPATIBLE_LOGIN_TYPES_BIGQUERYandBIGQUERY_WORKSPACE_LOGIN_TYPEadded inconstants.pywith detailed rationale comments ✓gotchas.md"workspace list / workspace detail" section updated with(since v0.58.0)inline tag for BigQuery qs_compatible semantics ✓workspace-workflow.mdupdated: Query Service line now says "Backend-agnostic: runs SELECTs against both Snowflake and BigQuery workspaces (BigQuery since v0.58.0)" ✓plugins/kbagent/agents/keboola-expert.md§3: no inline gotcha for BigQueryworkspace createlogin_type change; the behavior IS documented ingotchas.md, so this is advisory only (not blocking per CONTRIBUTING.md notes on §3)commands-reference.mdworkspace entries: no(updated v0.58.0)tag for the qs_compatible semantics change → NB-1 abovekeboola-expert.mdbyte size: 48840 bytes (well under 62000 cap) ✓make check(full suite) →3879 passed, 8 skipped, 124 deselected, 16 warnings in 80.90sexit 0;All checks passed!✓- Test coverage:
TestBigQueryQueryServiceSupport(6 unit tests intest_workspace_service.py) covers backend-aware classification, Snowflake-default regression guard, login-type helper, and end-to-end qs_compatible filter;TestUnwrapBigQueryError(4 unit tests intest_client.py) covers the regex extractor; 3 existingTestAutoDetectBackendasserts updated for thelogin_type="default"BigQuery change;test_e2e.pybackend-aware query +login_type/qs_compatibleasserts ✓ client.pyLOC: 3194 (main was 3169; hard ceiling is 2000) → NB-2 aboveworkspace_service.pyLOC: 1021 (main was 999; crossed soft ceiling of 1000 in this PR; hard ceiling 1500 still has headroom) → tracked as context for NB-2- Behavior verification via live run not performed (no credentials in CI environment). PR description provides detailed live test matrix against project 9621 (
e2e-bigquery,connection.keboola.com); results are internally consistent with the code changes.
Open questions for the author
(none)
Review feedback addressedBoth reviews (Devin +
|
The Query Service backend now runs BigQuery (as keboola-mcp-server already
uses). The query execution path was always backend-agnostic -- submit + CSV
export are identical for Snowflake and BigQuery -- so a SELECT against a
BigQuery workspace already returned rows. The gaps were classification and
error legibility:
- qs_compatible is now keyed by (backend, loginType). BigQuery workspaces
carry loginType `default`, which IS Query-Service-compatible, but `default`
was off the Snowflake-only whitelist, so every BigQuery workspace was
reported qs_compatible=false and hidden by `workspace list --qs-compatible`.
New QUERY_SERVICE_COMPATIBLE_LOGIN_TYPES_BIGQUERY whitelist is kept separate
because Snowflake's legacy `default` is rejected ('JWT token is invalid') --
the same string means compatible for BigQuery, incompatible for Snowflake.
- workspace create on BigQuery now requests loginType `default` explicitly
(matches keboola-mcp-server) instead of relying on the backend default.
- BigQuery query errors are unwrapped: the Query Service serializes them as
{Location: ...; Message: "..."; Reason: ...}; _unwrap_bigquery_error extracts
the inner Message so the error reads like Snowflake plain text.
Verified live against project 9621 (e2e-bigquery, connection.keboola.com):
create/list/detail/load/query/delete, real-data query, qs_compatible surfacing,
and clean error messages.
Tests: TestBigQueryQueryServiceSupport, TestUnwrapBigQueryError + a BigQuery
case in TestExtractQueryJobError; test_e2e workspace query is now backend-aware
(BigQuery back-tick vs Snowflake double-quote quoting).
… (review NB-1) Addresses the kbagent-pr-reviewer NB-1 finding: the commands-reference.md workspace list/detail/query entries documented the #304 qs_compatible fields but not the v0.58.0 backend-aware change. Without it, an agent on an older install hitting a BigQuery project would get zero `--qs-compatible` results with no explanation. - workspace list / detail: note qs_compatible is keyed by (backend, loginType) and BigQuery `default` workspaces are now qs_compatible=true (Snowflake's own legacy `default` stays false). - workspace query: note it is backend-agnostic since v0.58.0 and the Snowflake double-quote vs BigQuery back-tick dialect difference.
#401 The 0.58.0 BigQuery note was a single wall-of-text bullet. #401 (merged to main) introduced the `kbagent changelog` one-line-summary view + an authoring contract (one logical change per prefixed bullet, self-contained first sentence). Reformat the entry into New:/Fix:/Change: bullets so the default `kbagent changelog` view stays scannable, and add a bullet documenting the #401 feature itself (it shipped to main without a version bump, so 0.58.0 is its release home).
82faf3d to
62c32c8
Compare
What
Adds BigQuery support to
kbagent workspace queryand the surrounding workspace classification, matching whatkeboola-mcp-serveralready does. Bumps to 0.58.0.Why
The team enabled BigQuery in the Query Service server-side. Investigating against the real
e2e-bigqueryproject (id 9621,connection.keboola.com) showed the query execution path was already backend-agnostic —POST .../queries+ CSV export are identical for Snowflake and BigQuery, so aSELECTagainst a BigQuery workspace already returned rows. The actual defects were elsewhere:qs_compatiblewas Snowflake-only. BigQuery workspaces carrylogin_type: "default", which IS Query-Service-compatible, butdefaultwas off the whitelist, so every BigQuery workspace was reportedqs_compatible: falseand hidden byworkspace list --qs-compatible— misleading data-app developers into thinking BigQuery can't run Query-Service SELECTs.{Location: "query"; Message: "Syntax error: ..."; Reason: "invalidQuery"}instead of plain text.Changes
Code
constants.py— newQUERY_SERVICE_COMPATIBLE_LOGIN_TYPES_BIGQUERY({default}), kept separate from the Snowflake set because Snowflake's own legacydefaultis rejected by the Query Service (JWT token is invalid). Same string, opposite meaning per backend.services/workspace_service.py—_classify_qs_compatibility(login_type, backend)is now keyed by(backend, loginType);workspace createon BigQuery requestslogin_type="default"explicitly (parity with mcp-server) instead of relying on the backend default.client.py—_unwrap_bigquery_error(regex onMessage: "...") wired into_extract_query_job_error; Snowflake plain text passes through untouched.Docs — corrected
gotchas.md(the whitelist now documents the backend-keyed semantics) andworkspace-workflow.md. Changelog + plugin.json + marketplace.json + uv.lock synced to 0.58.0.Testing
Unit —
TestBigQueryQueryServiceSupport(backend-aware classification, Snowflake-defaultregression guard, BigQuery login-type, qs-compatible filter),TestUnwrapBigQueryError+ a BigQuery case inTestExtractQueryJobError, updatedTestAutoDetectBackendBigQuery create asserts. Full suite: 3854 passed, 0 failures. Lint/format/ty/skill/version gates green.Live — verified end-to-end against project 9621 (
e2e-bigquery,connection.keboola.com):workspace query(constant + real data viaworkspace load)workspace detail/list --qs-compatibleqs_compatible: true(wasfalse); 6 BigQuery workspaces surfaced (was 0)Unrecognized name: nonexistent_col at [1:8](no{Location...}wrapper)workspace create(explicitdefault)qs_compatible: true, query runsTest workspaces created during verification were cleaned up.
Reviewer notes
test_e2e.pyworkspace query is now backend-aware (BigQuery back-tick vs Snowflake double-quote identifier quoting) — running the E2E suite against a BigQuery project no longer fails on quoting;detailalso assertslogin_type/qs_compatible.workspace createbehaviour change (explicitlogin_type="default") updated 3 existingTestAutoDetectBackendasserts that pinned the old omit-loginType behaviour.Rebased onto
main2026-06-05: version re-bumped 0.56.0 -> 0.58.0 (0.56.0 was taken by the #399 maintenance re-release, main is now at 0.57.0). Changelog moved under the 0.58.0 key; full suite green (3879 passed).