feat: AJDA-2445 add command to migrate data-apps orchestrator/flow tasks to data-app-control#101
feat: AJDA-2445 add command to migrate data-apps orchestrator/flow tasks to data-app-control#101sykora-ji wants to merge 4 commits into
Conversation
…sks to data-app-control Adds manage:migrate-data-apps-orchestrator-tasks, which rewrites orchestration and conditional-flow tasks pointing at the legacy keboola.data-apps component so they use keboola.data-app-control instead. Supports a single project or an entire stack via a Manage token, defaults to dry-run, and is idempotent.
There was a problem hiding this comment.
Pull request overview
Adds an operational CLI command to bulk-migrate orchestrator/flow tasks from the legacy keboola.data-apps component to keboola.data-app-control, with a dry-run default and an opt-in force mode to persist changes via Storage API.
Changes:
- Introduces
manage:migrate-data-apps-orchestrator-taskscommand to iterate selected or all projects, mint temporary Storage tokens, and run the migration (dry-run by default;--forceupdates configs). - Adds
DataAppOrchestratorTaskMigratorto scankeboola.orchestratorandkeboola.flowconfigurations, rewrite eligible tasks, and summarize migrated vs skipped tasks. - Registers the command in
cli.phpand documents usage/behavior inREADME.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Keboola/Console/Command/MigrateDataAppsOrchestratorTasks.php | New Symfony Console command wiring Manage API + Storage API access, project selection, dry-run/force execution, and summary output. |
| src/Keboola/Console/Command/DataAppOrchestratorTaskMigrator.php | Implements the config/task scanning and rewrite logic for orchestrator/flow configurations. |
| README.md | Documents the new command under “Project manipulation”, including args/options and behavior. |
| cli.php | Registers the new command in the CLI application bootstrap. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Validate the projects argument strictly instead of silently dropping invalid IDs, catch Storage API errors when resolving a configId reference so one bad reference doesn't abort the whole project, cache resolved configIds to avoid repeated lookups, and fix the "Checked N projects" summary to include disabled/errored projects.
MiroCillik
left a comment
There was a problem hiding this comment.
Review summary
Solid, careful PR — dry-run by default, idempotent re-runs, short-lived scoped tokens, and per-project error isolation. I found no blocking correctness bugs. I confirmed via the Storage API SDK that listComponentConfigurations returns the full configuration payload by default, so the scan reads real task data.
Main non-blocking suggestions (see inline comments):
- Split the "couldn't resolve app id" case out of the generic
tasksSkippedUnsupportedcounter — for an ops tool, an unresolvable/inaccessible referenced config needs human follow-up, whereas create/delete tasks are deliberately skipped; lumping them can hide a real problem. - Consider a confirmation prompt for the
--force allcombination (stack-wide config mutation with no prompt; sibling commands useConfirmationQuestion). Dry-run default mitigates this. - The full replacement of
configDatadeliberately drops any other task-level params — worth an explicit comment since it's the highest-impact behavioral assumption.
Nits: no unit tests for the very testable pure helpers (extractAppId/resolveAppId) — consistent with the repo having none, but this rewrites stored customer configs, so a couple of tests would be cheap insurance.
Also: the PR body still has an unresolved [TODO: confirm whether Support needs to be notified...] in the Post-Release Support Plan — worth resolving before merge.
Nothing here blocks merge. 👍
Distinguish deliberately-skipped tasks (unsupported keboola.data-apps task variants like create/delete/terminate) from tasks that should have migrated but whose app id couldn't be resolved (missing/inaccessible configId reference, malformed parameters) - the latter is now a separate counter/message so it can't hide behind "unsupported shape" and needs manual follow-up. Add an interactive confirmation guard for the "all" + "--force" combination as a safety net against an accidental stack-wide run. Document that the configData replacement on migrated tasks is deliberate, not an oversight.
Adds phpunit/phpunit as a dev dependency and a small test suite covering DataAppOrchestratorTaskMigrator (inline configData migration, unsupported vs. unresolvable skip reasons, configId caching, phases/other task fields preserved, scans both keboola.orchestrator and keboola.flow, idempotency) and MigrateDataAppsOrchestratorTasks::parseProjectIds validation. Wires the suite into CI. This command rewrites stored project configurations, so the extra coverage is worth the one-off cost of introducing a test harness to a repo that previously had none.
|
Addressed the nits from the review summary:
🤖 Addressed by Claude Code |
MiroCillik
left a comment
There was a problem hiding this comment.
OK, muze se mergnout, ja to pak jeste nekde vyzkousim
Link to issue
Description
Adds a new
manage:migrate-data-apps-orchestrator-taskscommand to cli-utils that rewrites orchestration/flow tasks referencing the legacykeboola.data-appscomponent so they usekeboola.data-app-controlinstead.DataAppOrchestratorTaskMigrator(new): given a Storage APIComponentsclient, lists all configurations of bothkeboola.orchestrator(legacy orchestrations) andkeboola.flow(conditional flows — verified on a live stack to be a distinct component, not a variant ofkeboola.orchestrator), scans each configuration'stasks[]for tasks pointing atkeboola.data-apps, resolves the app ID from either inlineconfigDataor a referencedconfigId, and rewrites the task'scomponentIdtokeboola.data-app-controlwithparameters.appIdset — while preserving every other field on the task object (type,mode,delay,retry,variableOverrides) unchanged, since thekeboola.flowschema requirestypealongsidecomponentId/mode. Tasks whosekeboola.data-appsparameters carry an unsupportedtaskvalue (anything other than the implicit/app-start/startvariants used to launch an app) are left untouched and reported separately, so unrelated task types (create/delete/terminate/restore/...) are never silently mistransformed. Already-migrated tasks are skipped, making re-runs idempotent.MigrateDataAppsOrchestratorTasks(new command): accepts a Manage API token, stack URL, and either a comma-separated list of project IDs orall. For each target project it mints a short-lived, scoped Storage API token viacreateProjectStorageToken()and runs the migrator against it; failures on one project are caught and logged without aborting the run. Defaults to dry-run (only reports what would change);--force/-fperforms the actualupdateConfigurationcalls with achangeDescription. Prints a summary of projects/configurations/tasks scanned, migrated, and skipped.cli.phpand documented inREADME.mdunder "Project manipulation".Manually verified end-to-end against a test flow on the canary-orion stack (dry-run, force run, idempotent re-run, and schema validity after the rewrite) before opening this PR.
Release Notes
keboola.data-appscomponent to the newerkeboola.data-app-controlcomponent. Orchestrations and conditional flows created before this change still reference the legacy component, so a one-off bulk-migration tool is needed to move them over before the legacy component/UI path can be retired.keboola.data-apps; everything else is left untouched. Tasks with an unrecognizedkeboola.data-appsparameter shape are skipped rather than guessed at, to avoid silently breaking a flow.keboola.data-appstask.cli-utilsDocker image via the existing CI pipeline (tag push). The command itself is then run manually by an engineer, per project or per stack, as needed; it is not invoked automatically.changeDescriptionset by this command), so any specific config can be rolled back to a prior version via Storage API if needed.--forceagainst production projects/stacks, they will run a dry-run first to produce a report of every affected project/configuration, and track that list in the AJDA-2445 Linear issue so the rollout itself is auditable.