Skip to content

feat(0.22.0): flow CRUD + schedule binding - #200

Closed
ottomansky wants to merge 14 commits into
keboola:release/0.22.0from
ottomansky:fiia/pr6-flow-operations
Closed

feat(0.22.0): flow CRUD + schedule binding#200
ottomansky wants to merge 14 commits into
keboola:release/0.22.0from
ottomansky:fiia/pr6-flow-operations

Conversation

@ottomansky

Copy link
Copy Markdown
Contributor

Summary

  • 8 new kbagent flow subcommands: list, detail, schema, new, update, delete, schedule, schedule-remove
  • Supports both keboola.orchestrator (classic) and keboola.flow (new format) component IDs
  • Schedules stored as keboola.scheduler Storage API configs — no new HTTP client needed
  • Client-side DAG validation (Kahn's algorithm) catches cycles and unknown phase refs before any API write
  • Permissions registered in OPERATION_REGISTRY; hints registered in HintRegistry; kbagent context updated; SKILL.md regenerated; flow-workflow.md reference added

Test plan

  • 34 unit tests in tests/test_flow_service.py (helpers + all 8 service methods)
  • 16 unit tests in tests/test_flow_cli.py (all 8 commands via CliRunner)
  • 3 E2E tests in tests/test_e2e.py::TestE2EFlowOperations against real Keboola project:
    • Full lifecycle: schema → new → list → detail → update → schedule → schedule-remove → delete
    • DAG cycle detection rejects before API call
    • flow list without --project returns all projects
  • ruff check + ruff format --check clean
  • Full unit suite passes (pre-existing Windows 0600 doctor test failure unrelated to this PR)

…date/delete/schedule/schedule-remove)

Eight new subcommands under `kbagent flow`:
- list: all keboola.orchestrator + keboola.flow configs across projects
- detail: full phase/task breakdown with DAG rendering
- schema: YAML template for phases/tasks
- new: create flow with optional phases/tasks from --file; DAG validated before create
- update: partial update (name / description / phases+tasks); DAG validated before write
- delete: hard delete with --yes confirmation guard
- schedule: attach keboola.scheduler cron config; timezone + enabled/disabled
- schedule-remove: remove all schedules for a flow; idempotent

