From 78bdc925daeebfc44527e52d8b80967fe28f33c2 Mon Sep 17 00:00:00 2001 From: soustruh Date: Thu, 10 Sep 2026 02:13:11 +0200 Subject: [PATCH 1/4] fix(sync): keep a data app's runtime type through pull and clone (CLI-8) A data app's runtime type (python-js, streamlit, ...) lives only on the Data Science /apps record, not in the Storage config. sync pull read only the Storage config, so it lost the type. sync push and sync clone recreated the config through the Storage API alone. So a cloned python-js app deployed as the platform default, streamlit. The fix carries the type in two steps. On pull, kbagent reads the type from the /apps list. It writes the type into the config's _keboola metadata block. That block holds kbagent's own bookkeeping in each _config.yml and never reaches the Storage API. kbagent excludes that block from the config hash, so sync diff does not report the new line as a change. On push, kbagent creates a keboola.data-apps config through the Data Science create_app call. That call sends the type and updates the parameters.id link to the new app. The /apps list also returns sandbox and workspace records. So kbagent builds the type map from data-app records only. It writes the type into data-app configs only. --- .../services/_sync_push_ops.py | 40 ++- .../services/data_app_service.py | 65 +++++ .../services/sync_service.py | 74 ++++- src/keboola_agent_cli/sync/config_format.py | 13 +- tests/test_data_app_service.py | 84 ++++++ tests/test_sync_config_format.py | 40 +++ tests/test_sync_data_app_type.py | 272 ++++++++++++++++++ 7 files changed, 577 insertions(+), 11 deletions(-) create mode 100644 tests/test_sync_data_app_type.py diff --git a/src/keboola_agent_cli/services/_sync_push_ops.py b/src/keboola_agent_cli/services/_sync_push_ops.py index 0e8f2cc2..91ea985f 100644 --- a/src/keboola_agent_cli/services/_sync_push_ops.py +++ b/src/keboola_agent_cli/services/_sync_push_ops.py @@ -24,6 +24,7 @@ from ._encryption import encrypt_secrets_in_config from ._sync_baseline import apply_stamp, row_baseline from ._sync_writeback import writeback_after_push, writeback_create_row_in_manifest +from .data_app_service import DATA_APP_COMPONENT_ID, create_synced_data_app if TYPE_CHECKING: from .sync_service import SyncService @@ -385,11 +386,20 @@ def push_create( *, allow_plaintext_fallback: bool = False, warnings: list[dict[str, Any]] | None = None, + ds_client: Any = 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 +431,28 @@ 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. + result = create_synced_data_app( + client, + ds_client, + name=name, + description=description, + type_=data_app_type, + configuration=configuration, + branch_id=branch_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/data_app_service.py b/src/keboola_agent_cli/services/data_app_service.py index 33eabfa7..a01cca6d 100644 --- a/src/keboola_agent_cli/services/data_app_service.py +++ b/src/keboola_agent_cli/services/data_app_service.py @@ -141,6 +141,71 @@ def _has_control_chars(value: str, *, allow_whitespace: bool = False) -> bool: SECRET_OR_PLAIN_KEY_PATTERN = re.compile(r"^#?[A-Za-z][A-Za-z0-9_-]{0,63}$") +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, +) -> 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 lazily mint 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``. The ``parameters.id`` back-pointer is repointed at + the newly assigned app id — the cloned body still carries the SOURCE + project's app id, which is meaningless in the target. + + Returns the ``update_config`` response (the Storage config, whose ``id`` + is the newly assigned config ULID) so the caller's manifest writeback is + identical to the ``create_config`` path. + """ + shell = ds_client.create_app( + type_=type_, + name=name, + description="", # full description goes onto the Storage config below + config=configuration, + branch_id=branch_id, + ) + app_id = str(shell.get("id", "")) + config_id = str(shell.get("configId", "")) + if not app_id or not config_id: + raise KeboolaApiError( + message="POST /apps response missing id or configId", + status_code=500, + error_code=ErrorCode.API_ERROR, + retryable=False, + ) + + # Repoint the DS back-pointer at the target app; the cloned body still + # carries the source project's app id. + params = configuration.setdefault("parameters", {}) + if isinstance(params, dict): + params["id"] = app_id + + 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, + ) + + class DataAppService(BaseService): """Lifecycle service for Keboola data apps. diff --git a/src/keboola_agent_cli/services/sync_service.py b/src/keboola_agent_cli/services/sync_service.py index 57fc02bd..dc1413cc 100644 --- a/src/keboola_agent_cli/services/sync_service.py +++ b/src/keboola_agent_cli/services/sync_service.py @@ -14,6 +14,7 @@ import yaml +from ..config_store import ConfigStore from ..constants import ( ALWAYS_IGNORED_COMPONENTS, BRANCH_MAPPING_FILENAME, @@ -110,7 +111,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 +256,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 +573,35 @@ 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] = {} + 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") + } + 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) @@ -711,7 +758,16 @@ def pull( _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) + local_data = api_config_to_local( + component_id, + cfg, + config_id, + data_app_type=( + data_app_types.get(config_id) + if component_id == DATA_APP_COMPONENT_ID + else None + ), + ) # Hash of API-converted data. Stored as pull_config_hash so # diff can compare it directly with fresh remote data without @@ -1630,6 +1686,16 @@ def push( created_id_map: dict[tuple[str, str], str] = {} created_configs: list[CreatedConfig] = [] + # 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; closed after Phase A. + ds_client = None + if any( + c.get("change_type") == "added" and c.get("component_id") == DATA_APP_COMPONENT_ID + for c in config_changes + ): + ds_client = self._ds_client_factory(project.stack_url, project.token) + # ---- Phase A: config creates / updates / deletes ------------- for change in config_changes: change_type = change["change_type"] @@ -1649,6 +1715,7 @@ def push( branch_id, allow_plaintext_fallback=allow_plaintext_fallback, warnings=warnings, + ds_client=ds_client, ) if result: new_id = str(result.get("id", "")) @@ -1765,6 +1832,9 @@ def push( raise self._record_push_error(errors, change_type, component_id, config_id, exc) + if ds_client is not None: + ds_client.close() + # ---- Phase B: row creates / updates / deletes ---------------- # row placeholder id -> ULID; ULID parent -> rows created under it. created_row_id_map: dict[str, str] = {} 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_data_app_service.py b/tests/test_data_app_service.py index bf9d111a..eac50b2d 100644 --- a/tests/test_data_app_service.py +++ b/tests/test_data_app_service.py @@ -25,6 +25,7 @@ _redact_git_block, _redact_storage_config, _secret_fingerprint, + create_synced_data_app, ) TEST_TOKEN = "901-55555-fakeTestTokenDoNotUseXXXXXXXX" @@ -1904,3 +1905,86 @@ def test_tail_app_logs_no_params_sends_clean_url(self, httpx_mock) -> None: text = client.tail_app_logs("42") assert text == "full buffer\n" + + +# --------------------------------------------------------------------------- +# create_synced_data_app -- the sync/clone create path (CLI-8) +# --------------------------------------------------------------------------- + + +class TestCreateSyncedDataApp: + """The helper sync push uses to carry a data app's runtime type. + + A ``keboola.data-apps`` config created through the Storage API alone loses + its runtime type, since the type lives only on the Data Science ``/apps`` + record. This helper creates that record with the type, then fills the + Storage body. + """ + + def _body(self) -> dict[str, Any]: + # A cloned body: parameters.id still points at the SOURCE project's app. + return { + "parameters": {"id": "99999", "dataApp": {"slug": "api-test"}}, + "runtime": {"backend": {"size": "tiny"}}, + } + + def test_creates_ds_record_with_type(self) -> None: + ds = MagicMock() + ds.create_app.return_value = {"id": "43683849", "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=self._body(), + branch_id=None, + ) + + # The type is sent to the DS record -- the whole point of the fix. + assert ds.create_app.call_args.kwargs["type_"] == "python-js" + # Storage config filled at the SERVER-assigned config id, not a client guess. + assert storage.update_config.call_args.kwargs["config_id"] == "01NEWULID" + # Caller's writeback keys off result["id"] == the new config ULID. + assert result["id"] == "01NEWULID" + + def test_repoints_stale_back_pointer(self) -> None: + """parameters.id is rewritten from the source app id to the new one.""" + ds = MagicMock() + ds.create_app.return_value = {"id": "43683849", "configId": "01NEWULID"} + storage = MagicMock() + storage.update_config.return_value = {"id": "01NEWULID"} + + create_synced_data_app( + storage, + ds, + name="api-test", + description="", + type_="streamlit", + configuration=self._body(), + branch_id=None, + ) + + put_body = storage.update_config.call_args.kwargs["configuration"] + assert put_body["parameters"]["id"] == "43683849" + + def test_missing_config_id_raises(self) -> None: + ds = MagicMock() + ds.create_app.return_value = {"id": "43683849"} # no configId + storage = MagicMock() + + with pytest.raises(KeboolaApiError) as exc: + create_synced_data_app( + storage, + ds, + name="api-test", + description="", + type_="python-js", + configuration=self._body(), + branch_id=None, + ) + assert exc.value.error_code == ErrorCode.API_ERROR + storage.update_config.assert_not_called() 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..d700b2d0 --- /dev/null +++ b/tests/test_sync_data_app_type.py @@ -0,0 +1,272 @@ +"""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 + +import yaml + +from helpers import setup_single_project +from keboola_agent_cli.constants import CONFIG_FILENAME +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: + 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}) + 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) + + +# =================================================================== +# 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" + # 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 + + +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" From e278263f73d2e28878862da3ff16b121b22f376c Mon Sep 17 00:00:00 2001 From: soustruh Date: Fri, 11 Sep 2026 00:33:34 +0200 Subject: [PATCH 2/4] fix(sync): address #752 review for the data-app-type fix (CLI-8) - Move create_synced_data_app to a new _sync_data_app module. In data_app_service.py it broke the loc-check file-size gate. - Carry the config's is_disabled flag on the DS create path, a regression from the create_config path, which honored it. - Close the Data Science client through a context manager, so a mid-push raise no longer leaks it. - Delete the DS record if update_config fails after create_app, so a failed create leaves no orphan app in the target. - Drop the stale parameters.id from the create_app call, so the create never sends the source project's app id. - Add a gotchas.md entry for the behavior, with matching notes in keboola-expert.md and commands-reference.md, tagged vNEXT. --- plugins/kbagent/agents/keboola-expert.md | 1 + .../kbagent/references/commands-reference.md | 2 +- .../skills/kbagent/references/gotchas.md | 8 ++ .../services/_sync_data_app.py | 107 +++++++++++++++ .../services/_sync_push_ops.py | 4 +- .../services/data_app_service.py | 65 --------- .../services/sync_service.py | 30 +++-- tests/test_data_app_service.py | 84 ------------ tests/test_sync_data_app_type.py | 126 ++++++++++++++++++ 9 files changed, 262 insertions(+), 165 deletions(-) create mode 100644 src/keboola_agent_cli/services/_sync_data_app.py 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..4fffe719 --- /dev/null +++ b/src/keboola_agent_cli/services/_sync_data_app.py @@ -0,0 +1,107 @@ +"""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 copy +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``. The cloned body still points ``parameters.id`` at + the SOURCE project's app, so the create call drops that id and the update + call writes the new app's id. + + 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. + """ + # The cloned body still points parameters.id at the source project's app. + # Drop that stale id on create; the update call writes the correct one. + create_body = copy.deepcopy(configuration) + create_params = create_body.get("parameters") + if isinstance(create_params, dict): + create_params.pop("id", None) + + shell = ds_client.create_app( + type_=type_, + name=name, + description="", # full description goes onto the Storage config below + config=create_body, + branch_id=branch_id, + ) + app_id = str(shell.get("id", "")) + config_id = str(shell.get("configId", "")) + if not app_id or not config_id: + raise KeboolaApiError( + message="POST /apps response missing id or configId", + status_code=500, + error_code=ErrorCode.API_ERROR, + retryable=False, + ) + + params = configuration.setdefault("parameters", {}) + if isinstance(params, dict): + 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 91ea985f..e31bb2d5 100644 --- a/src/keboola_agent_cli/services/_sync_push_ops.py +++ b/src/keboola_agent_cli/services/_sync_push_ops.py @@ -23,8 +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, create_synced_data_app +from .data_app_service import DATA_APP_COMPONENT_ID if TYPE_CHECKING: from .sync_service import SyncService @@ -443,6 +444,7 @@ def push_create( type_=data_app_type, configuration=configuration, branch_id=branch_id, + is_disabled=bool(local_data.get("is_disabled", False)), ) else: result = client.create_config( diff --git a/src/keboola_agent_cli/services/data_app_service.py b/src/keboola_agent_cli/services/data_app_service.py index a01cca6d..33eabfa7 100644 --- a/src/keboola_agent_cli/services/data_app_service.py +++ b/src/keboola_agent_cli/services/data_app_service.py @@ -141,71 +141,6 @@ def _has_control_chars(value: str, *, allow_whitespace: bool = False) -> bool: SECRET_OR_PLAIN_KEY_PATTERN = re.compile(r"^#?[A-Za-z][A-Za-z0-9_-]{0,63}$") -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, -) -> 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 lazily mint 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``. The ``parameters.id`` back-pointer is repointed at - the newly assigned app id — the cloned body still carries the SOURCE - project's app id, which is meaningless in the target. - - Returns the ``update_config`` response (the Storage config, whose ``id`` - is the newly assigned config ULID) so the caller's manifest writeback is - identical to the ``create_config`` path. - """ - shell = ds_client.create_app( - type_=type_, - name=name, - description="", # full description goes onto the Storage config below - config=configuration, - branch_id=branch_id, - ) - app_id = str(shell.get("id", "")) - config_id = str(shell.get("configId", "")) - if not app_id or not config_id: - raise KeboolaApiError( - message="POST /apps response missing id or configId", - status_code=500, - error_code=ErrorCode.API_ERROR, - retryable=False, - ) - - # Repoint the DS back-pointer at the target app; the cloned body still - # carries the source project's app id. - params = configuration.setdefault("parameters", {}) - if isinstance(params, dict): - params["id"] = app_id - - 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, - ) - - class DataAppService(BaseService): """Lifecycle service for Keboola data apps. diff --git a/src/keboola_agent_cli/services/sync_service.py b/src/keboola_agent_cli/services/sync_service.py index dc1413cc..eb018371 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 @@ -1668,7 +1669,21 @@ 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() + + with client, ds_context: self._ensure_branch_registered(manifest, branch_id, client) branch_path = self._resolve_source_branch_path(manifest, project_root, branch_id) @@ -1686,16 +1701,6 @@ def push( created_id_map: dict[tuple[str, str], str] = {} created_configs: list[CreatedConfig] = [] - # 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; closed after Phase A. - ds_client = None - if any( - c.get("change_type") == "added" and c.get("component_id") == DATA_APP_COMPONENT_ID - for c in config_changes - ): - ds_client = self._ds_client_factory(project.stack_url, project.token) - # ---- Phase A: config creates / updates / deletes ------------- for change in config_changes: change_type = change["change_type"] @@ -1832,9 +1837,6 @@ def push( raise self._record_push_error(errors, change_type, component_id, config_id, exc) - if ds_client is not None: - ds_client.close() - # ---- Phase B: row creates / updates / deletes ---------------- # row placeholder id -> ULID; ULID parent -> rows created under it. created_row_id_map: dict[str, str] = {} diff --git a/tests/test_data_app_service.py b/tests/test_data_app_service.py index eac50b2d..bf9d111a 100644 --- a/tests/test_data_app_service.py +++ b/tests/test_data_app_service.py @@ -25,7 +25,6 @@ _redact_git_block, _redact_storage_config, _secret_fingerprint, - create_synced_data_app, ) TEST_TOKEN = "901-55555-fakeTestTokenDoNotUseXXXXXXXX" @@ -1905,86 +1904,3 @@ def test_tail_app_logs_no_params_sends_clean_url(self, httpx_mock) -> None: text = client.tail_app_logs("42") assert text == "full buffer\n" - - -# --------------------------------------------------------------------------- -# create_synced_data_app -- the sync/clone create path (CLI-8) -# --------------------------------------------------------------------------- - - -class TestCreateSyncedDataApp: - """The helper sync push uses to carry a data app's runtime type. - - A ``keboola.data-apps`` config created through the Storage API alone loses - its runtime type, since the type lives only on the Data Science ``/apps`` - record. This helper creates that record with the type, then fills the - Storage body. - """ - - def _body(self) -> dict[str, Any]: - # A cloned body: parameters.id still points at the SOURCE project's app. - return { - "parameters": {"id": "99999", "dataApp": {"slug": "api-test"}}, - "runtime": {"backend": {"size": "tiny"}}, - } - - def test_creates_ds_record_with_type(self) -> None: - ds = MagicMock() - ds.create_app.return_value = {"id": "43683849", "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=self._body(), - branch_id=None, - ) - - # The type is sent to the DS record -- the whole point of the fix. - assert ds.create_app.call_args.kwargs["type_"] == "python-js" - # Storage config filled at the SERVER-assigned config id, not a client guess. - assert storage.update_config.call_args.kwargs["config_id"] == "01NEWULID" - # Caller's writeback keys off result["id"] == the new config ULID. - assert result["id"] == "01NEWULID" - - def test_repoints_stale_back_pointer(self) -> None: - """parameters.id is rewritten from the source app id to the new one.""" - ds = MagicMock() - ds.create_app.return_value = {"id": "43683849", "configId": "01NEWULID"} - storage = MagicMock() - storage.update_config.return_value = {"id": "01NEWULID"} - - create_synced_data_app( - storage, - ds, - name="api-test", - description="", - type_="streamlit", - configuration=self._body(), - branch_id=None, - ) - - put_body = storage.update_config.call_args.kwargs["configuration"] - assert put_body["parameters"]["id"] == "43683849" - - def test_missing_config_id_raises(self) -> None: - ds = MagicMock() - ds.create_app.return_value = {"id": "43683849"} # no configId - storage = MagicMock() - - with pytest.raises(KeboolaApiError) as exc: - create_synced_data_app( - storage, - ds, - name="api-test", - description="", - type_="python-js", - configuration=self._body(), - branch_id=None, - ) - assert exc.value.error_code == ErrorCode.API_ERROR - storage.update_config.assert_not_called() diff --git a/tests/test_sync_data_app_type.py b/tests/test_sync_data_app_type.py index d700b2d0..456154c0 100644 --- a/tests/test_sync_data_app_type.py +++ b/tests/test_sync_data_app_type.py @@ -13,11 +13,15 @@ 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, @@ -48,6 +52,8 @@ 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: @@ -270,3 +276,123 @@ def test_push_create_data_app_without_type_falls_back(tmp_config_dir: Path, tmp_ 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_omits_stale_id_and_update_writes_the_new_one(self) -> None: + """create_app never sees the source app id; update_config writes the new one.""" + 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 "id" not in create_body["parameters"] + 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(self) -> None: + 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() From c26867af3d5d66f48aab766ee32804cc227fd031 Mon Sep 17 00:00:00 2001 From: soustruh Date: Fri, 11 Sep 2026 01:19:07 +0200 Subject: [PATCH 3/4] fix(sync): correct the data-app create body and branch for the DS API (CLI-8) The live test against a real Data Science API found two create-path bugs that the mocked tests missed, because the Data Science double ignores the body shape and branch_id. - Build the create-shell shape for POST /apps: parameters.size, autoSuspendAfterSeconds, dataApp.slug, and authorization. The full Storage body carries runtime.backend.size, which POST /apps rejects with HTTP 422. - Pass branchId=null to POST /apps on a production push. The sync engine carries production as the manifest default branch id, but POST /apps wants null there and a numeric id only on a dev branch. Live check: a sync push of a python-js app to project 4214 created a DS record with type=python-js. I then deleted the test app. The unit tests now assert the shell shape and branchId=null. --- .../services/_sync_data_app.py | 39 ++++++++++++------- .../services/_sync_push_ops.py | 5 ++- .../services/sync_service.py | 8 ++++ tests/test_sync_data_app_type.py | 21 ++++++++-- 4 files changed, 54 insertions(+), 19 deletions(-) diff --git a/src/keboola_agent_cli/services/_sync_data_app.py b/src/keboola_agent_cli/services/_sync_data_app.py index 4fffe719..4828b2c3 100644 --- a/src/keboola_agent_cli/services/_sync_data_app.py +++ b/src/keboola_agent_cli/services/_sync_data_app.py @@ -10,7 +10,6 @@ from __future__ import annotations -import copy import logging from typing import Any @@ -46,9 +45,13 @@ def create_synced_data_app( 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``. The cloned body still points ``parameters.id`` at - the SOURCE project's app, so the create call drops that id and the update - call writes the new app's id. + 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``. 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 @@ -58,18 +61,26 @@ def create_synced_data_app( the new config ULID) so the caller's manifest writeback is identical to the ``create_config`` path. """ - # The cloned body still points parameters.id at the source project's app. - # Drop that stale id on create; the update call writes the correct one. - create_body = copy.deepcopy(configuration) - create_params = create_body.get("parameters") - if isinstance(create_params, dict): - create_params.pop("id", None) + # 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=create_body, + config=initial_config, branch_id=branch_id, ) app_id = str(shell.get("id", "")) @@ -82,9 +93,9 @@ def create_synced_data_app( retryable=False, ) - params = configuration.setdefault("parameters", {}) - if isinstance(params, dict): - params["id"] = app_id + target_params = configuration.setdefault("parameters", {}) + if isinstance(target_params, dict): + target_params["id"] = app_id try: return storage_client.update_config( diff --git a/src/keboola_agent_cli/services/_sync_push_ops.py b/src/keboola_agent_cli/services/_sync_push_ops.py index e31bb2d5..a104d805 100644 --- a/src/keboola_agent_cli/services/_sync_push_ops.py +++ b/src/keboola_agent_cli/services/_sync_push_ops.py @@ -388,6 +388,7 @@ 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. @@ -436,6 +437,8 @@ def push_create( 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, @@ -443,7 +446,7 @@ def push_create( description=description, type_=data_app_type, configuration=configuration, - branch_id=branch_id, + branch_id=ds_branch_id, is_disabled=bool(local_data.get("is_disabled", False)), ) else: diff --git a/src/keboola_agent_cli/services/sync_service.py b/src/keboola_agent_cli/services/sync_service.py index eb018371..ed7c9e84 100644 --- a/src/keboola_agent_cli/services/sync_service.py +++ b/src/keboola_agent_cli/services/sync_service.py @@ -1683,6 +1683,13 @@ def push( 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) @@ -1721,6 +1728,7 @@ def push( 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/tests/test_sync_data_app_type.py b/tests/test_sync_data_app_type.py index 456154c0..44f76266 100644 --- a/tests/test_sync_data_app_type.py +++ b/tests/test_sync_data_app_type.py @@ -72,7 +72,9 @@ def create_app( 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}) + 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": []} @@ -245,6 +247,8 @@ def test_push_create_data_app_sends_type(tmp_config_dir: Path, tmp_path: Path) - # 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] @@ -317,8 +321,14 @@ def test_creates_ds_record_with_type(self) -> None: # The caller's writeback keys off result["id"] == the new config ULID. assert result["id"] == "01NEWULID" - def test_create_omits_stale_id_and_update_writes_the_new_one(self) -> None: - """create_app never sees the source app id; update_config writes the new one.""" + 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() @@ -335,7 +345,10 @@ def test_create_omits_stale_id_and_update_writes_the_new_one(self) -> None: ) create_body = ds.create_app.call_args.kwargs["config"] - assert "id" not in create_body["parameters"] + 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" From 3ea2e77047e37a1ac19034dd013ea039da4abb27 Mon Sep 17 00:00:00 2001 From: soustruh Date: Mon, 14 Sep 2026 00:52:34 +0200 Subject: [PATCH 4/4] fix(sync): data-app id writeback, type on pull, orphan cleanup (CLI-8) Three fixes from the #752 review. - Persist the new app's parameters.id to the local file after creating a data app. Without it, the next push (an update) sends the stale source id and reverts the remote back-pointer. - On pull, keep an existing data_app_type when the Data Science lookup fails. A failed lookup used to remove the type from every data-app config. config_hash ignores _keboola, so the diff showed nothing. - Delete the DS record when POST /apps returns an id without a configId. The target then keeps no orphan app, as the docstring promises. --- .../services/_sync_data_app.py | 14 +++++- .../services/_sync_push_ops.py | 8 ++++ .../services/sync_service.py | 21 ++++++--- tests/test_sync_data_app_type.py | 46 ++++++++++++++++++- 4 files changed, 81 insertions(+), 8 deletions(-) diff --git a/src/keboola_agent_cli/services/_sync_data_app.py b/src/keboola_agent_cli/services/_sync_data_app.py index 4828b2c3..460ed7de 100644 --- a/src/keboola_agent_cli/services/_sync_data_app.py +++ b/src/keboola_agent_cli/services/_sync_data_app.py @@ -51,7 +51,9 @@ def create_synced_data_app( 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``. + 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 @@ -86,6 +88,16 @@ def create_synced_data_app( 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, diff --git a/src/keboola_agent_cli/services/_sync_push_ops.py b/src/keboola_agent_cli/services/_sync_push_ops.py index a104d805..6f1dac20 100644 --- a/src/keboola_agent_cli/services/_sync_push_ops.py +++ b/src/keboola_agent_cli/services/_sync_push_ops.py @@ -449,6 +449,14 @@ def push_create( 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, diff --git a/src/keboola_agent_cli/services/sync_service.py b/src/keboola_agent_cli/services/sync_service.py index ed7c9e84..17cc5064 100644 --- a/src/keboola_agent_cli/services/sync_service.py +++ b/src/keboola_agent_cli/services/sync_service.py @@ -587,6 +587,7 @@ def pull( # 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) @@ -598,6 +599,7 @@ def pull( 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 @@ -758,16 +760,23 @@ 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 + # 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=( - data_app_types.get(config_id) - if component_id == DATA_APP_COMPONENT_ID - else None - ), + data_app_type=da_type, ) # Hash of API-converted data. Stored as pull_config_hash so diff --git a/tests/test_sync_data_app_type.py b/tests/test_sync_data_app_type.py index 44f76266..3891311e 100644 --- a/tests/test_sync_data_app_type.py +++ b/tests/test_sync_data_app_type.py @@ -228,6 +228,44 @@ def list_apps(self) -> list[dict[str, Any]]: 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 # =================================================================== @@ -255,6 +293,9 @@ def test_push_create_data_app_sends_type(tmp_config_dir: Path, tmp_path: Path) - 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: @@ -392,7 +433,9 @@ def test_deletes_orphan_when_update_fails(self) -> None: ds.delete_app.assert_called_once_with("77777") - def test_missing_config_id_raises(self) -> None: + 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() @@ -409,3 +452,4 @@ def test_missing_config_id_raises(self) -> None: ) assert exc.value.error_code == ErrorCode.API_ERROR storage.update_config.assert_not_called() + ds.delete_app.assert_called_once_with("77777")