Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 35 additions & 4 deletions plugins/kbagent/skills/kbagent/references/library-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,46 @@ shell operations, use the `kbagent` CLI.
|--------|---------|
| `Client(url, token, *, branch_id=None)` | Stateless entry point to one project; context manager |
| `Client.query(workspace_id, sql, *, transactional=False, limit=500)` | Run SQL in a workspace -> `list[dict]` |
| `Client.query_result(workspace_id, sql, ...)` | Same, but typed -> `QueryResult` (columns + truncation) |
| `Client.run_job(component_id, config_id, *, wait=False, ...)` | Run a Queue job -> `JobResult` |
| `Client.config_detail(component_id, config_id, *, branch_id=None)` | One config's detail -> `ConfigDetailResult` |
| `Client.upload_table(table_id, file_path, *, incremental=False, ...)` | Import a CSV into an existing table -> `UploadTableResult` |
| `Client.files.upload(source, *, name=None, tags=None, permanent=False)` | Upload a path **or** bytes -> `FileEntry` |
| `Client.files.read_bytes(file_id)` | Download a file fully into memory -> `bytes` |
| `Client.files.list(*, tags=None, query=None, limit=100, ...)` | List files -> `list[FileEntry]` |
| `Client.files.delete(file_id)` | Delete a file |
| `Client.raw` | The underlying `KeboolaClient` for endpoints the facade omits |
| `FileEntry` | Uniform file shape: `id, name, tags, created, size_bytes, is_permanent, raw` |

Everything exported from `keboola_agent_cli` (`Client`, `Files`, `FileEntry`) is
committed public API and follows semver.
Everything exported from `keboola_agent_cli` (`Client`, `Files`, `FileEntry`, and
the typed result models `JobResult`, `QueryResult`, `UploadTableResult`,
`SyncPushResult`, `ConfigDetailResult`) is committed public API and follows
semver. Since 0.63.0 the package ships a **`py.typed`** marker (PEP 561), so
`mypy` / `ty` / IDEs treat the SDK as typed -- a contract change surfaces at
type-check time, not at runtime.

## Typed result models

The high-traffic operations return pydantic models (`result_models.py`) instead
of bare dicts, so you get autocomplete and a versioned contract:

```python
job = kbc.run_job("keboola.ex-db-snowflake", "12345", wait=True)
if job.succeeded: # -> JobResult
print(job.id, job.result)

res = kbc.query_result(ws, 'SELECT id, name FROM t') # -> QueryResult
print(res.columns, res.row_count, res.truncated)

cfg = kbc.config_detail("keboola.ex-http", "98765") # -> ConfigDetailResult
print(cfg.name, cfg.version, cfg.configuration)
```

Every model is **tolerant of extra fields** (`extra="allow"`): the named fields
are the stable surface, but anything else the API returns is preserved
(reachable via attribute access and `model_dump()`), so a new backend field
never raises. They also accept the raw API key *or* the snake_case field name, so
`JobResult.model_validate(service_dict)` works directly on a service-layer dict.

## Auth & construction

Expand Down Expand Up @@ -100,8 +131,8 @@ kbc.files.delete(meta.id)

## Lower-level access

For endpoints the facade does not wrap (buckets, tables, jobs, branches, ...),
reach for the underlying client:
For endpoints the facade does not wrap (buckets, tables, branches, job polling
internals, ...), reach for the underlying client:

```python
client = kbc.raw # a KeboolaClient
Expand Down
5 changes: 5 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ packages = ["src/keboola_agent_cli"]

[tool.hatch.build.targets.wheel.force-include]
"src/keboola_agent_cli/_ui_dist" = "keboola_agent_cli/_ui_dist"
# PEP 561: ship the py.typed marker so downstream type checkers treat the
# importable SDK (Client + result_models contracts, issue #428) as typed.
# Forced in explicitly rather than relying on hatchling's default file
# selection, so the marker is guaranteed to land in the wheel.
"src/keboola_agent_cli/py.typed" = "keboola_agent_cli/py.typed"

[tool.hatch.build.targets.wheel.hooks.custom]
path = "scripts/hatch_build.py"
Expand Down
19 changes: 18 additions & 1 deletion src/keboola_agent_cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,27 @@

from .constants import APP_NAME
from .lib import Client, FileEntry, Files
from .result_models import (
ConfigDetailResult,
JobResult,
QueryResult,
SyncPushResult,
UploadTableResult,
)

try:
__version__ = version(APP_NAME)
except PackageNotFoundError:
__version__ = "0.0.0-dev"

__all__ = ["Client", "FileEntry", "Files", "__version__"]
__all__ = [
"Client",
"ConfigDetailResult",
"FileEntry",
"Files",
"JobResult",
"QueryResult",
"SyncPushResult",
"UploadTableResult",
"__version__",
]
227 changes: 199 additions & 28 deletions src/keboola_agent_cli/lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@
meta = kbc.files.upload(b"hello", name="greeting.txt", tags=["x"])
data = kbc.files.read_bytes(meta.id) # bytes
metas = kbc.files.list(tags=["x"]) # list[FileEntry]
job = kbc.run_job("keboola.ex-db-snowflake", "12345", wait=True) # JobResult

The facade is a thin convenience wrapper over :class:`KeboolaClient`
(``client.py``); it adds the high-level shapes -- ``list[dict]`` rows, ``bytes``
file reads, a stable :class:`FileEntry` -- that the CLI used to assemble inside
its service layer. Auth is the storage token passed at construction (12-factor:
read it from ``KBC_TOKEN`` yourself); nothing is persisted to disk.
file reads, a stable :class:`FileEntry`, and the typed result models in
``result_models.py`` (:class:`JobResult`, :class:`QueryResult`,
:class:`UploadTableResult`, :class:`ConfigDetailResult`) -- that the CLI used to
assemble inside its service layer. Auth is the storage token passed at
construction (12-factor: read it from ``KBC_TOKEN`` yourself); nothing is
persisted to disk.

Everything exported here is committed public API and changes follow semver. For
lower-level access (raw Queue/Storage endpoints) reach for the underlying
Expand All @@ -33,8 +37,14 @@
from typing import Any

from .client import KeboolaClient, _collect_inline_results
from .constants import QUERY_RESULTS_DEFAULT_LIMIT
from .constants import (
DEFAULT_JOB_MODE,
DEFAULT_JOB_RUN_TIMEOUT,
DEFAULT_POLL_STRATEGY,
QUERY_RESULTS_DEFAULT_LIMIT,
)
from .errors import ErrorCode, KeboolaApiError
from .result_models import ConfigDetailResult, JobResult, QueryResult, UploadTableResult

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -234,6 +244,54 @@ def _effective_branch_id(self) -> int:
error_code=ErrorCode.NOT_FOUND,
)

def _run_query(
self,
workspace_id: int,
sql: str,
*,
transactional: bool,
limit: int,
) -> QueryResult:
"""Submit SQL, wait for completion, and collect the last result set.

Shared by :meth:`query` (which returns just the rows) and
:meth:`query_result` (which returns the full typed shape). Mirrors the
Query Service inline-results fast path: the rows of the *last*
result-producing statement win, statements without a result set yield
nothing, and an over-``limit`` result is capped with a logged warning.
"""
branch_id = self._effective_branch_id()
job = self._client.submit_query(
branch_id=branch_id,
workspace_id=workspace_id,
statements=[sql],
transactional=transactional,
)
job_id = str(job.get("queryJobId", job.get("id", "")))
completed = self._client.wait_for_query_job(job_id)

result = QueryResult()
for stmt in completed.get("statements", []):
num_rows = stmt.get("numberOfRows", stmt.get("resultRows", 0))
if stmt.get("status") != "completed" or not num_rows:
continue
inline = _collect_inline_results(self._client, job_id, str(stmt.get("id", "")), limit)
col_names = [col.get("name", "") for col in inline.columns]
result = QueryResult(
columns=col_names,
rows=[dict(zip(col_names, row, strict=False)) for row in inline.rows],
truncated=inline.truncated,
total_rows=inline.total_rows,
)
if inline.truncated:
logger.warning(
"query result truncated to %d rows (warehouse has %s); "
"raise limit= to fetch more",
result.row_count,
inline.total_rows,
)
return result

def query(
self,
workspace_id: int,
Expand All @@ -259,6 +317,8 @@ def query(
statement that produced a result set are returned (so ``USE ...; SELECT
...`` yields the SELECT). Statements without a result set yield ``[]``.

For column order and truncation metadata, use :meth:`query_result`.

Args:
workspace_id: Target workspace ID.
sql: One or more SQL statements.
Expand All @@ -267,32 +327,143 @@ def query(
If the warehouse has more, the result is capped and a warning is
logged -- raise ``limit`` to fetch more.
"""
branch_id = self._effective_branch_id()
job = self._client.submit_query(
branch_id=branch_id,
workspace_id=workspace_id,
statements=[sql],
transactional=transactional,
return self._run_query(workspace_id, sql, transactional=transactional, limit=limit).rows

def query_result(
self,
workspace_id: int,
sql: str,
*,
transactional: bool = False,
limit: int = QUERY_RESULTS_DEFAULT_LIMIT,
) -> QueryResult:
"""Run SQL in a workspace and return a typed :class:`QueryResult`.

Same execution as :meth:`query`, but returns the full tabular shape --
``columns`` (in warehouse order), ``rows`` (list of dicts), ``truncated``
and ``total_rows`` -- instead of just the row list. Use this when you
need the column ordering or want to detect a ``limit`` cap. The
string-typing gotcha from :meth:`query` applies to ``rows`` here too.
"""
return self._run_query(workspace_id, sql, transactional=transactional, limit=limit)

def run_job(
self,
component_id: str,
config_id: str,
*,
config_row_ids: list[str] | None = None,
variable_values_id: str | None = None,
branch_id: int | None = None,
mode: str = DEFAULT_JOB_MODE,
wait: bool = False,
timeout: float = DEFAULT_JOB_RUN_TIMEOUT,
poll_strategy: str = DEFAULT_POLL_STRATEGY,
) -> JobResult:
"""Run a Queue API job and return a typed :class:`JobResult`.

Creates the job, and -- when ``wait=True`` -- polls until it reaches a
terminal state (or ``timeout`` elapses). Unlike ``JobService.run_job``
this thin facade does **not** auto-resolve linked variable values; pass
``variable_values_id`` explicitly if the config needs a values row.

Args:
component_id: Component to run, e.g. ``keboola.ex-db-snowflake``.
config_id: Configuration ID to run.
config_row_ids: Optional row IDs (omit to run the whole config).
variable_values_id: Optional explicit values row for linked variables.
branch_id: Dev branch to run on. Defaults to the client's branch
(``None`` = production).
mode: Queue job mode (``run`` | ``debug`` | ``forceRun``).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 run_job docstring lists forceRun as a valid mode but VALID_JOB_MODES constant doesn't include it

The docstring at lib.py:377 says mode accepts run | debug | forceRun, but constants.py:116 defines VALID_JOB_MODES = frozenset({"run", "debug"}). No validation is performed against VALID_JOB_MODES in either run_job or create_job (client.py:2496), so forceRun would be accepted by the API if the Queue API supports it. This is a documentation inconsistency rather than a runtime bug — either the constant should include forceRun or the docstring should be narrowed. The existing JobService.run_job in the CLI validates against VALID_JOB_MODES (which excludes forceRun), so there's a discrepancy in what the facade vs. CLI allows.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

wait: If True, poll until the job finishes or ``timeout`` elapses.
timeout: Max seconds to wait (only used when ``wait=True``).
poll_strategy: Wait cadence, one of ``VALID_POLL_STRATEGIES``.
"""
effective_branch = branch_id if branch_id is not None else self._resolved_branch_id
job = self._client.create_job(
component_id=component_id,
config_id=config_id,
config_row_ids=config_row_ids,
mode=mode,
branch_id=effective_branch,
variable_values_id=variable_values_id,
)
job_id = str(job.get("queryJobId", job.get("id", "")))
completed = self._client.wait_for_query_job(job_id)
job_id = str(job.get("id", ""))
if wait and job_id:
job = self._client.wait_for_queue_job(
job_id, max_wait=timeout, poll_strategy=poll_strategy
)
return JobResult.model_validate(job)

rows: list[dict[str, Any]] = []
for stmt in completed.get("statements", []):
num_rows = stmt.get("numberOfRows", stmt.get("resultRows", 0))
if stmt.get("status") != "completed" or not num_rows:
continue
inline = _collect_inline_results(self._client, job_id, str(stmt.get("id", "")), limit)
col_names = [col.get("name", "") for col in inline.columns]
rows = [dict(zip(col_names, row, strict=False)) for row in inline.rows]
if inline.truncated:
logger.warning(
"query result truncated to %d rows (warehouse has %s); "
"raise limit= to fetch more",
len(rows),
inline.total_rows,
)
return rows
def config_detail(
self,
component_id: str,
config_id: str,
*,
branch_id: int | None = None,
) -> ConfigDetailResult:
"""Fetch one configuration's detail as a typed :class:`ConfigDetailResult`.

Args:
component_id: Owning component ID.
config_id: Configuration ID.
branch_id: Dev branch to read from. Defaults to the client's branch
(``None`` = production).
"""
effective_branch = branch_id if branch_id is not None else self._resolved_branch_id
detail = dict(
self._client.get_config_detail(component_id, config_id, branch_id=effective_branch)
)
detail.setdefault("component_id", component_id)
if effective_branch is not None:
detail.setdefault("branch_id", effective_branch)
Comment on lines +413 to +419

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Lazy default-branch cache in _resolved_branch_id leaks into config_detail, run_job, and upload_table

The field self._resolved_branch_id serves two incompatible purposes: (1) storing the user's explicit branch_id from the constructor, and (2) caching the lazily-resolved default branch inside _effective_branch_id() (called by query()/query_result()). The new methods at lines 382, 413, 448 all read self._resolved_branch_id directly via effective_branch = branch_id if branch_id is not None else self._resolved_branch_id. If a user creates Client(url=..., token=...) (no branch), calls query() first (which resolves and caches the default branch, e.g. 123, at lib.py:240), then calls config_detail("comp", "cfg"), effective_branch becomes 123 instead of None. This causes config_detail to inject branch_id=123 into the result (line 418-419), making ConfigDetailResult.branch_id non-None even though the user is targeting production. The docstring at result_models.py:227 states "None = production", so this is a contract violation: the same config_detail() call returns different branch_id metadata depending on whether an unrelated query() was called earlier.

Prompt for agents
The root cause is that _resolved_branch_id is a single field serving two roles: the user-supplied explicit branch (from the constructor) and a cached default-branch ID (set as a side effect of _effective_branch_id). The new methods (run_job, config_detail, upload_table) read _resolved_branch_id directly and thus pick up the cached default when it was never the user's intent.

Fix approach: split the field into two:
  - self._user_branch_id = branch_id  (the value passed at construction, never mutated)
  - self._cached_default_branch_id: int | None = None  (set only by _effective_branch_id)

Then _effective_branch_id returns self._user_branch_id if not None, else resolves and caches in _cached_default_branch_id. The new methods (run_job, config_detail, upload_table) should use self._user_branch_id (the explicit choice), not the cache. This way calling query() has no side effects on the other methods.

Affected locations:
  - lib.py __init__: split _resolved_branch_id into two fields
  - lib.py _effective_branch_id: read _user_branch_id, cache in _cached_default_branch_id
  - lib.py run_job (line 382), config_detail (line 413), upload_table (line 448): read _user_branch_id instead of _resolved_branch_id
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

return ConfigDetailResult.model_validate(detail)

def upload_table(
self,
table_id: str,
file_path: str | Path,
*,
incremental: bool = False,
delimiter: str = ",",
enclosure: str = '"',
branch_id: int | None = None,
) -> UploadTableResult:
"""Import a CSV into an **existing** Storage table -> :class:`UploadTableResult`.

Unlike ``StorageService.upload_table`` the facade does **not** auto-create
a missing bucket/table (it has no config-dir / service context); the
target table must already exist. Use the CLI (``kbagent storage
upload-table``) for the auto-create path.

Args:
table_id: Target table ID (must exist).
file_path: Local CSV path.
incremental: Append rows (True) or full load (False).
delimiter: CSV column delimiter.
enclosure: CSV value enclosure character.
branch_id: Dev branch to target. Defaults to the client's branch
(``None`` = production).
"""
effective_branch = branch_id if branch_id is not None else self._resolved_branch_id
file_size_bytes = Path(file_path).stat().st_size
results = self._client.upload_table(
table_id=table_id,
file_path=str(file_path),
incremental=incremental,
delimiter=delimiter,
enclosure=enclosure,
branch_id=effective_branch,
)
return UploadTableResult.model_validate(
{
"table_id": table_id,
"incremental": incremental,
"file_size_bytes": file_size_bytes,
"imported_rows": results.get("importedRowsCount"),
"warnings": results.get("warnings", []),
}
)

def close(self) -> None:
"""Close the underlying HTTP client."""
Expand Down
3 changes: 3 additions & 0 deletions src/keboola_agent_cli/py.typed
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# PEP 561 marker: keboola_agent_cli ships inline type annotations.
# Downstream mypy / ty / IDEs treat the importable SDK (Client, the
# result_models.* contracts) as typed. See issue #428.
Loading