Schedules stored as keboola.scheduler Storage API configs (no new HTTP client).
Client-side DAG validation (Kahn's algorithm) rejects cycles and unknown refs before
any API call. Permissions registered; hints registered; context docs updated; SKILL.md
regenerated; 50 unit tests + 3 E2E tests all pass.
…ct YAML --file

- set_flow_schedule now checks for an existing keboola.scheduler config targeting
  the flow and updates it in-place (status=updated) rather than always creating a
  new one, preventing duplicate schedules on repeated calls
- _load_flow_yaml raises ValueError when YAML parses to a non-dict type (e.g. a
  YAML list), giving a clear error instead of an AttributeError deep in .get()
- Add tests: upsert path, invalid YAML type exit-2
…help + stale docs

- remove_flow_schedule now wraps each delete in try/except: partial success returns
  deleted IDs; all-fail raises SCHEDULE_DELETE_FAILED (consistent with list_flows
  per-project error accumulation pattern)
- flow detail --component-id help text warns about keboola.flow vs keboola.orchestrator
  mismatch so users know to pass --component-id keboola.flow for newer flows
- context.py, flow-workflow.md, hints/definitions/flow.py updated to reflect upsert
  semantics (was stale 'creates one per call' wording)
- Tests: partial-delete-succeeds, all-delete-fails paths
@ottomansky
ottomansky marked this pull request as draft April 22, 2026 14:03
…e/delete/schedule-remove

- flow_update, flow_delete, flow_schedule_remove were missing emit_hint blocks
- hints/definitions/flow.py was missing registrations for flow.update, flow.delete, flow.schedule-remove
- Narrow bare `except Exception` to (ConfigError, KeboolaApiError) in schedule-remove preview
@ottomansky
ottomansky marked this pull request as ready for review April 22, 2026 20:59
padak
padak previously requested changes Apr 22, 2026
@padak
padak dismissed their stale review April 22, 2026 21:41

Replacing with English version for broader team visibility.

@padak padak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes requested — two blockers per CONTRIBUTING.md

The PR is architecturally clean (correctly reuses list_component_configs/update_config — no new HTTP client needed), DAG validation + upsert semantics are thoughtful, test coverage is excellent (27 service + 22 CLI + E2E). Before merge please also:

Required fixes

1. --dry-run on destructive operations (CONTRIBUTING.md line 221: "Destructive operations have --dry-run and --yes flags")

  • src/keboola_agent_cli/commands/flow.py:511-565 (flow delete) — missing --dry-run
  • src/keboola_agent_cli/commands/flow.py:653-731 (flow schedule-remove) — missing --dry-run. For schedule-remove it should print the list of scheduler configs (cron/timezone) without calling delete_config.

Other destructive commands (storage delete-table/bucket/column/file-delete) do have --dry-run — this PR diverges from the convention.

2. Schema mismatch between flow schema and flow-workflow.md

  • commands/flow.py:36-74 (the flow schema output) shows a flat shape with componentId/configId directly on the task object.
  • plugins/kbagent/skills/kbagent/references/flow-workflow.md uses the nested form task: {mode, componentId, configId}.

The API accepts both, but it confuses users. Unify on one — I recommend the nested form (task: {...}), which matches the keboola-as-code convention.

Nice-to-have (follow-up)

  • commands/flow.py:374, 462except (FileNotFoundError, Exception) as exc: Exception subsumes FileNotFoundError, the tuple is misleading. Narrow to (OSError, yaml.YAMLError, ValueError).
  • Add to gotchas.md:
    • The default component_id difference between flow new (keboola.flow) and flow detail/update/delete/schedule (keboola.orchestrator). A user who creates a flow via new and then calls detail without --component-id gets NOT_FOUND.
    • schedule is an upsert (hence no schedule-update).
  • flow schedule-remove --dry-run could ideally print the same list that's shown before the confirm prompt today.

Positives

  • The architectural choice not to add a separate Scheduler HTTP client is correct — schedule is keboola.scheduler config-sugar.
  • _validate_dag (Kahn's topological sort) elegantly handles cycle detection + unknown refs.
  • Fix commits 783eeb8, 04bd64f nicely address upsert semantics (no duplicates on repeated calls).
  • Complete documentation chain: context.py + CLAUDE.md + SKILL.md + commands-reference.md + new flow-workflow.md. Unlike a couple of other PRs 👍

@ottomansky

Copy link
Copy Markdown
Contributor Author

Addressed both blockers + all nice-to-haves:

  • feat(flow): add --dry-run to flow delete7f82f91
  • feat(flow): add --dry-run to flow schedule-remove listing affected configsd22965d--dry-run prints the same list of scheduler configs (cron + timezone) shown before the confirm prompt.
  • refactor(flow): unify schema on nested task shape matching keboola-as-code454da3dflow schema now emits task: {mode, componentId, configId} matching flow-workflow.md.
  • refactor(flow): narrow except tuples at flow.py:374,462eb11545except (FileNotFoundError, Exception)except (OSError, yaml.YAMLError, ValueError). FileNotFoundError stays covered via OSError inheritance.
  • docs(gotchas): document flow component_id default difference and schedule upsert semanticsce6b3b5.

Targeted tests: 62/62 pass (4 new tests added for the dry-run and nested-schema behavior). make lint format-check skill-check version-check changelog-check all green.

@ottomansky
ottomansky requested a review from padak April 23, 2026 10:40
@padak
padak changed the base branch from main to release/0.22.0 April 23, 2026 11:56
padak added a commit that referenced this pull request Apr 23, 2026
New: kbagent flow list/detail/schema/new/update/delete/schedule/schedule-remove
covering both keboola.orchestrator (classic) and keboola.flow (new) component
types. Schedules stored as keboola.scheduler Storage API configs; client-side
Kahn DAG validation rejects cycles and unknown phase refs before API write.

Max's review fixes: --dry-run on delete + schedule-remove, nested task shape
matching keboola-as-code convention, narrow except tuples, gotchas docs.
@padak

padak commented Apr 23, 2026

Copy link
Copy Markdown
Member

Integrated into release/0.22.0 as squash commit bb1ad2c (rebased locally on top of v0.21.2 to resolve merge conflicts with main; all Max's review fixes preserved). Original branch fiia/... unchanged.

Full integration PR coming as release/0.22.0 -> main.

@padak padak closed this Apr 23, 2026
@ottomansky
ottomansky deleted the fiia/pr6-flow-operations branch May 11, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants