diff --git a/plugins/kbagent/agents/keboola-expert.md b/plugins/kbagent/agents/keboola-expert.md index 0a6a6e2a..ef8d9d42 100644 --- a/plugins/kbagent/agents/keboola-expert.md +++ b/plugins/kbagent/agents/keboola-expert.md @@ -355,6 +355,7 @@ its absence is NOT a promise the entry is version-independent (see ยง1 Rule 6). `config detail` -> `configuration.runtime` FIRST (an empty `data-app logs` grep rules nothing out). `create` defaults it ON at **0.87.0+**; <= 0.86.0 patch + redeploy. +- **Data-app type in `sync`**: a `keboola.data-apps` config's runtime type (`python-js` / `streamlit`) lives only on the Data Science `/apps` record. `sync pull` records it as `_keboola.data_app_type`, and `sync push` / `sync clone` send it through `create_app`. A tree pulled before this carries no type, so re-pull the source before you clone, or the app deploys under the platform default, `streamlit` (since vNEXT). - **`ENCRYPTION_FAILED` on an Azure stack is a VERSION GATE, not a bad token**: <= 0.85.0 rejected the Azure `KBC::ProjectSecureKV::` cipher, so private-repo `create` and `secrets-set` could not work there at all. Upgrade to 0.86.0+; do diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index e1dfa2a5..f4111a9a 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -335,7 +335,7 @@ Requires the project to be added with its **master ('owner') Storage API token** - `sync init --project ALIAS [--directory DIR] [--git-branching] [--adopt-existing]` -- initialize sync working directory; `--adopt-existing` adopts a `.keboola/manifest.json` already written by the kbc Go CLI without overwriting (idempotent; validates `project_id` against the alias token) - `sync pull --project ALIAS [--all-projects] [--force] [--theirs] [--dry-run] [--with-samples] [--no-storage] [--no-jobs] [--job-limit N] [--branch ID]` -- download configs to local files. **Auto-inits:** if the target directory has no `.keboola/manifest.json`, pull runs `init` first, so a separate `sync init` is not needed for a first checkout of a project. For large projects (>100 configs), automatically fetches jobs per-config when the grouped API limit is insufficient. `--force` is conflict-aware (since 0.53.0): a locally-modified config whose remote is unchanged is **preserved** (pending delta stays pushable, never silently re-stamped); a true merge conflict (local AND remote both changed since last pull) **aborts** the pull (exit 1, `SYNC_CONFLICT`; `--json` lists `details.conflicts`); local-untouched + remote-changed takes remote. `--theirs` (since v0.72.0) is the supported "discard local, take production" reconcile path: overwrites locally-modified configs/rows, restores deleted/missing files, resolves conflicts by taking remote (no abort, no manifest surgery). Since v0.72.0 plain pull also re-materializes a tracked config whose local dir was deleted (manifest<->disk invariant), so delete-dir-then-pull refetches. Config-level `isDisabled` round-trips (since v0.72.0) as sparse `is_disabled: true` in `_config.yml` -- absent key = enabled. `--branch` (0.47.0+) per-invocation dev-branch override, beats every other branch source. Ignored components (since 0.91.0): `keboola.sandboxes` + `keboola.mcp-server-tool` are always excluded, unioned with the manifest's `ignoredComponents` list; a component newly ignored has its manifest entry dropped and local directory removed, reported with pull action `"ignored"` (distinct from `"removed"` = genuinely deleted on remote). - `sync push --project ALIAS [--all-projects] [--dry-run] [--force] [--allow-plaintext-on-encrypt-failure] [--branch ID] [--no-name-drift-warnings]` -- push local changes (auto-encrypts secrets, fails if encryption fails). Fresh-CREATE writeback updates placeholder manifest entries in place (since 0.47.0) and propagates any `KBC.configuration.*` metadata via `set_config_metadata`. Fresh-CREATE variable binding (since 0.47.2): when a `keboola.variables` config + its values row are created alongside a transformation in the same push, the transformation's `variables_id` / `variables_values_id` placeholders are rebound to the assigned ULIDs and the row's `values` are hoisted even without a `_keboola` block, so `job run` succeeds with no post-push `config variables-set` step (unresolvable/ambiguous links surface a `variable_link` entry in `errors[]`, never a broken link). Never-fetched guard (since v0.72.0): a manifest entry with an empty `pull_hash` and no local files (pre-0.72 name-collision phantom) is **never** planned as a remote DELETE -- diff/push exclude it and report it under `never_fetched` with a warning (run `sync pull` to materialize); local deletion of a properly-pulled config still deletes on push. Adopted-by-id writeback (since v0.72.0): pushing an untracked file whose `_keboola.config_id` resolves on the branch also writes the manifest entry, so follow-up diffs are stable. `--branch` (0.47.0+) per-invocation override; when no `/` subtree exists on disk (since 0.47.2) the local default tree (`main/`) is promoted to the target branch (API writes still target the branch id); `--no-name-drift-warnings` (0.47.0+) drops the cosmetic warnings array. Branch-scoped since v0.89.0 (issue #649): push consumes the diff's changeset, so configs tracked on another branch's tree are never planned as creates -- they ride along on the result envelope under `orphaned` instead (see `sync diff`). **Since 0.91.0 (#686)** the manifest baseline `pull_config_hash` is stamped from the API response (or a read-back), not from the files on disk, so a pushed multi-statement SQL transformation -- or anything disabled in the UI whose local YAML lacks `is_disabled` -- no longer shows permanent phantom `REMOTE MODIFIED` drift; if the config cannot be read back after the write the baseline is left UNTOUCHED and a `warnings[]` entry says to run `sync pull` (never a disk-derived fallback). One legacy change is refused per-change with `SYNC_LEGACY_BOUNDARY`: a tree pulled before statement-boundary markers existed whose only difference from the remote is the lost boundaries (pushing it would collapse separate SQL statements into one) -- run `sync pull` for that project first. Ignored components (since 0.91.0) are filtered out on both sides of the diff push builds on, so a stale local directory for an ignored component (e.g. `keboola.mcp-server-tool`) is never classified as `DELETED` and can never be pushed as a remote deletion. -- `sync clone --source DIR --target ALIAS --target-dir DIR [--bucket-map FILE] [--variable-values FILE] [--instance-rename FILE] [--dry-run] [--branch ID]` -- clone a reference synced project into a **fresh** target project and parameterize it. Copies the reference tree at `--source` into `--target-dir`, applies declarative overrides from JSON/YAML files (`--bucket-map` `{old_bucket_id: new_bucket_id}` rewrites storage input/output table refs; `--variable-values` `{var_name: value}` overrides `keboola.variables` rows; `--instance-rename` `{old_path_prefix: new_path_prefix}` renames config dirs + manifest paths), re-points the manifest at the target project, and pushes. Because the reference's config ids do not exist in the fresh target, every config is CREATEd fresh and **keboola.flow task `configId`s + transformation variable links are remapped reference->ULID** by push Phase C/D (the push result carries `flow_task_remaps`). **Idempotent**: re-running with an existing `--target-dir` skips copy/overrides and just pushes, reporting `no_changes` / `created: 0`. Fails fast (`CONFIG_ERROR`) if the target already contains the reference's configs -- clone requires a fresh/empty target. `SyncService.clone_project(...)` returns a typed `CloneResult` for in-process SDK callers. Override files must be flat `{id: scalar}` mappings *(since v0.89.0)* -- a nested mapping, list, or null value is rejected with `CONFIG_ERROR` (exit 5) naming the key and its actual type. `--branch` is optional on a fresh clone *(since v0.93.1)*. It defaults to the target's production branch, resolved from the API the same way `sync init` does. Pass `--branch ` only to target a dev branch. +- `sync clone --source DIR --target ALIAS --target-dir DIR [--bucket-map FILE] [--variable-values FILE] [--instance-rename FILE] [--dry-run] [--branch ID]` -- clone a reference synced project into a **fresh** target project and parameterize it. Copies the reference tree at `--source` into `--target-dir`, applies declarative overrides from JSON/YAML files (`--bucket-map` `{old_bucket_id: new_bucket_id}` rewrites storage input/output table refs; `--variable-values` `{var_name: value}` overrides `keboola.variables` rows; `--instance-rename` `{old_path_prefix: new_path_prefix}` renames config dirs + manifest paths), re-points the manifest at the target project, and pushes. Because the reference's config ids do not exist in the fresh target, every config is CREATEd fresh and **keboola.flow task `configId`s + transformation variable links are remapped reference->ULID** by push Phase C/D (the push result carries `flow_task_remaps`). **Idempotent**: re-running with an existing `--target-dir` skips copy/overrides and just pushes, reporting `no_changes` / `created: 0`. Fails fast (`CONFIG_ERROR`) if the target already contains the reference's configs -- clone requires a fresh/empty target. `SyncService.clone_project(...)` returns a typed `CloneResult` for in-process SDK callers. Override files must be flat `{id: scalar}` mappings *(since v0.89.0)* -- a nested mapping, list, or null value is rejected with `CONFIG_ERROR` (exit 5) naming the key and its actual type. `--branch` is optional on a fresh clone *(since v0.93.1)*. It defaults to the target's production branch, resolved from the API the same way `sync init` does. Pass `--branch ` only to target a dev branch. **Data-app runtime type (since vNEXT)**: a `keboola.data-apps` config's type (`python-js` / `streamlit`) lives only on the Data Science `/apps` record, so `sync pull` records it in `_keboola.data_app_type` and clone sends it through `create_app`. Re-pull the source before cloning a tree pulled by an older version, or the app deploys under the platform default. - `sync diff --project ALIAS [--all-projects] [--branch ID]` -- 3-way diff (local vs base vs remote), detects conflicts. `--branch` (0.47.0+) per-invocation dev-branch override. Branch-scoped since v0.89.0 (issue #649): the local side is read from exactly ONE tree (the target branch's subtree, or `main/` when the target has none). Manifest entries belonging to another branch's tree -- what `sync pull --branch ` leaves behind when it re-targets the manifest -- are excluded from the changeset and reported under `orphaned` (`summary.orphaned` + details with `component_id`, `config_id`, `path`, `branch_id`, `branch_path`, `exists_on_target`, `reason`, `hint`); human mode previews the first 10. An orphaned FILE whose `_keboola.config_id` still resolves on the target is adopted (diffed as `unchanged`/`modified`), never re-created; same-tree id claims keep the #482/#497 fork-by-copy CREATE. Fix a non-zero `summary.orphaned` with `sync pull`. **Since 0.91.0 (#686)** a manifest entry without `metadata.config_hash_version` (written by a pre-0.91.0 kbagent) is compared leniently: a stored hash equal to the pre-0.91.0 hash of the SAME remote config counts as in sync, so the phantom `codes changed` entries disappear immediately; every other field is still pinned by that hash, so real remote drift is unaffected. One `sync pull` per project stamps the version and ends the leniency. Ignored components (since 0.91.0) -- `keboola.sandboxes`, `keboola.mcp-server-tool`, and anything listed in the manifest's `ignoredComponents` -- are excluded from BOTH sides of the comparison, so a stale local directory for one of them never shows up as `DELETED`. - `sync status [--directory DIR]` -- show locally modified/added/deleted configs. Also surfaces `plaintext_secret_warnings` (since 0.55.0): in-sync configs/rows whose `#`-secrets are still plaintext on the remote (a leftover from pre-0.54.0 writes; #378). Pending (un-pushed) edits are not flagged. Fix = re-push on >=0.54.0 + rotate (version history keeps the plaintext). - `sync branch-link --project ALIAS [--branch-id ID] [--branch-name NAME]` -- link git branch to Keboola dev branch diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index 36cf2f1e..462f47e3 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -783,6 +783,14 @@ a `name_drift_warnings: [...]` array on the result envelope. The still runs, so a future operator who wants to audit can flip the flag off without losing data. +## `sync` carries a data app's runtime type: pull records it, push and clone send it + +A `keboola.data-apps` config's runtime type (`python-js` / `streamlit` / ...) lives only on the Data Science `/apps` record, never in the Storage config body. So `sync pull` used to drop it, and `sync push` / `sync clone` recreated the config through the Storage API alone. A cloned `python-js` app then deployed under the platform default, `streamlit` (since vNEXT). + +`sync pull` now reads the type from the DS `/apps` list and records it in the config's `_keboola` block as `data_app_type`. The config hash already ignores that key, so it adds no `sync diff` noise. `sync push` and `sync clone` route a `keboola.data-apps` CREATE through the Data Science `create_app` when the local config carries a `data_app_type`. That call sends the type and writes the new app's `parameters.id`. A config with no recorded type still uses the plain `create_config` path. + +The DS `/apps` list also returns sandbox and workspace records. Each carries a parent component's id and a backend `type` such as `snowflake`. So kbagent builds the type map from `componentId == keboola.data-apps` records only. + ## `semantic-layer search-context` + `get-context` cover the upstream `search_semantic_context` / `get_semantic_context` parity `kbagent semantic-layer search-context --project P [--pattern G ...] [--type T] [--limit N]` diff --git a/src/keboola_agent_cli/services/_sync_data_app.py b/src/keboola_agent_cli/services/_sync_data_app.py new file mode 100644 index 00000000..460ed7de --- /dev/null +++ b/src/keboola_agent_cli/services/_sync_data_app.py @@ -0,0 +1,130 @@ +"""Data-app create for the sync engine (CLI-8). + +``sync push`` / ``sync clone`` create configs through the Storage API only. +A ``keboola.data-apps`` config has a second half, the Data Science ``/apps`` +deployment record, and the runtime type (``python-js`` / ``streamlit`` / ...) +lives ONLY on that record. This module owns the one create path that carries +the type, kept out of ``data_app_service`` so that already-large module does +not grow past its file-size budget. +""" + +from __future__ import annotations + +import logging +from typing import Any + +from ..client import KeboolaClient +from ..data_science_client import DataScienceClient +from ..errors import ErrorCode, KeboolaApiError +from .data_app_service import DATA_APP_COMPONENT_ID + +logger = logging.getLogger(__name__) + + +def create_synced_data_app( + storage_client: KeboolaClient, + ds_client: DataScienceClient, + *, + name: str, + description: str, + type_: str, + configuration: dict[str, Any], + branch_id: int | None, + is_disabled: bool = False, +) -> dict[str, Any]: + """Create a ``keboola.data-apps`` config together with its Data Science + deployment record, carrying the runtime ``type_`` (CLI-8). + + ``sync push`` / ``sync clone`` create configs through the Storage API only + (``create_config``). A ``keboola.data-apps`` config has a second half, the + Data Science ``/apps`` deployment record, and the runtime type + (``python-js`` / ``streamlit`` / ...) lives ONLY on that record, never in + the Storage config body. A plain ``create_config`` therefore leaves the + platform to create the DS record under its default type, so a cloned + ``python-js`` app deploys as ``streamlit``. + + This routes creation through ``create_app`` (which creates BOTH the DS + record with ``type_`` and its Storage config), then fills the full body + via ``update_config``. ``POST /apps`` validates its ``config``: it wants + the create-shell shape (``parameters.size`` / ``autoSuspendAfterSeconds`` + / ``dataApp.slug`` + ``authorization``), not the full Storage body, which + carries ``runtime.backend.size`` instead of ``parameters.size``. So the + create call sends the minimal shell (the same shape as + ``DataAppService.create``) and the update call sends the full body with the + new app's ``parameters.id``. This mutates the passed ``configuration``: + ``parameters.id`` is set to the new app id, so the caller can persist it to + the local file (else the next push reverts the back-pointer). + + If ``update_config`` fails after ``create_app`` already created the record, + the record is deleted, so a failed sync create leaves no orphan app in the + target (the same guard as ``DataAppService.create``). + + Returns the ``update_config`` response (the Storage config, whose ``id`` is + the new config ULID) so the caller's manifest writeback is identical to the + ``create_config`` path. + """ + # Build the minimal shell POST /apps accepts (see docstring). The full + # Storage body -- runtime.backend.size, the git block, the source app id -- + # goes on the update_config call below, not here. + params = configuration.get("parameters") or {} + data_app = params.get("dataApp") or {} + backend = (configuration.get("runtime") or {}).get("backend") or {} + initial_parameters: dict[str, Any] = {"dataApp": {"slug": data_app.get("slug", "")}} + if "size" in backend: + initial_parameters["size"] = backend["size"] + if "autoSuspendAfterSeconds" in params: + initial_parameters["autoSuspendAfterSeconds"] = params["autoSuspendAfterSeconds"] + initial_config: dict[str, Any] = {"parameters": initial_parameters} + if "authorization" in configuration: + initial_config["authorization"] = configuration["authorization"] + + shell = ds_client.create_app( + type_=type_, + name=name, + description="", # full description goes onto the Storage config below + config=initial_config, + branch_id=branch_id, + ) + app_id = str(shell.get("id", "")) + config_id = str(shell.get("configId", "")) + if not app_id or not config_id: + # An id without a configId still leaves a shell behind. Delete it so a + # failed create leaves no orphan app, as the docstring promises. + if app_id: + try: + ds_client.delete_app(app_id) + except Exception: + logger.warning( + "Failed to delete orphan data app %s after an incomplete create response", + app_id, + ) + raise KeboolaApiError( + message="POST /apps response missing id or configId", + status_code=500, + error_code=ErrorCode.API_ERROR, + retryable=False, + ) + + target_params = configuration.setdefault("parameters", {}) + if isinstance(target_params, dict): + target_params["id"] = app_id + + try: + return storage_client.update_config( + component_id=DATA_APP_COMPONENT_ID, + config_id=config_id, + name=name, + description=description, + configuration=configuration, + change_description="Created via kbagent sync", + branch_id=branch_id, + is_disabled=is_disabled, + ) + except Exception: + # The DS record and its bare Storage config exist, but the full body + # did not land. Delete the record so the target keeps no orphan app. + try: + ds_client.delete_app(app_id) + except Exception: + logger.warning("Failed to delete orphan data app %s after a failed sync create", app_id) + raise diff --git a/src/keboola_agent_cli/services/_sync_push_ops.py b/src/keboola_agent_cli/services/_sync_push_ops.py index 0e8f2cc2..6f1dac20 100644 --- a/src/keboola_agent_cli/services/_sync_push_ops.py +++ b/src/keboola_agent_cli/services/_sync_push_ops.py @@ -23,7 +23,9 @@ from ..sync.manifest import Manifest, ManifestConfiguration from ._encryption import encrypt_secrets_in_config from ._sync_baseline import apply_stamp, row_baseline +from ._sync_data_app import create_synced_data_app from ._sync_writeback import writeback_after_push, writeback_create_row_in_manifest +from .data_app_service import DATA_APP_COMPONENT_ID if TYPE_CHECKING: from .sync_service import SyncService @@ -385,11 +387,21 @@ def push_create( *, allow_plaintext_fallback: bool = False, warnings: list[dict[str, Any]] | None = None, + ds_client: Any = None, + ds_branch_id: int | None = None, ) -> dict[str, Any] | None: """Create a new config from a local _config.yml file. ``warnings`` accumulates the ``script[]`` normalization records of :func:`guard_script_shape` for the push envelope. + + ``ds_client`` is a Data Science client, passed only when the tree holds + ``keboola.data-apps`` configs. A data app carries its runtime type on the + DS ``/apps`` record, not in the Storage config, so a plain + ``create_config`` would drop it and the cloned app would deploy under the + platform default (CLI-8). When a data-apps config records its type in the + ``_keboola`` footer, creation is routed through the DS client so the type + travels; otherwise the plain Storage path is unchanged. """ branch_path = service._resolve_source_branch_path(manifest, project_root, branch_id) config_dir = project_root / branch_path / config_path_str @@ -421,14 +433,39 @@ def push_create( allow_plaintext_fallback=allow_plaintext_fallback, ) - result = client.create_config( - component_id=component_id, - name=name, - configuration=configuration, - description=description, - branch_id=branch_id, - is_disabled=bool(local_data.get("is_disabled", False)), - ) + data_app_type = (local_data.get("_keboola") or {}).get("data_app_type") + if component_id == DATA_APP_COMPONENT_ID and data_app_type and ds_client is not None: + # Carry the DS runtime type into the target (CLI-8): create the DS + # /apps record with the type, then fill the Storage config body. + # ds_branch_id is None for a production push (POST /apps wants + # branchId=null there), the dev branch id otherwise. + result = create_synced_data_app( + client, + ds_client, + name=name, + description=description, + type_=data_app_type, + configuration=configuration, + branch_id=ds_branch_id, + is_disabled=bool(local_data.get("is_disabled", False)), + ) + # create_synced_data_app set parameters.id on `configuration` to the new + # app id. Persist it locally too, or the next push (an update) would send + # the stale source id and revert the remote back-pointer. + new_app_id = (configuration.get("parameters") or {}).get("id") + if new_app_id: + pristine_params = pristine_data.setdefault("parameters", {}) + if isinstance(pristine_params, dict): + pristine_params["id"] = new_app_id + else: + result = client.create_config( + component_id=component_id, + name=name, + configuration=configuration, + description=description, + branch_id=branch_id, + is_disabled=bool(local_data.get("is_disabled", False)), + ) new_config_id = result.get("id", "") logger.info("Created config %s/%s (ID: %s)", component_id, name, new_config_id) diff --git a/src/keboola_agent_cli/services/sync_service.py b/src/keboola_agent_cli/services/sync_service.py index 57fc02bd..17cc5064 100644 --- a/src/keboola_agent_cli/services/sync_service.py +++ b/src/keboola_agent_cli/services/sync_service.py @@ -4,6 +4,7 @@ in a dev-friendly format (YAML configs), and tracking local changes. """ +import contextlib import hashlib import json import logging @@ -14,6 +15,7 @@ import yaml +from ..config_store import ConfigStore from ..constants import ( ALWAYS_IGNORED_COMPONENTS, BRANCH_MAPPING_FILENAME, @@ -110,7 +112,12 @@ stamp_created_config, stamp_updated_config, ) -from .base import BaseService, find_default_branch_id +from .base import BaseService, ClientFactory, find_default_branch_id +from .data_app_service import ( + DATA_APP_COMPONENT_ID, + DataScienceClientFactory, + _default_ds_client_factory, +) logger = logging.getLogger(__name__) @@ -250,6 +257,18 @@ class SyncService(BaseService): config_store and client_factory following the BaseService pattern. """ + def __init__( + self, + config_store: ConfigStore, + client_factory: ClientFactory | None = None, + ds_client_factory: DataScienceClientFactory | None = None, + ) -> None: + # ``ds_client_factory`` builds a Data Science client. It is used only + # for ``keboola.data-apps`` configs, whose runtime type lives on the DS + # ``/apps`` record and would otherwise be lost by pull/clone (CLI-8). + super().__init__(config_store=config_store, client_factory=client_factory) + self._ds_client_factory = ds_client_factory or _default_ds_client_factory + # ------------------------------------------------------------------ # init # ------------------------------------------------------------------ @@ -555,6 +574,37 @@ def pull( if with_samples and tables_data: samples_data = fetch_samples(client, tables_data, sample_limit, max_samples) + # Data-app runtime types (CLI-8). The type (python-js / streamlit / ...) + # lives only on the Data Science /apps record, never in the Storage + # config, so it is fetched here and stamped into each data-app's + # _config.yml. One list call covers the project; skipped entirely when + # the tree holds no data apps. A DS failure degrades to no type (the + # pre-fix behavior), never an aborted pull. + # + # /apps also returns sandbox/workspace records (componentId of the + # parent component, e.g. keboola.ex-db-mysql, and a backend `type` such + # as `snowflake`), so the map is restricted to real data-app records -- + # otherwise a workspace's type would land on an unrelated config that + # happens to share the id. + data_app_types: dict[str, str] = {} + ds_types_available = False + if any(comp.get("id") == DATA_APP_COMPONENT_ID for comp in components): + try: + ds_client = self._ds_client_factory(project.stack_url, project.token) + with ds_client: + data_app_types = { + str(app.get("configId")): str(app.get("type")) + for app in ds_client.list_apps() + if app.get("componentId") == DATA_APP_COMPONENT_ID + and app.get("configId") + and app.get("type") + } + ds_types_available = True + except Exception: + logger.warning( + "Failed to fetch data-app types from Data Science API", exc_info=True + ) + # Determine branch directory name branch_dir_name = self._find_branch_path(manifest, branch_id) @@ -710,8 +760,24 @@ def pull( # future regression in the sanitizer or template parsing. _ensure_within_branch(branch_dir, config_dir, component_id, config_id) - # Convert API format to local _config.yml - local_data = api_config_to_local(component_id, cfg, config_id) + # Convert API format to local _config.yml. For a data app the + # runtime type comes from the DS /apps list. If that lookup + # failed, keep the type already on disk instead of stripping it + # -- config_hash ignores _keboola, so a strip would be invisible. + da_type: str | None = None + if component_id == DATA_APP_COMPONENT_ID: + if ds_types_available: + da_type = data_app_types.get(config_id) + else: + existing = self._read_config_file(config_dir) + if existing is not None: + da_type = (existing.get("_keboola") or {}).get("data_app_type") + local_data = api_config_to_local( + component_id, + cfg, + config_id, + data_app_type=da_type, + ) # Hash of API-converted data. Stored as pull_config_hash so # diff can compare it directly with fresh remote data without @@ -1612,7 +1678,28 @@ def push( pushed_details: list[dict[str, str]] = [] manifest_dirty = False - with client: + # A data-app CREATE needs a Data Science client so its runtime type + # travels into the target (CLI-8). Built only when the changeset + # actually creates a data app, and entered alongside ``client`` so its + # close() runs on every exit from the push block, a mid-push raise + # included. + ds_client = None + if any( + c.get("change_type") == "added" and c.get("component_id") == DATA_APP_COMPONENT_ID + for c in changes + if not bool(c.get("is_row")) + ): + ds_client = self._ds_client_factory(project.stack_url, project.token) + ds_context = ds_client if ds_client is not None else contextlib.nullcontext() + + # POST /apps wants branchId=null for the default (production) branch and + # a numeric id only for a dev branch. The sync engine carries production + # as the manifest's default branch id, so map it back to None for the DS + # create (data-app create does the same). Live-verified against 4214. + default_branch_id = manifest.branches[0].id if manifest.branches else None + ds_branch_id = None if branch_id == default_branch_id else branch_id + + with client, ds_context: self._ensure_branch_registered(manifest, branch_id, client) branch_path = self._resolve_source_branch_path(manifest, project_root, branch_id) @@ -1649,6 +1736,8 @@ def push( branch_id, allow_plaintext_fallback=allow_plaintext_fallback, warnings=warnings, + ds_client=ds_client, + ds_branch_id=ds_branch_id, ) if result: new_id = str(result.get("id", "")) diff --git a/src/keboola_agent_cli/sync/config_format.py b/src/keboola_agent_cli/sync/config_format.py index c44c95a3..6f88555e 100644 --- a/src/keboola_agent_cli/sync/config_format.py +++ b/src/keboola_agent_cli/sync/config_format.py @@ -159,6 +159,7 @@ def api_config_to_local( config_id: str, *, legacy_scripts: bool = False, + data_app_type: str | None = None, ) -> dict[str, Any]: """Convert an API configuration response to the local ``_config.yml`` structure. @@ -172,7 +173,7 @@ def api_config_to_local( - ``configuration.storage.input`` -> ``input`` - ``configuration.storage.output`` -> ``output`` - ``configuration.processors`` -> ``processors`` - - ``_keboola``: ``{component_id, config_id}`` + - ``_keboola``: ``{component_id, config_id}`` (+ ``data_app_type`` when given) Any remaining keys inside ``configuration`` that are not explicitly promoted are preserved under a ``_configuration_extra`` key so that @@ -184,6 +185,14 @@ def api_config_to_local( (:func:`_normalize_scripts_legacy`) so the caller can recompute the hash an older kbagent would have stored for this same remote config. Never pass it on a path that WRITES the result. + data_app_type: The Data Science runtime type (``python-js`` / + ``streamlit`` / ...) of a ``keboola.data-apps`` config. It lives + only on the DS ``/apps`` record, never in the Storage config body, + so ``sync push``/``sync clone`` would otherwise drop it and the + cloned app would deploy under the platform default (CLI-8). It is + recorded in the ``_keboola`` footer, which is stripped before + hashing (:data:`diff_engine._IGNORED_KEYS`), so it never shows a + spurious diff. """ configuration: dict[str, Any] = config_data.get("configuration") or {} @@ -223,6 +232,8 @@ def api_config_to_local( "component_id": component_id, "config_id": config_id, } + if data_app_type: + local["_keboola"]["data_app_type"] = data_app_type return local diff --git a/tests/test_sync_config_format.py b/tests/test_sync_config_format.py index c90191e0..395a733d 100644 --- a/tests/test_sync_config_format.py +++ b/tests/test_sync_config_format.py @@ -13,6 +13,7 @@ local_config_to_api, local_row_to_api, ) +from keboola_agent_cli.sync.diff_engine import config_hash SAMPLE_API_CONFIG: dict[str, Any] = { "id": "cfg-123", @@ -143,6 +144,45 @@ def test_api_config_to_local_no_configuration(self) -> None: assert "processors" not in local assert "_configuration_extra" not in local + def test_data_app_type_recorded_in_keboola(self) -> None: + """A data-app's runtime type lands in the _keboola footer (CLI-8).""" + local = api_config_to_local( + "keboola.data-apps", + SAMPLE_API_CONFIG, + SAMPLE_CONFIG_ID, + data_app_type="python-js", + ) + assert local["_keboola"]["data_app_type"] == "python-js" + + def test_data_app_type_absent_by_default(self) -> None: + """Without a type, the footer is unchanged -- non-data-app configs never get the key.""" + local = api_config_to_local(SAMPLE_COMPONENT_ID, SAMPLE_API_CONFIG, SAMPLE_CONFIG_ID) + assert "data_app_type" not in local["_keboola"] + + def test_data_app_type_is_hash_invisible(self) -> None: + """The type sits in _keboola (an ignored key), so it never changes the + config hash -- pulling it in adds no spurious diff on existing configs.""" + without = api_config_to_local("keboola.data-apps", SAMPLE_API_CONFIG, SAMPLE_CONFIG_ID) + with_type = api_config_to_local( + "keboola.data-apps", + SAMPLE_API_CONFIG, + SAMPLE_CONFIG_ID, + data_app_type="streamlit", + ) + assert config_hash(without) == config_hash(with_type) + + def test_data_app_type_does_not_leak_into_api_body(self) -> None: + """local_config_to_api drops _keboola, so the type never reaches the Storage body.""" + local = api_config_to_local( + "keboola.data-apps", + SAMPLE_API_CONFIG, + SAMPLE_CONFIG_ID, + data_app_type="python-js", + ) + _, _, configuration = local_config_to_api(local) + assert "data_app_type" not in configuration + assert "python-js" not in str(configuration) + class TestLocalConfigToApiRoundTrip: """Tests for local_config_to_api() and round-trip conversion.""" diff --git a/tests/test_sync_data_app_type.py b/tests/test_sync_data_app_type.py new file mode 100644 index 00000000..3891311e --- /dev/null +++ b/tests/test_sync_data_app_type.py @@ -0,0 +1,455 @@ +"""sync pull / push carry a data app's runtime type (CLI-8). + +A ``keboola.data-apps`` config has two halves: the Storage config (body) and +the Data Science ``/apps`` deployment record. The runtime type (``python-js`` +/ ``streamlit`` / ...) lives ONLY on the DS record, never in the Storage body. +So a plain pull drops it and a plain push (``create_config``) never sends it -- +a cloned ``python-js`` app then deploys under the platform default +(``streamlit``). + +pull stamps the type into the ``_keboola`` footer; push routes a data-app +CREATE through the DS ``create_app`` so the type travels. +""" + +from pathlib import Path +from typing import Any, Self +from unittest.mock import MagicMock + +import pytest +import yaml + +from helpers import setup_single_project +from keboola_agent_cli.constants import CONFIG_FILENAME +from keboola_agent_cli.errors import ErrorCode, KeboolaApiError +from keboola_agent_cli.services._sync_data_app import create_synced_data_app +from keboola_agent_cli.services.sync_service import SyncService +from test_sync_baseline_stamping import ( + FakeApi, + _client_for, + _config_file, + _init_and_pull, + _sql_components, +) + +DATA_APP_COMPONENT = "keboola.data-apps" + + +class FakeDs: + """Data Science double. + + ``create_app`` also appends the Storage config to the wrapped ``FakeApi``, + exactly as the real ``POST /apps`` does -- otherwise the follow-up + ``update_config`` could not resolve the server-assigned config id. + """ + + def __init__(self, api: FakeApi, list_result: list[dict[str, Any]] | None = None): + self.api = api + self.create_app_calls: list[dict[str, Any]] = [] + self._list_result = list_result or [] + self.closed = False + + def __enter__(self) -> Self: + return self + + def __exit__(self, *args: object) -> bool: + # Mirror DataScienceClient.__exit__, which closes on context exit. + self.close() + return False + + def close(self) -> None: + self.closed = True + + def list_apps(self) -> list[dict[str, Any]]: + return self._list_result + + def create_app( + self, + *, + type_: str, + name: str, + description: str, + config: dict[str, Any], + branch_id: int | None = None, + use_managed_git_repo: bool = False, + ) -> dict[str, Any]: + self.create_app_calls.append( + {"type_": type_, "name": name, "config": config, "branch_id": branch_id} + ) + new_config_id = "cfg-da-new" + new_app_id = "77777" + record = {"id": new_config_id, "name": name, "configuration": config, "rows": []} + for comp in self.api.components: + if comp["id"] == DATA_APP_COMPONENT: + comp["configurations"].append(record) + break + else: + self.api.components.append( + {"id": DATA_APP_COMPONENT, "type": "application", "configurations": [record]} + ) + return {"id": new_app_id, "configId": new_config_id} + + +MYSQL_COMPONENT = "keboola.ex-db-mysql" + + +def _mixed_components(config_id: str = "cfg-da") -> list[dict[str, Any]]: + """A data-app config and a NON-data-app config that shares its id. + + Config ids are unique only per component, so a data app and (say) a MySQL + extractor can legally hold the same id. The MySQL config must never receive + a data_app_type. + """ + return [ + { + "id": DATA_APP_COMPONENT, + "type": "application", + "configurations": [ + { + "id": config_id, + "name": "api-test", + "description": "A JS data app", + # parameters.id is the DS back-pointer; no runtime type here. + "configuration": { + "parameters": {"id": "99999", "dataApp": {"slug": "api-test"}} + }, + "rows": [], + } + ], + }, + { + "id": MYSQL_COMPONENT, + "type": "extractor", + "configurations": [ + { + "id": config_id, # same id, different component + "name": "mysql-ex", + "description": "", + "configuration": {"parameters": {"host": "db.example.com"}}, + "rows": [], + } + ], + }, + ] + + +def _service(store: Any, api: FakeApi, ds: Any) -> SyncService: + # ``ds`` is a structural DataScienceClient double (FakeDs), typed Any so the + # factory signature accepts it. + return SyncService( + config_store=store, + client_factory=lambda url, token: _client_for(api), + ds_client_factory=lambda url, token: ds, + ) + + +def _find_config(project_root: Path, component_id: str) -> dict[str, Any]: + """Return the parsed _config.yml of the pulled config for *component_id*.""" + for path in project_root.rglob(CONFIG_FILENAME): + data = yaml.safe_load(path.read_text(encoding="utf-8")) + if (data.get("_keboola") or {}).get("component_id") == component_id: + return data + raise AssertionError(f"no {component_id} _config.yml in the pulled tree") + + +def _author_data_app(project_root: Path, *, with_type: bool) -> None: + """Write an untracked data-app _config.yml so push sees it as CREATE.""" + keboola: dict[str, Any] = {"component_id": DATA_APP_COMPONENT} + if with_type: + keboola["data_app_type"] = "python-js" + new_dir = _config_file(project_root).parent.parent / "new-data-app" + new_dir.mkdir(parents=True) + (new_dir / CONFIG_FILENAME).write_text( + yaml.safe_dump( + { + "version": 2, + "name": "api-test", + "description": "A JS data app", + "parameters": {"id": "99999", "dataApp": {"slug": "api-test"}}, + "_keboola": keboola, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + + +# =================================================================== +# pull: the DS runtime type lands in the local _config.yml +# =================================================================== + + +def test_pull_stamps_data_app_type(tmp_config_dir: Path, tmp_path: Path) -> None: + """The data-app gets its type; a non-data-app sharing the id does not. + + The DS /apps list mixes real data apps with sandbox/workspace records that + carry another component's id and a backend `type` (e.g. `snowflake`). The + map must be built from data-app records only, and only data-app configs may + be stamped -- these are the two live-observed failure modes (CLI-8). + """ + project_root = tmp_path / "project" + project_root.mkdir() + api = FakeApi(_mixed_components("cfg-da")) + ds = FakeDs( + api, + list_result=[ + {"configId": "cfg-da", "componentId": DATA_APP_COMPONENT, "type": "python-js"}, + # A sandbox record: same config id, another component, a backend type. + # It must not overwrite the data-app's type nor reach the map. + {"configId": "cfg-da", "componentId": MYSQL_COMPONENT, "type": "snowflake"}, + ], + ) + store = setup_single_project(tmp_config_dir) + + service = _service(store, api, ds) + service.init_sync(alias="prod", project_root=project_root) + service.pull(alias="prod", project_root=project_root, no_storage=True, no_jobs=True) + + # The data app took its own type, not the sandbox record's `snowflake`. + assert ( + _find_config(project_root, DATA_APP_COMPONENT)["_keboola"]["data_app_type"] == "python-js" + ) + # The MySQL config -- same id -- was never stamped. + assert "data_app_type" not in _find_config(project_root, MYSQL_COMPONENT)["_keboola"] + + +def test_pull_without_data_apps_never_calls_ds(tmp_config_dir: Path, tmp_path: Path) -> None: + """No data-apps in the tree => the DS API is not touched at all.""" + project_root = tmp_path / "project" + api = FakeApi(_sql_components(["SELECT 1;"])) + + # A DS double that fails if used proves pull skips it when unneeded. + class ExplodingDs(FakeDs): + def list_apps(self) -> list[dict[str, Any]]: + raise AssertionError("list_apps must not be called without data apps") + + store = setup_single_project(tmp_config_dir) + service = _service(store, api, ExplodingDs(api)) + service.init_sync(alias="prod", project_root=project_root) + service.pull(alias="prod", project_root=project_root, no_storage=True, no_jobs=True) + + +def test_pull_preserves_type_when_ds_lookup_fails(tmp_config_dir: Path, tmp_path: Path) -> None: + """A DS outage during pull must not strip an already-recorded data_app_type. + + config_hash ignores _keboola, so a strip would be invisible to sync diff -- + a later clone would then deploy under the platform default. + """ + project_root = tmp_path / "project" + project_root.mkdir() + api = FakeApi(_mixed_components("cfg-da")) + store = setup_single_project(tmp_config_dir) + + # First pull with a working DS records the type. + working_ds = FakeDs( + api, + list_result=[ + {"configId": "cfg-da", "componentId": DATA_APP_COMPONENT, "type": "python-js"} + ], + ) + service = _service(store, api, working_ds) + service.init_sync(alias="prod", project_root=project_root) + service.pull(alias="prod", project_root=project_root, no_storage=True, no_jobs=True) + assert ( + _find_config(project_root, DATA_APP_COMPONENT)["_keboola"]["data_app_type"] == "python-js" + ) + + # Second pull with a failing DS must keep the type on disk, not strip it. + class ExplodingDs(FakeDs): + def list_apps(self) -> list[dict[str, Any]]: + raise RuntimeError("DS outage") + + _service(store, api, ExplodingDs(api)).pull( + alias="prod", project_root=project_root, no_storage=True, no_jobs=True + ) + assert ( + _find_config(project_root, DATA_APP_COMPONENT)["_keboola"]["data_app_type"] == "python-js" + ) + + +# =================================================================== +# push: a data-app CREATE routes through the DS client with the type +# =================================================================== + + +def test_push_create_data_app_sends_type(tmp_config_dir: Path, tmp_path: Path) -> None: + project_root = tmp_path / "project" + api = FakeApi(_sql_components(["SELECT 1;"])) + store = _init_and_pull(tmp_config_dir, project_root, api) + _author_data_app(project_root, with_type=True) + + ds = FakeDs(api) + result = _service(store, api, ds).push(alias="prod", project_root=project_root) + + assert result["errors"] == [] + assert result["created"] == 1 + # The type reached the DS record -- the fix. + assert len(ds.create_app_calls) == 1 + assert ds.create_app_calls[0]["type_"] == "python-js" + # A production push maps to branchId=null for POST /apps. + assert ds.create_app_calls[0]["branch_id"] is None + # The Storage body was filled and the stale back-pointer repointed at the + # newly assigned app id. + da_updates = [c for c in api.update_calls if c["component_id"] == DATA_APP_COMPONENT] + assert da_updates, "expected an update_config for the data-app" + assert da_updates[-1]["configuration"]["parameters"]["id"] == "77777" + assert ds.closed is True + # The new app id is persisted to the LOCAL file too, so a later push does + # not revert the remote back-pointer to the source id. + assert _find_config(project_root, DATA_APP_COMPONENT)["parameters"]["id"] == "77777" + + +def test_push_create_data_app_without_type_falls_back(tmp_config_dir: Path, tmp_path: Path) -> None: + """A tree pulled before the fix (no recorded type) keeps the old behavior: + a plain create_config, no DS record -- no regression, no wrong type sent.""" + project_root = tmp_path / "project" + api = FakeApi(_sql_components(["SELECT 1;"])) + store = _init_and_pull(tmp_config_dir, project_root, api) + _author_data_app(project_root, with_type=False) + + ds = FakeDs(api) + result = _service(store, api, ds).push(alias="prod", project_root=project_root) + + assert result["errors"] == [] + assert result["created"] == 1 + # No type recorded => DS is never asked to create the app. + assert ds.create_app_calls == [] + # It went through the plain Storage create instead (FakeApi mints "cfg-new"). + created = next( + c + for comp in api.components + if comp["id"] == DATA_APP_COMPONENT + for c in comp["configurations"] + ) + assert created["id"] == "cfg-new" + + +# =================================================================== +# create_synced_data_app -- the DS-aware create the push path delegates to +# =================================================================== + + +def _cloned_body() -> dict[str, Any]: + # parameters.id still points at the SOURCE project's app. + return { + "parameters": {"id": "99999", "dataApp": {"slug": "api-test"}}, + "runtime": {"backend": {"size": "tiny"}}, + } + + +class TestCreateSyncedDataApp: + """The helper that carries a data app's runtime type into the target.""" + + def test_creates_ds_record_with_type(self) -> None: + ds = MagicMock() + ds.create_app.return_value = {"id": "77777", "configId": "01NEWULID"} + storage = MagicMock() + storage.update_config.return_value = {"id": "01NEWULID", "version": "2"} + + result = create_synced_data_app( + storage, + ds, + name="api-test", + description="desc", + type_="python-js", + configuration=_cloned_body(), + branch_id=None, + ) + + # The type reaches the DS record -- the whole point of the fix. + assert ds.create_app.call_args.kwargs["type_"] == "python-js" + # The Storage body is filled at the SERVER-assigned config id. + assert storage.update_config.call_args.kwargs["config_id"] == "01NEWULID" + # The caller's writeback keys off result["id"] == the new config ULID. + assert result["id"] == "01NEWULID" + + def test_create_uses_minimal_shell_update_writes_full_body(self) -> None: + """create_app gets the shell shape POST /apps accepts; update_config gets + the full body with the new app id. + + POST /apps rejects the full Storage body with HTTP 422 (it wants + parameters.size, not runtime.backend.size), and it must never see the + source project's app id. Live-verified against project 4214. + """ + ds = MagicMock() + ds.create_app.return_value = {"id": "77777", "configId": "01NEWULID"} + storage = MagicMock() + storage.update_config.return_value = {"id": "01NEWULID"} + + create_synced_data_app( + storage, + ds, + name="api-test", + description="", + type_="streamlit", + configuration=_cloned_body(), + branch_id=None, + ) + + create_body = ds.create_app.call_args.kwargs["config"] + assert create_body["parameters"]["dataApp"]["slug"] == "api-test" + assert create_body["parameters"]["size"] == "tiny" # from runtime.backend.size + assert "runtime" not in create_body # the full-body shape POST /apps rejects + assert "id" not in create_body["parameters"] # never the source app id + put_body = storage.update_config.call_args.kwargs["configuration"] + assert put_body["parameters"]["id"] == "77777" + + def test_forwards_is_disabled(self) -> None: + ds = MagicMock() + ds.create_app.return_value = {"id": "77777", "configId": "01NEWULID"} + storage = MagicMock() + storage.update_config.return_value = {"id": "01NEWULID"} + + create_synced_data_app( + storage, + ds, + name="api-test", + description="", + type_="python-js", + configuration=_cloned_body(), + branch_id=None, + is_disabled=True, + ) + + assert storage.update_config.call_args.kwargs["is_disabled"] is True + + def test_deletes_orphan_when_update_fails(self) -> None: + """A create_app that succeeds then an update_config that fails must not + leave an orphan app in the target.""" + ds = MagicMock() + ds.create_app.return_value = {"id": "77777", "configId": "01NEWULID"} + storage = MagicMock() + storage.update_config.side_effect = RuntimeError("boom") + + with pytest.raises(RuntimeError): + create_synced_data_app( + storage, + ds, + name="api-test", + description="", + type_="python-js", + configuration=_cloned_body(), + branch_id=None, + ) + + ds.delete_app.assert_called_once_with("77777") + + def test_missing_config_id_raises_and_cleans_up(self) -> None: + """An id without a configId leaves a shell behind -- it must be deleted, + so a failed create leaves no orphan app (docstring promise).""" + ds = MagicMock() + ds.create_app.return_value = {"id": "77777"} # no configId + storage = MagicMock() + + with pytest.raises(KeboolaApiError) as exc: + create_synced_data_app( + storage, + ds, + name="api-test", + description="", + type_="python-js", + configuration=_cloned_body(), + branch_id=None, + ) + assert exc.value.error_code == ErrorCode.API_ERROR + storage.update_config.assert_not_called() + ds.delete_app.assert_called_once_with("77777")