From eb3aee38d2be4aecbde0f78fcdd6ab535b328be2 Mon Sep 17 00:00:00 2001 From: Matthew Moorcroft Date: Thu, 23 Jul 2026 16:50:59 +0100 Subject: [PATCH 1/9] Fix cross-bundle run_job_task refs pointing at non-existent nodes ExecutePipeline emits run_job_task.job_id = ${resources.jobs.X.id}, which only resolves when X is a job in this bundle. In a multi-pipeline migration each ADF pipeline becomes its own bundle, so a ref to a sibling pipeline points at a node that does not exist here and `bundle deploy` fails with "no such node resources.jobs.X". Add _rewrite_cross_bundle_run_job_refs: before databricks.yml/resource YAML are written, rewrite run_job_task refs to out-of-bundle jobs into ${var.X} and register X in _cross_bundle_variables (which the existing YAML builder declares). Operator supplies the numeric job id at deploy via --var, as SETUP.md documents. Recurses into for_each_task bodies. This is the stopgap that #10 (ordered cross-pipeline deploy from control lineage) builds on and keeps as the fallback for unresolved callees. Closes #23 Co-authored-by: Isaac --- src/flowx/bundler/dab_writer.py | 56 +++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/flowx/bundler/dab_writer.py b/src/flowx/bundler/dab_writer.py index 436900d..1291415 100644 --- a/src/flowx/bundler/dab_writer.py +++ b/src/flowx/bundler/dab_writer.py @@ -131,6 +131,17 @@ def write_bundle( _any_task_uses_classic_cluster(inner.tasks) for inner in workflow.inner_workflows ) + # Rewrite run_job_task refs to sibling-bundle jobs (${resources.jobs.X.id} where X is not a job in + # this bundle) into ${var.X}, registering each so _build_databricks_yml declares the variable. Must + # run before databricks.yml and the resource YAML are written below, or the ref points at a + # non-existent resource node and `bundle deploy` fails ("no such node resources.jobs.X"). + _known_bundle_jobs_for_rewrite = {normalize_task_key(workflow.name)} | { + normalize_task_key(inner.name) for inner in workflow.inner_workflows + } + _rewrite_cross_bundle_run_job_refs(workflow.tasks, _known_bundle_jobs_for_rewrite, _cross_bundle_variables) + for inner in workflow.inner_workflows: + _rewrite_cross_bundle_run_job_refs(inner.tasks, _known_bundle_jobs_for_rewrite, _cross_bundle_variables) + pipeline_resources = _collect_pipeline_resources(workflow) pipeline_variable_declarations = _build_pipeline_variable_declarations(pipeline_resources, catalog, schema) # sql_task references ${var.warehouse_id}; declare it (no default -> user supplies at deploy). @@ -1543,6 +1554,51 @@ def visit(task: dict[str, Any]) -> None: return neutralized +_CROSS_BUNDLE_JOB_ID_REF = re.compile(r"\$\{resources\.jobs\.([^.]+)\.id\}") + + +def _rewrite_cross_bundle_run_job_refs( + tasks: list[dict[str, Any]], + known_bundle_jobs: set[str], + cross_bundle_variables: dict[str, str], +) -> int: + """Rewrites ``run_job_task`` refs to jobs outside this bundle into ``${var.X}``. + + An ExecutePipeline activity is emitted as ``run_job_task.job_id = + ${resources.jobs.X.id}`` (see ``execute_pipeline.prepare``). That resolves + only when job ``X`` is a resource in *this* bundle. In a multi-pipeline + migration each ADF pipeline becomes its **own** bundle, so a reference to a + sibling pipeline points at a resource node that does not exist here and + ``bundle deploy`` fails with ``no such node "resources.jobs.X"``. + + For every ``run_job_task.job_id`` whose target is not in *known_bundle_jobs*, + rewrite it to ``${var.X}`` and register ``X`` in *cross_bundle_variables* so + the ``databricks.yml`` builder declares a matching bundle variable (the user + supplies the numeric job id at deploy time, per SETUP.md). Recurses into + ``for_each_task.task`` bodies. Returns the number of refs rewritten. + """ + rewritten = 0 + + def visit(task: dict[str, Any]) -> None: + nonlocal rewritten + run_job = task.get("run_job_task") + if isinstance(run_job, dict): + match = _CROSS_BUNDLE_JOB_ID_REF.fullmatch(str(run_job.get("job_id", ""))) + if match: + target = match.group(1) + if target not in known_bundle_jobs: + run_job["job_id"] = f"${{var.{target}}}" + cross_bundle_variables[target] = target + rewritten += 1 + for_each = task.get("for_each_task") + if for_each and isinstance(for_each.get("task"), dict): + visit(for_each["task"]) + + for task in tasks: + visit(task) + return rewritten + + def _collect_all_task_keys(tasks: list[dict[str, Any]]) -> set[str]: """Collects every task_key reachable from the job's top-level task list.""" keys: set[str] = set() From 05ce0021c20df82d98891c708ae88e7d936df254 Mon Sep 17 00:00:00 2001 From: Matthew Moorcroft Date: Fri, 24 Jul 2026 10:49:01 +0100 Subject: [PATCH 2/9] Drop unused return from _rewrite_cross_bundle_run_job_refs The rewrite count was returned but discarded at both call sites. The function's real output is its in-place mutation of cross_bundle_variables (declared in databricks.yml and surfaced in SETUP.md), so return None and remove the dead counter. Co-authored-by: Isaac --- src/flowx/bundler/dab_writer.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/flowx/bundler/dab_writer.py b/src/flowx/bundler/dab_writer.py index 1291415..8c8acb2 100644 --- a/src/flowx/bundler/dab_writer.py +++ b/src/flowx/bundler/dab_writer.py @@ -1561,7 +1561,7 @@ def _rewrite_cross_bundle_run_job_refs( tasks: list[dict[str, Any]], known_bundle_jobs: set[str], cross_bundle_variables: dict[str, str], -) -> int: +) -> None: """Rewrites ``run_job_task`` refs to jobs outside this bundle into ``${var.X}``. An ExecutePipeline activity is emitted as ``run_job_task.job_id = @@ -1575,12 +1575,12 @@ def _rewrite_cross_bundle_run_job_refs( rewrite it to ``${var.X}`` and register ``X`` in *cross_bundle_variables* so the ``databricks.yml`` builder declares a matching bundle variable (the user supplies the numeric job id at deploy time, per SETUP.md). Recurses into - ``for_each_task.task`` bodies. Returns the number of refs rewritten. + ``for_each_task.task`` bodies. The rewritten refs are surfaced to the + operator via *cross_bundle_variables* (declared in ``databricks.yml`` and + listed in SETUP.md), so this mutates in place and returns nothing. """ - rewritten = 0 def visit(task: dict[str, Any]) -> None: - nonlocal rewritten run_job = task.get("run_job_task") if isinstance(run_job, dict): match = _CROSS_BUNDLE_JOB_ID_REF.fullmatch(str(run_job.get("job_id", ""))) @@ -1589,14 +1589,12 @@ def visit(task: dict[str, Any]) -> None: if target not in known_bundle_jobs: run_job["job_id"] = f"${{var.{target}}}" cross_bundle_variables[target] = target - rewritten += 1 for_each = task.get("for_each_task") if for_each and isinstance(for_each.get("task"), dict): visit(for_each["task"]) for task in tasks: visit(task) - return rewritten def _collect_all_task_keys(tasks: list[dict[str, Any]]) -> set[str]: From 1b22101433dc96d156a14f5fb72e28b858c97fdc Mon Sep 17 00:00:00 2001 From: Zanita Rahimi Date: Wed, 5 Aug 2026 15:09:50 +0200 Subject: [PATCH 3/9] initial commit, introduce flowx deploy skill --- .claude-plugin/plugin.json | 1 + AGENTS.md | 10 +- skills/flowx-deploy/SKILL.md | 89 +++ skills/flowx-package/SKILL.md | 31 +- src/flowx/adapter/__main__.py | 52 +- src/flowx/adapter/constants.py | 7 + src/flowx/adapter/session.py | 27 + src/flowx/bundler/dab_writer.py | 879 +++++++++++++++++++++------- src/flowx/bundler/deploy_writer.py | 159 +++++ src/flowx/bundler/deployer.py | 394 +++++++++++++ src/flowx/bundler/pipeline_graph.py | 175 ++++++ src/flowx/bundler/prereqs_writer.py | 25 + src/flowx/sources/adf/loader.py | 2 + tests/unit/test_adapter.py | 24 + tests/unit/test_deploy_writer.py | 61 ++ tests/unit/test_deployer.py | 235 ++++++++ tests/unit/test_packaging_modes.py | 447 ++++++++++++++ tests/unit/test_pipeline_graph.py | 76 +++ 18 files changed, 2483 insertions(+), 211 deletions(-) create mode 100644 skills/flowx-deploy/SKILL.md create mode 100644 src/flowx/bundler/deploy_writer.py create mode 100644 src/flowx/bundler/deployer.py create mode 100644 src/flowx/bundler/pipeline_graph.py create mode 100644 tests/unit/test_deploy_writer.py create mode 100644 tests/unit/test_deployer.py create mode 100644 tests/unit/test_packaging_modes.py create mode 100644 tests/unit/test_pipeline_graph.py diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index e41c463..204d6a9 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -13,6 +13,7 @@ "./skills/flowx-discover", "./skills/flowx-convert", "./skills/flowx-package", + "./skills/flowx-deploy", "./skills/flowx-migrate" ] } diff --git a/AGENTS.md b/AGENTS.md index e1ae8c4..7bbddfb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,7 +58,10 @@ intermediates under `.work/` (pruned by `package`). 1. **Discover** -- Parse ADF JSON from UC volumes -> typed AST -> `metadata/inventory.json` + `metadata/profile_report.csv` + verbatim `metadata/.arm.json` 2. **Convert** -- Registry dispatch + topological sort -> Pipeline IR (deterministic + agentic gaps); transient report at `.work/translation_report.json` -3. **Package** -- IR -> DAB YAML + generated notebooks + setup scripts; prunes `.work/` +3. **Package** -- IR -> DAB YAML + generated notebooks + setup scripts; prunes `.work/`. The + `--packaging-mode` flag (`per-pipeline` default / `single` / `per-group`) controls how a + multi-pipeline factory is laid out into bundles; a top-level `DEPLOY.md` records the suggested + callees-first deploy order for every mode. ### Key Patterns - `@dataclass(slots=True, kw_only=True)` for all models @@ -80,7 +83,10 @@ intermediates under `.work/` (pruned by `package`). | `preparer/workflow_preparer.py` | Orchestrates activity preparers | | `preparer/code_generator.py` | Notebook code generation for activity types | | `preparer/activity_preparers/` | One module per activity type | -| `bundler/dab_writer.py` | Generates databricks.yml, job YAML, resources | +| `bundler/dab_writer.py` | Generates databricks.yml, job YAML, resources; groups pipelines into bundles per `--packaging-mode` | +| `bundler/pipeline_graph.py` | Run Pipeline (ExecutePipeline) dependency graph: grouping (connected components) + deploy order (topo sort) | +| `bundler/deploy_writer.py` | Renders the top-level `DEPLOY.md` (bundle layout + suggested deploy order) | +| `bundler/deployer.py` | Ordered multi-bundle deploy: discovers bundles, deploys callees first, wires cross-bundle job ids | | `bundler/notebook_writer.py` | Writes generated notebooks to bundle | | `bundler/setup_generator.py` | Setup scripts for UC volumes, secrets, connections | | `reporting/coverage.py` | Builds per-pipeline coverage rows from `metadata/` | diff --git a/skills/flowx-deploy/SKILL.md b/skills/flowx-deploy/SKILL.md new file mode 100644 index 0000000..45fe8f5 --- /dev/null +++ b/skills/flowx-deploy/SKILL.md @@ -0,0 +1,89 @@ +--- +name: flowx-deploy +description: > + Deploy the per-pipeline Databricks Asset Bundles from a multi-pipeline flowx + migration in dependency order, resolving cross-bundle job ids automatically. + Local CLI only. +triggers: + - "deploy bundles" + - "deploy in dependency order" + - "ordered deploy" + - "deploy flowx bundles" + - "deploy multi pipeline migration" +--- + +# Deploy per-pipeline flowx bundles in dependency order + +Deploy every bundle produced by a multi-pipeline migration, in the right order, wiring cross-bundle +`ExecutePipeline` references automatically. + +## Context + +flowx emits **one bundle per ADF pipeline** under the output directory (`//`). +When pipeline A calls pipeline B via `ExecutePipeline`, the generated `run_job_task` in A's bundle +references B — a job that lives in B's *own* bundle. flowx rewrites that out-of-bundle reference to +`${var.}` and declares a matching bundle variable, so each bundle is deploy-valid on its own; but +the operator otherwise has to find B's numeric job id and pass it to A by hand. + +The package phase writes a top-level `DEPLOY.md` describing the bundle layout, cross-bundle +dependencies, and the suggested callees-first deploy order. This skill is the **automated** form of +those instructions — read `DEPLOY.md` for the human-readable version. + +This skill automates that: + +1. Discovers the bundles under the output directory (any immediate subdirectory with a + `databricks.yml`) — no manifest needed. +2. Reads each bundle's job resource keys and its `${var.}` cross-bundle dependencies straight + from the generated `resources/*.yml`. +3. Topologically sorts them (callees first) — a cyclic call graph is rejected with a clear error. +4. Deploys each bundle with `databricks bundle deploy`. +5. After each deploy, reads the deployed job id from `databricks bundle summary -o json` and injects + it into callers via `--var "="`. + +Because it captures and injects the **numeric job id** (not a name), dev-mode `[dev ]` job-name +prefixes are irrelevant — it works identically for `dev` and `prod` targets. + +## Prerequisites + +- An output directory with the per-pipeline bundle subdirectories (from a multi-pipeline migration). +- A working local `databricks` CLI with a configured profile / auth for the target workspace. + +> **Not available on Databricks serverless / Genie Code.** `databricks bundle deploy` and +> `bundle summary` do not run on serverless compute, so this is a **local venv-CLI** (or web-terminal) +> step only. + +## How to run + +Use the venv interpreter from the marker file (`/.migration-venv`) with `src/` on +`PYTHONPATH`: + +```bash +export PYTHONPATH="/src" +PY="$(cat /.migration-venv)" + +# 1. Preview the deploy order and per-bundle commands without deploying: +"$PY" -m flowx.adapter deploy --output-dir --target dev --dry-run + +# 2. Deploy for real: +"$PY" -m flowx.adapter deploy --output-dir --target dev [--profile ] +``` + +Flags: + +- `--output-dir` — directory holding the per-pipeline bundle subdirectories (default `./flowx_output`). +- `--target` — bundle target to deploy (default `dev`). +- `--profile` — Databricks CLI profile used for both `deploy` and `summary`. +- `--dry-run` — print the dependency order and each `databricks bundle deploy …` command (with + `=` placeholders), without deploying. +- `--allow-missing-deps` — continue when a bundle references a callee that isn't present under the + output dir; that dependency's `--var` is skipped and must be set manually (see the bundle's + `SETUP.md`). Without this flag, a missing dependency is a hard error. + +## Behavior and failure handling + +- **Ordering:** callees always deploy before their callers. The order is deterministic. +- **Deploy failure:** if any bundle's `databricks bundle deploy` fails, deployment stops immediately; + dependents are not deployed. The failing bundle and its stderr are printed. +- **Job-id capture:** ids are read from `bundle summary -o json` at `.resources.jobs..id`. A + resource without a deployed job id (e.g. a Lakeflow pipeline resource) is skipped — no empty `--var`. +- **Cycles:** a cyclic call graph cannot be ordered; the command errors out. diff --git a/skills/flowx-package/SKILL.md b/skills/flowx-package/SKILL.md index fda6dc7..fce6547 100644 --- a/skills/flowx-package/SKILL.md +++ b/skills/flowx-package/SKILL.md @@ -99,8 +99,26 @@ Ask the user for the following (provide defaults): | Bundle name | Name for the DABs project | derived from first pipeline name | | Target environments | Deployment targets to configure | `dev, staging, prod` | | Warehouse ID | SQL warehouse for SQL tasks (optional) | prompt if SQL tasks exist | +| Packaging mode | How to lay out bundles for a multi-pipeline factory (`--packaging-mode`): `per-pipeline`, `single`, or `per-group`. See below. | `per-pipeline` | | Databricks CLI profile | Profile used to download workspace-resident notebooks / JARs / Python files (`--profile`). Required only when the bundle references absolute workspace paths. | resolved from `~/.databrickscfg` (auto-prompt if multiple) | +**Packaging mode** (`--packaging-mode`, surfaced as the `packaging_mode` input) controls how a +multi-pipeline migration is laid out. For a single-pipeline migration every mode is equivalent. + +- **`per-pipeline`** (default) — one Databricks Asset Bundle per ADF pipeline, each in its own + `//` subdirectory. Cross-pipeline `ExecutePipeline` calls become + `${var.}` job-id references wired at deploy time. +- **`single`** — every pipeline in one bundle at the output root. Intra-bundle `ExecutePipeline` + calls resolve directly via `${resources.jobs..id}` (no deploy-time wiring needed). +- **`per-group`** — pipelines grouped into bundles. By default (`--group-by inferred`) groups are + the connected components of the Run Pipeline call graph, so pipelines that call one another ship + together. Pass `--group-by spec --group-spec ` to use an explicit JSON/YAML mapping + (`{pipeline: group}` or `{group: [pipelines]}`); the `group_spec` input captures that path. + +Regardless of mode, a single top-level `DEPLOY.md` is written describing every bundle, its +cross-bundle dependencies, and a suggested callees-first deploy order. Use the `flowx-deploy` skill +(`python -m flowx.adapter deploy`) to deploy the bundles in that order automatically. + ### Step 2.5 — Detect workspace artifacts and authenticate > **Databricks runtime (serverless / cluster):** Authentication is auto-configured @@ -168,6 +186,8 @@ Execute the DAB writer: --catalog \ --schema \ --bundle-name \ + [--packaging-mode per-pipeline|single|per-group] \ + [--group-by inferred|spec] [--group-spec ] \ [--profile ] \ [--no-download-workspace-files] \ [--keep-intermediates] @@ -221,6 +241,7 @@ Show the user what was generated: create_secrets.py register_connections.py SETUP.md + DEPLOY.md # bundle layout + suggested deploy order (top level) metadata/ # kept migration metadata (from discover + modify) inventory.json profile_report.csv @@ -245,7 +266,7 @@ Emphasize that the user should review these scripts before running them, especia Briefly describe: - **databricks.yml** — The root bundle config with workspace, target environments (dev/staging/prod), and variable definitions. Variables are parameterized for environment-specific values (catalog, schema, warehouse). -- **resources/*.yml** — One YAML file per Databricks Lakeflow Job (one per ADF pipeline). Each job contains tasks mapped from ADF activities, with dependencies matching the original ADF dependency chains. +- **resources/*.yml** — One YAML file per Databricks Lakeflow Job. In `per-pipeline` mode each bundle holds one pipeline's job (plus any inner ForEach jobs); in `single`/`per-group` mode a bundle holds several pipelines' jobs side by side. Each job contains tasks mapped from ADF activities, with dependencies matching the original ADF dependency chains. - **src/notebooks/*.py** — Python notebooks for activities that translate to notebook_task. These contain the actual data movement or transformation logic. - **tests/*.py** — Skeleton test files for validating the migrated jobs. @@ -279,6 +300,11 @@ Next Steps Recommend running `databricks bundle validate` first to catch any configuration issues before deployment. +When the migration produced **multiple bundles** (`per-pipeline` or `per-group` mode), point the +user at the top-level `DEPLOY.md` for the suggested callees-first deploy order, and recommend the +`flowx-deploy` skill (`python -m flowx.adapter deploy --output-dir `) to deploy them in +order and wire cross-bundle job ids automatically. + ### Step 8 — (Optional) Persist coverage results and install a dashboard This step only applies when running with workspace auth (Genie Code, or a configured @@ -336,7 +362,8 @@ All under the shared ``: | `resources/*.yml` | Job and pipeline YAML definitions | | `src/notebooks/*.py` | Generated notebooks | | `src/setup/*.py` | Infrastructure setup scripts | -| `SETUP.md` | Human-readable setup instructions | +| `SETUP.md` | Human-readable setup instructions (one per bundle) | +| `DEPLOY.md` | Top-level bundle layout + suggested deploy order (all packaging modes) | | `metadata/inventory.json` | Activity inventory (from discover) | | `metadata/profile_report.csv` | Per-pipeline complexity report (from profile) | | `metadata/.arm.json` | Verbatim original ADF/ARM source (from discover) | diff --git a/src/flowx/adapter/__main__.py b/src/flowx/adapter/__main__.py index 38545be..c4b7d06 100644 --- a/src/flowx/adapter/__main__.py +++ b/src/flowx/adapter/__main__.py @@ -2,8 +2,8 @@ Exposes stateless subcommands -- the ``discover``/``convert``/``package`` phase runners plus ``inspect``, ``modify``, ``resolve-agentic``, ``inputs``, ``materialize-lookup``, ``workspace-paths``, -``record-results``, and ``install-dashboard`` -- so each agent turn runs as an independent process -holding no session state across user prompts. +``record-results``, ``install-dashboard``, and ``deploy`` -- so each agent turn runs as an independent +process holding no session state across user prompts. """ from __future__ import annotations @@ -89,6 +89,8 @@ def main(argv: list[str] | None = None) -> int: return _run_record_results(args) if args.command == "install-dashboard": return _run_install_dashboard(args) + if args.command == "deploy": + return _run_deploy(args) parser.print_help(sys.stderr) return 2 @@ -140,6 +142,23 @@ def _run_resolve_agentic(args: argparse.Namespace) -> int: return 0 +def _run_deploy(args: argparse.Namespace) -> int: + """Implements ``deploy``: deploy per-pipeline bundles in dependency order. + + Local-CLI only — shells out to ``databricks bundle deploy`` / ``summary``, which are not + available on Databricks serverless / Genie Code. Returns the deployer's exit code. + """ + from flowx.bundler.deployer import run as run_deploy + + return run_deploy( + args.output_dir, + target=args.target, + profile=args.profile, + dry_run=args.dry_run, + allow_missing_deps=args.allow_missing_deps, + ) + + def _run_record_results(args: argparse.Namespace) -> int: """Implements ``record-results``: write per-pipeline coverage to a UC table. @@ -527,6 +546,35 @@ def _build_parser() -> argparse.ArgumentParser: help="Workspace folder for the dashboard (defaults to the current user's home).", ) + deploy = subparsers.add_parser( + "deploy", + help="Deploy per-pipeline bundles in dependency order, wiring cross-bundle job ids (local CLI).", + ) + deploy.add_argument( + "--output-dir", + type=Path, + default=Path("./flowx_output"), + help="Directory holding the per-pipeline bundle subdirectories.", + ) + deploy.add_argument("--target", type=str, default="dev", help="Bundle target to deploy (default: dev).") + deploy.add_argument("--profile", type=str, default=None, help="Databricks CLI profile for deploy and summary.") + deploy.add_argument( + "--dry-run", + action="store_true", + help="Print the dependency order and deploy commands without deploying.", + ) + deploy.add_argument( + "--allow-missing-deps", + action="store_true", + help=( + "Order and attempt to deploy even when a bundle references a callee absent from the output " + "dir. The missing ${var.} is declared without a default, so that bundle's deploy " + "still fails until you supply the value manually (edit its databricks.yml default or " + "`databricks bundle deploy --var =` per SETUP.md); this flag only unblocks " + "the ordering, not the deploy." + ), + ) + # Unified phase runners: `adapter --source -- ` routes discover/convert # to the named source's phase module. --source is required for those phases (no default); # package is source-independent. --source-path (and each source's own alias, e.g. diff --git a/src/flowx/adapter/constants.py b/src/flowx/adapter/constants.py index 00eaead..ad87c74 100644 --- a/src/flowx/adapter/constants.py +++ b/src/flowx/adapter/constants.py @@ -42,6 +42,13 @@ INPUT_RESULTS_TABLE: Final[str] = "results_table" INPUT_RESULTS_WAREHOUSE: Final[str] = "results_warehouse_id" INPUT_INSTALL_DASHBOARD: Final[str] = "install_dashboard" +INPUT_PACKAGING_MODE: Final[str] = "packaging_mode" +INPUT_GROUP_SPEC: Final[str] = "group_spec" + +# Packaging-mode answers accepted by the package phase's --packaging-mode flag. +PACKAGING_MODE_PER_PIPELINE: Final[str] = "per-pipeline" +PACKAGING_MODE_SINGLE: Final[str] = "single" +PACKAGING_MODE_PER_GROUP: Final[str] = "per-group" LAKEFLOW_CONNECTOR_TYPE_QUERY_BASED: Final[str] = "query_based" LAKEFLOW_CONNECTOR_TYPE_CDC: Final[str] = "cdc" diff --git a/src/flowx/adapter/session.py b/src/flowx/adapter/session.py index 0990e00..c7c3912 100644 --- a/src/flowx/adapter/session.py +++ b/src/flowx/adapter/session.py @@ -21,10 +21,12 @@ INPUT_CATALOG, INPUT_DATABRICKS_PROFILE, INPUT_GLOBAL_PARAMETER_RESOLUTION, + INPUT_GROUP_SPEC, INPUT_INSTALL_DASHBOARD, INPUT_INVENTORY_PATH, INPUT_OUTPUT_BUNDLE_PATH, INPUT_OUTPUT_DIR, + INPUT_PACKAGING_MODE, INPUT_RESULTS_TABLE, INPUT_RESULTS_WAREHOUSE, INPUT_SCHEMA, @@ -373,6 +375,31 @@ def _convert_options(source: str) -> tuple[MigrationInputOption, ...]: default="", required=False, ), + MigrationInputOption( + option_id=INPUT_PACKAGING_MODE, + prompt="How should pipelines be packaged into bundles?", + description=( + "One of ``per-pipeline`` (default — one Databricks Asset Bundle per ADF pipeline), " + "``single`` (all pipelines in one bundle), or ``per-group`` (group pipelines into " + "bundles by their Run Pipeline call graph, or by an explicit --group-spec). Forwarded " + "to ``package`` as ``--packaging-mode``. For a single-pipeline migration every mode is " + "equivalent." + ), + default="per-pipeline", + required=False, + ), + MigrationInputOption( + option_id=INPUT_GROUP_SPEC, + prompt="Path to a pipeline->group spec (only for --packaging-mode per-group with explicit groups)?", + description=( + "Optional. JSON/YAML file mapping pipelines to group names (``{pipeline: group}`` or " + "``{group: [pipelines]}``). When set, package is run with ``--packaging-mode per-group " + "--group-by spec --group-spec ``. Leave blank to infer groups from the Run " + "Pipeline call graph." + ), + default="", + required=False, + ), MigrationInputOption( option_id=INPUT_DATABRICKS_PROFILE, prompt="Databricks CLI profile?", diff --git a/src/flowx/bundler/dab_writer.py b/src/flowx/bundler/dab_writer.py index 8c8acb2..cdd539d 100644 --- a/src/flowx/bundler/dab_writer.py +++ b/src/flowx/bundler/dab_writer.py @@ -24,9 +24,10 @@ ) from flowx.bundler.inner_job_params import normalize_value from flowx.bundler.notebook_writer import write_notebooks +from flowx.bundler.pipeline_graph import CROSS_BUNDLE_JOB_ID_REF as _CROSS_BUNDLE_JOB_ID_REF from flowx.bundler.prereqs_writer import ManualParameter, build_prereqs, render_setup_md from flowx.bundler.setup_generator import generate_setup_tasks -from flowx.models.dab import DabNotebook +from flowx.models.dab import DabNotebook, SecretInstruction, SetupTask from flowx.models.ir import ( Activity, AppendVariableActivity, @@ -63,6 +64,16 @@ from flowx.utils import normalize_task_key +class MalformedReportError(Exception): + """A translation report contained an entry flowx could not have produced. + + Reports are machine-generated by ``engine.py`` (``_pipeline_to_dict`` always + emits ``name`` + ``tasks``), so a non-conforming entry signals corruption or + an internal bug. Raising -- rather than silently dropping the entry -- keeps + ``package`` from emitting a bundle that is quietly missing a pipeline. + """ + + class _BundleYamlDumper(yaml.SafeDumper): """YAML dumper that leaves keys unquoted and only quotes values when needed.""" @@ -79,6 +90,11 @@ class _BundleYamlDumper(yaml.SafeDumper): # neutralised (always-true) branch predicate is never silent. _neutralized_conditions: list[dict[str, str]] = [] +# Job parameters that had no ADF default and so were emitted with a synthetic ``default: ""`` to satisfy +# the DAB schema. Reset per write_bundle_group call and surfaced in SETUP.md so an operator knows the +# value was caller-supplied in ADF and a run that omits it receives "" rather than failing fast. +_synthetic_default_parameters: list[dict[str, str]] = [] + _WIDGET_REFERENCE = re.compile(r"""dbutils\.widgets\.get\(\s*["']([^"']+)["']\s*\)""") _JOB_RESOURCE_ID_REFERENCE = re.compile(r"\$\{resources\.jobs\.([^.}]+)\.id\}") @@ -91,7 +107,11 @@ def write_bundle( bundle_name: str | None = None, skipped_pipelines: list[str] | None = None, ) -> list[Path]: - """Writes all DAB files to output_dir. + """Writes all DAB files for a single workflow to output_dir. + + Thin wrapper over :func:`write_bundle_group` for the one-pipeline-per-bundle case (the + ``per-pipeline`` packaging mode and every existing caller). Kept as the stable single-workflow + entry point; the group writer is byte-for-byte identical for a one-element group. Args: workflow: The PreparedWorkflow to serialize. @@ -105,47 +125,217 @@ def write_bundle( Returns: List of absolute paths to all created files. """ - # Reset module-level accumulators so successive write_bundle calls (CLI loops, tests) don't carry - # warnings or cross-bundle variables from one bundle into the next. + return write_bundle_group( + [workflow], + output_dir, + catalog=catalog, + schema=schema, + bundle_name=bundle_name, + skipped_pipelines=skipped_pipelines, + ) + + +def _dedupe_notebooks(notebooks: list[DabNotebook]) -> list[DabNotebook]: + """Returns *notebooks* with duplicate ``relative_path`` entries collapsed (first wins). + + Guards against the same notebook appearing twice in one bundle's writelist — e.g. a workflow and + its inner ForEach job sharing a notebook object, or setup notebooks the generator emits once per + workflow (``create_secrets.py``). It does NOT dedupe two pipelines' same-named generated + notebooks in a multi-pipeline bundle: :func:`_namespace_bundle_artifacts` runs first and gives + each pipeline's notebooks a per-pipeline path prefix, so those never share a ``relative_path`` + (that separation is deliberate — the two files can carry different, pipeline-specific content). + """ + seen: set[str] = set() + unique: list[DabNotebook] = [] + for notebook in notebooks: + if notebook.relative_path in seen: + continue + seen.add(notebook.relative_path) + unique.append(notebook) + return unique + + +def _rewrite_task_string_values(tasks: list[dict[str, Any]], replacements: dict[str, str]) -> None: + """Replaces exact string values matching *replacements* anywhere in the task tree, in place. + + Walks every task dict (descending into ``for_each_task.task`` bodies and nested dicts/lists) and + swaps any string equal to a key of *replacements* for its mapped value. Used to keep notebook / + python-file paths and ``run_job_task`` job-id refs in sync after a namespacing rename, without + hard-coding every field name (``notebook_path``, ``python_file``, ``job_id``, …). + """ + if not replacements: + return + + def visit(node: Any) -> Any: + if isinstance(node, str): + return replacements.get(node, node) + if isinstance(node, dict): + for key, value in node.items(): + node[key] = visit(value) + return node + if isinstance(node, list): + for index, item in enumerate(node): + node[index] = visit(item) + return node + return node + + for task in tasks: + visit(task) + + +def _namespace_bundle_artifacts(workflow: PreparedWorkflow, prefix: str) -> None: + """Namespaces a workflow's notebooks and inner ForEach job keys by *prefix*, in place. + + When several pipelines share one bundle (``single`` / ``per-group`` modes), their notebook file + paths (derived from activity names) and inner ForEach job keys (``_inner_tasks``, + unique only within a pipeline) can collide in the shared ``resources/`` and ``src/`` dirs — two + pipelines writing the same path, the second silently overwriting the first while both jobs still + reference it. Prefixing every such artifact with the owning pipeline key makes them unique. + + Rewrites, consistently: + * each notebook ``relative_path`` (``notebooks/x.py`` -> ``notebooks//x.py``) and every + ``../src/notebooks/x.py`` task reference to it; + * each inner ForEach job ``name`` (so its resource key becomes ``__``) and every + ``${resources.jobs..id}`` ref to it. + + Pipeline-level job keys are NOT namespaced: pipeline names are unique within a migration, so they + never collide, and namespacing them would break cross-bundle ``${var.}`` wiring. + """ + replacements: dict[str, str] = {} + + # 1. Inner ForEach job keys: rename inner.name, map old resources.jobs ref -> new. + for inner in workflow.inner_workflows: + old_key = normalize_task_key(inner.name) + inner.name = f"{prefix}__{inner.name}" + new_key = normalize_task_key(inner.name) + if old_key != new_key: + replacements[f"${{resources.jobs.{old_key}.id}}"] = f"${{resources.jobs.{new_key}.id}}" + + # 2. Notebook paths: prefix each unique relative_path with the pipeline key as a subdirectory. + for wf in (workflow, *workflow.inner_workflows): + for notebook in wf.notebooks: + old_path = notebook.relative_path + new_path = _prefixed_notebook_relative_path(old_path, prefix) + if old_path != new_path: + notebook.relative_path = new_path + replacements[f"../src/{old_path}"] = f"../src/{new_path}" + # Some generated bodies reference their own bundle-relative path (e.g. the Spark-Python + # placeholder's `databricks fs cp ... src/` download hint). Rewrite that too so an + # operator following the instruction downloads to where the task now looks. + if f"src/{old_path}" in notebook.content: + notebook.content = notebook.content.replace(f"src/{old_path}", f"src/{new_path}") + + # 3. Rewrite every matching ref across the parent and inner task trees. + for wf in (workflow, *workflow.inner_workflows): + _rewrite_task_string_values(wf.tasks, replacements) + + +def _prefixed_notebook_relative_path(relative_path: str, prefix: str) -> str: + """Inserts *prefix* as a subdirectory under the top-level segment of a notebook relative path. + + ``notebooks/copy_data.py`` -> ``notebooks//copy_data.py``; a path with no ``/`` is just + prefixed. Idempotent when the prefix segment is already present. + """ + head, sep, tail = relative_path.partition("/") + if not sep: + return f"{prefix}/{relative_path}" + if tail.startswith(f"{prefix}/"): + return relative_path + return f"{head}/{prefix}/{tail}" + + +def write_bundle_group( + workflows: list[PreparedWorkflow], + output_dir: Path, + catalog: str = "main", + schema: str = "default", + bundle_name: str | None = None, + skipped_pipelines: list[str] | None = None, +) -> list[Path]: + """Writes one DAB bundle holding every workflow in *workflows*. + + Each workflow contributes its own job resource file (plus its inner ForEach job files and any + Lakeflow pipeline resources) into the bundle's shared ``resources/`` directory, exactly as the + per-pipeline path does — a bundle with several pipelines is just several job resources under one + ``databricks.yml``, reusing the same machinery that already emits a parent job plus its inner + ForEach jobs. Generated notebooks, setup notebooks, secrets, and the SETUP.md prerequisites are + unioned (and de-duplicated) across the group so shared code and setup scripts appear once. + + A ``run_job_task`` targeting a pipeline that is *also* in this group keeps its + ``${resources.jobs.X.id}`` ref (resolves within the bundle); one targeting a pipeline in another + bundle is rewritten to ``${var.X}`` and wired at deploy time — the cross-bundle set is simply + every pipeline/inner job key in the group. + + Args: + workflows: The PreparedWorkflows to co-locate in one bundle (>= 1). + output_dir: Root directory for the bundle output. + catalog: Default target catalog name. + schema: Default target schema name. + bundle_name: Bundle name (defaults to the first workflow's resource key). + + Returns: + List of absolute paths to all created files. + """ + # Reset module-level accumulators so successive write_bundle_group calls (CLI loops, tests) don't + # carry warnings or cross-bundle variables from one bundle into the next. _bundle_warnings.clear() _cross_bundle_variables.clear() _neutralized_conditions.clear() + _synthetic_default_parameters.clear() - workflow = copy.deepcopy(workflow) + if not workflows: + raise ValueError("write_bundle_group requires at least one workflow") + + # Deep-copy the input so writing a bundle never mutates the caller's PreparedWorkflows: several steps + # below rewrite the task dicts in place (namespacing, ${resources.jobs.X.id} -> ${var.X}), and leaking + # that back would erase the Run Pipeline edges pipeline_graph reads. Copying makes the writer a pure + # sink so callers can build the dependency graph before or after writing, in any order. + workflows = [copy.deepcopy(workflow) for workflow in workflows] output_dir = Path(output_dir) output_dir.mkdir(parents=True, exist_ok=True) + # When >1 pipeline shares this bundle, namespace each pipeline's notebooks and inner ForEach job + # keys by its pipeline key so activity-derived paths / inner-job keys (unique only within a + # pipeline) can't collide in the shared resources/ and src/ dirs. Single-pipeline bundles are left + # untouched, so the per-pipeline path stays byte-for-byte identical. + if len(workflows) > 1: + for workflow in workflows: + _namespace_bundle_artifacts(workflow, normalize_task_key(workflow.name)) + created_files: list[Path] = [] - resource_key = normalize_task_key(workflow.name) - effective_name = bundle_name or resource_key - known_bundle_jobs = _known_bundle_job_keys(workflow, resource_key) - _rewrite_cross_bundle_job_references(workflow, known_bundle_jobs) + effective_name = bundle_name or normalize_task_key(workflows[0].name) - # Bind clusters across the parent and inner workflows up front to decide whether databricks.yml needs + # Bind clusters across every workflow (parent + inner) up front to decide whether databricks.yml needs # cluster tunables at all. Binding is idempotent, so _build_job_resource re-checking these is harmless. - _bind_cluster_to_notebook_tasks(workflow.tasks) - for inner in workflow.inner_workflows: - _bind_cluster_to_notebook_tasks(inner.tasks) - bundle_uses_classic_cluster = _any_task_uses_classic_cluster(workflow.tasks) or any( - _any_task_uses_classic_cluster(inner.tasks) for inner in workflow.inner_workflows - ) + bundle_uses_classic_cluster = False + for workflow in workflows: + _bind_cluster_to_notebook_tasks(workflow.tasks) + for inner in workflow.inner_workflows: + _bind_cluster_to_notebook_tasks(inner.tasks) + if _any_task_uses_classic_cluster(workflow.tasks) or any( + _any_task_uses_classic_cluster(inner.tasks) for inner in workflow.inner_workflows + ): + bundle_uses_classic_cluster = True # Rewrite run_job_task refs to sibling-bundle jobs (${resources.jobs.X.id} where X is not a job in - # this bundle) into ${var.X}, registering each so _build_databricks_yml declares the variable. Must - # run before databricks.yml and the resource YAML are written below, or the ref points at a - # non-existent resource node and `bundle deploy` fails ("no such node resources.jobs.X"). - _known_bundle_jobs_for_rewrite = {normalize_task_key(workflow.name)} | { - normalize_task_key(inner.name) for inner in workflow.inner_workflows - } - _rewrite_cross_bundle_run_job_refs(workflow.tasks, _known_bundle_jobs_for_rewrite, _cross_bundle_variables) - for inner in workflow.inner_workflows: - _rewrite_cross_bundle_run_job_refs(inner.tasks, _known_bundle_jobs_for_rewrite, _cross_bundle_variables) - - pipeline_resources = _collect_pipeline_resources(workflow) + # this bundle) into ${var.X_job_id}, registering each so _build_databricks_yml declares the variable. + # The "known" set is every pipeline + inner + dbt-factory job key across the whole group, so + # intra-group calls stay as direct ${resources.jobs.X.id} refs. Must run before databricks.yml / + # resource YAML are written below, or the ref points at a non-existent node and deploy fails. + known_bundle_jobs: set[str] = set() + for workflow in workflows: + known_bundle_jobs |= _known_bundle_job_keys(workflow, normalize_task_key(workflow.name)) + for workflow in workflows: + _rewrite_cross_bundle_job_references(workflow, known_bundle_jobs) + + pipeline_resources: list[dict[str, Any]] = [] + for workflow in workflows: + pipeline_resources.extend(_collect_pipeline_resources(workflow)) pipeline_variable_declarations = _build_pipeline_variable_declarations(pipeline_resources, catalog, schema) - # sql_task references ${var.warehouse_id}; declare it (no default -> user supplies at deploy). - if _bundle_uses_sql_task(workflow): + # sql_task references ${var.warehouse_id}; declare it once if any workflow in the group uses one + # (no default -> user supplies at deploy). + if any(_bundle_uses_sql_task(workflow) for workflow in workflows): pipeline_variable_declarations.setdefault( "warehouse_id", {"description": "SQL warehouse id for sql_task queries"} ) @@ -155,12 +345,19 @@ def write_bundle( # dbt-factory PyDABs hooks: each `resources._dbt_job:load_resources` module must be # registered under the `python.resources` block so `bundle deploy` runs it to build the dbt job. - pydabs_resource_entries = _collect_pydabs_resource_entries(workflow) + # Union across every workflow in the group. + pydabs_resource_entries: list[str] = [] + for workflow in workflows: + pydabs_resource_entries.extend(_collect_pydabs_resource_entries(workflow)) + pydabs_resource_entries = list(dict.fromkeys(pydabs_resource_entries)) # 1. Write databricks.yml. When any task runs on classic compute, spark_version / node_type_id # defaults come from the ADF linked-service configs; when every task is serverless, they're omitted. + all_cluster_hints: list[dict[str, Any]] = [] + for workflow in workflows: + all_cluster_hints.extend(workflow.cluster_hints) databricks_yml_path = output_dir / "databricks.yml" - inferred_spark_version, inferred_node_type_id = _infer_bundle_cluster_defaults(workflow) + inferred_spark_version, inferred_node_type_id = _infer_cluster_defaults_from_hints(all_cluster_hints) databricks_yml_dict = _build_databricks_yml( effective_name, catalog, @@ -183,44 +380,48 @@ def write_bundle( ) created_files.append(databricks_yml_path.resolve()) - # 2. Write job resource YAML. Strip broken base_parameters from existing-notebook tasks first — - # they're surfaced in SETUP.md further down and shouldn't ship in the YAML as malformed values. - manual_parameters: list[ManualParameter] = _extract_manual_parameters_from_existing_notebook_tasks(workflow.tasks) - for inner in workflow.inner_workflows: - manual_parameters.extend(_extract_manual_parameters_from_existing_notebook_tasks(inner.tasks)) - + # 2. Write one job resource YAML per workflow (plus its inner ForEach job files) into the shared + # resources/ dir. Strip broken base_parameters from existing-notebook tasks first — they're + # surfaced in SETUP.md further down and shouldn't ship in the YAML as malformed values. + manual_parameters: list[ManualParameter] = [] resources_dir = output_dir / "resources" resources_dir.mkdir(parents=True, exist_ok=True) - job_yml_path = resources_dir / f"{resource_key}.yml" hoisted_global_names = set(hoisted_global_variables) - job_resource = _build_job_resource(workflow, resource_key, hoisted_globals=hoisted_global_names) - job_yml_path.write_text( - yaml.dump( - job_resource, default_flow_style=False, sort_keys=False, allow_unicode=True, Dumper=_BundleYamlDumper - ), - encoding="utf-8", - ) - created_files.append(job_yml_path.resolve()) - - # Write inner workflows as additional resource files. Inner tasks reuse notebooks from the parent's - # list, so pass those in for the inner job's widget auto-augmentation. - for inner in workflow.inner_workflows: - inner_key = normalize_task_key(inner.name) - inner_yml_path = resources_dir / f"{inner_key}.yml" - inner_resource = _build_job_resource( - inner, inner_key, extra_notebooks_for_augment=workflow.notebooks, hoisted_globals=hoisted_global_names - ) - inner_yml_path.write_text( + for workflow in workflows: + resource_key = normalize_task_key(workflow.name) + manual_parameters.extend(_extract_manual_parameters_from_existing_notebook_tasks(workflow.tasks)) + for inner in workflow.inner_workflows: + manual_parameters.extend(_extract_manual_parameters_from_existing_notebook_tasks(inner.tasks)) + + job_yml_path = resources_dir / f"{resource_key}.yml" + job_resource = _build_job_resource(workflow, resource_key, hoisted_globals=hoisted_global_names) + job_yml_path.write_text( yaml.dump( - inner_resource, - default_flow_style=False, - sort_keys=False, - allow_unicode=True, - Dumper=_BundleYamlDumper, + job_resource, default_flow_style=False, sort_keys=False, allow_unicode=True, Dumper=_BundleYamlDumper ), encoding="utf-8", ) - created_files.append(inner_yml_path.resolve()) + created_files.append(job_yml_path.resolve()) + + # Write inner workflows as additional resource files. Inner tasks reuse notebooks from the parent's + # list, so pass those in for the inner job's widget auto-augmentation. + for inner in workflow.inner_workflows: + inner_key = normalize_task_key(inner.name) + inner_yml_path = resources_dir / f"{inner_key}.yml" + inner_resource = _build_job_resource( + inner, inner_key, extra_notebooks_for_augment=workflow.notebooks, hoisted_globals=hoisted_global_names + ) + inner_yml_path.write_text( + yaml.dump( + inner_resource, + default_flow_style=False, + sort_keys=False, + allow_unicode=True, + Dumper=_BundleYamlDumper, + ), + encoding="utf-8", + ) + created_files.append(inner_yml_path.resolve()) # 2b. Write Lakeflow pipeline resources (Lakeflow Connect ingestion defs from the Copy preparer's LFC # branch). Each lives in its own YAML so the bundle parser merges them via the ``include`` glob. @@ -238,10 +439,17 @@ def write_bundle( ) created_files.append(resource_yml_path.resolve()) - # 3. Write generated notebooks. PyDABs hook modules (relative_path under ``resources/``) are - # Python resources the bundle imports as ``resources.`` from the bundle root, so they - # go to output_dir; all other generated notebooks go under ``src/``. + # 3. Write generated notebooks (union of every workflow's + inner ForEach jobs'; dedupe drops repeats + # of the same path — see _dedupe_notebooks). PyDABs hook modules (relative_path under ``resources/``) + # and pyproject.toml are Python resources the bundle imports from its root, so they go to + # output_dir; every other generated notebook goes under ``src/``. src_dir = output_dir / "src" + generated_notebooks: list[DabNotebook] = [] + for workflow in workflows: + generated_notebooks.extend(workflow.notebooks) + for inner in workflow.inner_workflows: + generated_notebooks.extend(inner.notebooks) + generated_notebooks = _dedupe_notebooks(generated_notebooks) def _write_generated(notebooks: list[DabNotebook]) -> None: root_artifacts = [ @@ -255,84 +463,52 @@ def _write_generated(notebooks: list[DabNotebook]) -> None: if root_artifacts: created_files.extend(write_notebooks(root_artifacts, output_dir)) - if workflow.notebooks: - _write_generated(workflow.notebooks) + if generated_notebooks: + _write_generated(generated_notebooks) # 4. Generate and write setup notebooks (create-scope, create-volume, etc.) — the executable - # provisioning artifacts; SETUP.md (below) is the human-readable companion. + # provisioning artifacts; SETUP.md (below) is the human-readable companion. Generate once over the + # unioned secrets/setup tasks so a bundle with several pipelines has one consolidated setup/ dir. + all_secrets = [secret for workflow in workflows for secret in _iter_workflow_secrets(workflow)] + all_setup_tasks = [task for workflow in workflows for task in _iter_workflow_setup_tasks(workflow)] setup_notebooks: list[DabNotebook] = generate_setup_tasks( - secrets=workflow.secrets, - setup_tasks=workflow.setup_tasks, + secrets=all_secrets, + setup_tasks=all_setup_tasks, catalog=catalog, schema=schema, ) if setup_notebooks: - created_files.extend(write_notebooks(setup_notebooks, src_dir)) - - # Collect notebooks from inner workflows - for inner in workflow.inner_workflows: - if inner.notebooks: - _write_generated(inner.notebooks) - inner_setup = generate_setup_tasks( - secrets=inner.secrets, - setup_tasks=inner.setup_tasks, - catalog=catalog, - schema=schema, - ) - if inner_setup: - created_files.extend(write_notebooks(inner_setup, src_dir)) + created_files.extend(write_notebooks(_dedupe_notebooks(setup_notebooks), src_dir)) + + # 5. Build one SETUP.md for the whole bundle — a root-level, human-readable summary of every external + # step needed before ``bundle run``. Additive to the setup/ notebooks above (the executable path). + # Unions every workflow (parent + inner ForEach jobs) so a multi-pipeline bundle gets one SETUP.md. + all_notebooks: list[DabNotebook] = list(generated_notebooks) + all_tasks: list[dict[str, Any]] = [] + parameter_approximations: list[Any] = [] + rollup_configs: list[dict[str, Any]] = [] + dynamic_dispatch_configs: list[dict[str, Any]] = [] + unresolved_library_configs: list[dict[str, Any]] = [] + manual_variable_init_configs: list[dict[str, Any]] = [] + manual_schedule_time_of_day_configs: list[dict[str, Any]] = [] + manual_credential_configs: list[dict[str, Any]] = [] + airflow_backfill_configs: list[dict[str, Any]] = [] + pydabs_dbt_factory_configs: list[dict[str, Any]] = [] + for workflow in workflows: + for wf in (workflow, *workflow.inner_workflows): + all_tasks.extend(wf.tasks) + parameter_approximations.extend(wf.parameter_approximations) + rollup_configs.extend(t.config for t in wf.setup_tasks if t.type == "manual_variable_rollup") + dynamic_dispatch_configs.extend(t.config for t in wf.setup_tasks if t.type == "dynamic_notebook_dispatch") + unresolved_library_configs.extend(t.config for t in wf.setup_tasks if t.type == "unresolved_library") + manual_variable_init_configs.extend(t.config for t in wf.setup_tasks if t.type == "manual_variable_init") + manual_schedule_time_of_day_configs.extend( + t.config for t in wf.setup_tasks if t.type == "manual_schedule_time_of_day" + ) + manual_credential_configs.extend(t.config for t in wf.setup_tasks if t.type == "manual_credential") + airflow_backfill_configs.extend(t.config for t in wf.setup_tasks if t.type == "airflow_backfill") + pydabs_dbt_factory_configs.extend(t.config for t in wf.setup_tasks if t.type == "pydabs_dbt_factory") - # 5. Build SETUP.md — a root-level, human-readable summary of every external step needed before - # ``bundle run``. Additive to the setup/ notebooks above (those are the executable path). - all_notebooks = list(workflow.notebooks) - for inner in workflow.inner_workflows: - all_notebooks.extend(inner.notebooks) - all_tasks = list(workflow.tasks) - for inner in workflow.inner_workflows: - all_tasks.extend(inner.tasks) - parameter_approximations = list(workflow.parameter_approximations) - for inner in workflow.inner_workflows: - parameter_approximations.extend(inner.parameter_approximations) - known_bundle_jobs = _known_bundle_job_keys(workflow, resource_key) - # manual_parameters was collected above (before YAML emission) so broken values are stripped on disk too. - # VAREX3-003: manual_variable_rollup SetupTasks from workflow_preparer surface in SETUP.md so the user - # knows where to add a roll-up notebook. - rollup_configs = [task.config for task in workflow.setup_tasks if task.type == "manual_variable_rollup"] - for inner in workflow.inner_workflows: - rollup_configs.extend(task.config for task in inner.setup_tasks if task.type == "manual_variable_rollup") - dynamic_dispatch_configs = [ - task.config for task in workflow.setup_tasks if task.type == "dynamic_notebook_dispatch" - ] - unresolved_library_configs = [task.config for task in workflow.setup_tasks if task.type == "unresolved_library"] - manual_variable_init_configs = [task.config for task in workflow.setup_tasks if task.type == "manual_variable_init"] - manual_schedule_time_of_day_configs = [ - task.config for task in workflow.setup_tasks if task.type == "manual_schedule_time_of_day" - ] - manual_credential_configs = [task.config for task in workflow.setup_tasks if task.type == "manual_credential"] - airflow_backfill_configs = [task.config for task in workflow.setup_tasks if task.type == "airflow_backfill"] - pydabs_dbt_factory_configs = [task.config for task in workflow.setup_tasks if task.type == "pydabs_dbt_factory"] - for inner in workflow.inner_workflows: - pydabs_dbt_factory_configs.extend( - task.config for task in inner.setup_tasks if task.type == "pydabs_dbt_factory" - ) - dynamic_dispatch_configs.extend( - task.config for task in inner.setup_tasks if task.type == "dynamic_notebook_dispatch" - ) - unresolved_library_configs.extend( - task.config for task in inner.setup_tasks if task.type == "unresolved_library" - ) - manual_variable_init_configs.extend( - task.config for task in inner.setup_tasks if task.type == "manual_variable_init" - ) - manual_schedule_time_of_day_configs.extend( - task.config for task in inner.setup_tasks if task.type == "manual_schedule_time_of_day" - ) - manual_credential_configs.extend(task.config for task in inner.setup_tasks if task.type == "manual_credential") - # LSC3-006: union typed SecretInstructions (workflow + inner) with notebook-scanned scopes so SETUP.md - # and create_secrets.py reference the same set of (scope, key) pairs. - all_secret_instructions = list(workflow.secrets) - for inner in workflow.inner_workflows: - all_secret_instructions.extend(inner.secrets) prereqs = build_prereqs( notebooks=all_notebooks, tasks=all_tasks, @@ -341,7 +517,7 @@ def _write_generated(notebooks: list[DabNotebook]) -> None: manual_parameters=manual_parameters, parameter_approximations=parameter_approximations, manual_variable_rollups=rollup_configs, - secret_instructions=all_secret_instructions, + secret_instructions=all_secrets, dynamic_notebook_dispatches=dynamic_dispatch_configs, unresolved_libraries=unresolved_library_configs, manual_variable_inits=manual_variable_init_configs, @@ -352,12 +528,13 @@ def _write_generated(notebooks: list[DabNotebook]) -> None: pydabs_dbt_factories=pydabs_dbt_factory_configs, airflow_backfills=airflow_backfill_configs, skipped_pipelines=list(skipped_pipelines or []), + synthetic_default_parameters=list(_synthetic_default_parameters), ) setup_path = output_dir / "SETUP.md" setup_path.write_text(render_setup_md(prereqs, bundle_name=effective_name), encoding="utf-8") created_files.append(setup_path.resolve()) - # 5. Write warnings file if any warnings were collected + # 6. Write warnings file if any warnings were collected if _bundle_warnings: warnings_path = output_dir / "WARNINGS.md" lines = [ @@ -374,6 +551,22 @@ def _write_generated(notebooks: list[DabNotebook]) -> None: return created_files +def _iter_workflow_secrets(workflow: PreparedWorkflow) -> list[SecretInstruction]: + """Returns a workflow's own secrets plus its inner ForEach jobs' secrets (LSC3-006 union).""" + secrets = list(workflow.secrets) + for inner in workflow.inner_workflows: + secrets.extend(inner.secrets) + return secrets + + +def _iter_workflow_setup_tasks(workflow: PreparedWorkflow) -> list[SetupTask]: + """Returns a workflow's own setup tasks plus its inner ForEach jobs' setup tasks.""" + tasks = list(workflow.setup_tasks) + for inner in workflow.inner_workflows: + tasks.extend(inner.setup_tasks) + return tasks + + def _default_report_path(output_dir: Path) -> Path: """Returns the conventional report path under a migration dir's .work/ folder. @@ -387,6 +580,167 @@ def _default_report_path(output_dir: Path) -> Path: return work / "translation_report.json" +PACKAGING_MODES = ("per-pipeline", "single", "per-group") + + +def _load_group_spec(path: Path) -> dict[str, str]: + """Loads a user-specified pipeline->group map for ``per-group`` grouping. + + Accepts JSON or YAML. Two shapes are supported: + + - ``{"": "", ...}`` — a flat pipeline-to-group map. + - ``{"": ["", ...], ...}`` — a group-to-pipelines map (a list value). + + Pipeline names are normalized with :func:`normalize_task_key` so the spec can use either the ADF + display name or the resource key. Returns a ``{pipeline_key: group_name}`` map. + """ + raw = yaml.safe_load(Path(path).read_text(encoding="utf-8")) or {} + if not isinstance(raw, dict): + raise ValueError(f"Group spec {path} must be a mapping, got {type(raw).__name__}.") + mapping: dict[str, str] = {} + for key, value in raw.items(): + if isinstance(value, list): + for pipeline in value: + mapping[normalize_task_key(str(pipeline))] = str(key) + else: + mapping[normalize_task_key(str(key))] = str(value) + return mapping + + +def _group_workflows( + workflows: list[PreparedWorkflow], + *, + mode: str, + group_by: str = "inferred", + group_spec: dict[str, str] | None = None, + bundle_name: str | None = None, + pipeline_deps: dict[str, set[str]] | None = None, +) -> list[tuple[str, list[PreparedWorkflow]]]: + """Maps workflows to bundles per the packaging *mode*. + + Args: + workflows: Prepared workflows loaded from the translation report. + mode: One of :data:`PACKAGING_MODES`. + group_by: For ``per-group`` only — ``"inferred"`` (connected components of the Run Pipeline + call graph) or ``"spec"`` (honor *group_spec*). + group_spec: For ``per-group`` + ``group_by="spec"`` — ``{pipeline_key: group_name}``. + bundle_name: Optional bundle name; used to name the single bundle in ``single`` mode. + pipeline_deps: Precomputed Run Pipeline dependency graph (from + :func:`flowx.bundler.pipeline_graph.build_pipeline_dependencies`). Passed in by ``main`` + so the ``per-group inferred`` branch reuses it instead of rescanning every task tree; + computed on demand when omitted (e.g. direct callers/tests). + + Returns: + A list of ``(bundle_dir_name, [workflows])`` tuples, one per bundle to write. Bundle dir + names are normalized and deterministic. For a single workflow this collapses to one bundle + regardless of mode. + """ + if not workflows: + return [] + + by_key = {normalize_task_key(wf.name): wf for wf in workflows} + # Every downstream keying (grouping, resource filenames, ${resources.jobs.X.id} refs, the pipeline + # graph) assumes each pipeline has a unique normalized key. Two pipeline names that collapse to the + # same key (e.g. "Load Sales" and "load-sales") would silently drop one here and from every mode's + # output. Fail loudly instead so the collision is fixed at the source rather than shipped incomplete. + if len(by_key) != len(workflows): + from collections import Counter + + counts = Counter(normalize_task_key(wf.name) for wf in workflows) + dupes = sorted(key for key, count in counts.items() if count > 1) + raise ValueError( + "Pipeline names collide on normalized key(s): " + + ", ".join(dupes) + + ". Rename the offending pipelines so each maps to a unique resource key." + ) + + if mode == "single" or len(workflows) == 1: + name = normalize_task_key(bundle_name) if bundle_name else normalize_task_key(workflows[0].name) + if mode == "single" and not bundle_name and len(workflows) > 1: + name = "flowx_bundle" + return [(name, list(workflows))] + + if mode == "per-pipeline": + return [(key, [wf]) for key, wf in by_key.items()] + + if mode == "per-group": + from flowx.bundler.pipeline_graph import ( + PipelineCycleError, + build_pipeline_dependencies, + connected_components, + topo_order, + ) + + if group_by == "spec": + if not group_spec: + raise ValueError("per-group with --group-by spec requires a --group-spec file.") + explicit_groups = {normalize_task_key(g) for g in group_spec.values()} + groups: dict[str, list[PreparedWorkflow]] = {} + for key, wf in by_key.items(): + if key in group_spec: + group_name = normalize_task_key(group_spec[key]) + else: + # Pipelines absent from the spec become their own single-pipeline bundle. Guard + # against a pipeline's own key colliding with an explicit group name, which would + # silently merge it into that group's bundle. + group_name = normalize_task_key(key) + if group_name in explicit_groups: + raise ValueError( + f"Pipeline '{wf.name}' is absent from the group spec, so it would form its " + f"own bundle named '{group_name}', which collides with an explicit group " + "name. Add it to the spec or rename the group." + ) + groups.setdefault(group_name, []).append(wf) + return [(name, groups[name]) for name in sorted(groups)] + + # Inferred: connected components of the Run Pipeline call graph. Reuse the graph main already + # computed when provided, so a full package run scans the task trees only once. + deps = pipeline_deps if pipeline_deps is not None else build_pipeline_dependencies(workflows) + # Callees-first order across the whole migration; used to pick each component's callee root as + # the bundle name. Falls back to alphabetical when the graph is cyclic (no valid topo order). + try: + order_rank = {key: rank for rank, key in enumerate(topo_order(deps))} + except PipelineCycleError: + order_rank = {} + result: list[tuple[str, list[PreparedWorkflow]]] = [] + for component in connected_components(deps): + members = [by_key[key] for key in component if key in by_key] + # Name the bundle after the component's callee root: the member that comes earliest in + # callees-first topo order (deploys first, others depend on it). Ties / cyclic graphs fall + # back to the alphabetically-first member (component is already sorted for determinism). + root = min(component, key=lambda key: (order_rank.get(key, 0), key)) + result.append((root, members)) + return result + + raise ValueError(f"Unknown packaging mode: {mode!r}. Expected one of {PACKAGING_MODES}.") + + +def _render_deploy_md_for_run( + written_groups: list[tuple[str, list[PreparedWorkflow]]], + pipeline_deps: dict[str, set[str]], + *, + single_bundle: bool, + packaging_mode: str, +) -> str: + """Builds DEPLOY.md from the workflows written this run (bridges to :mod:`deploy_writer`). + + *pipeline_deps* must be computed **before** write_bundle_group mutates the task dicts (it rewrites + cross-bundle ``${resources.jobs.X.id}`` refs to ``${var.X}`` in place, which erases the edges). + """ + from flowx.bundler.deploy_writer import render_deploy_md + + groups = [ + (group_name, [normalize_task_key(wf.name) for wf in group_workflows]) + for group_name, group_workflows in written_groups + ] + return render_deploy_md( + groups, + pipeline_deps, + single_bundle=single_bundle, + packaging_mode=packaging_mode, + ) + + def main(argv: list[str] | None = None) -> int: """Package-phase entry point for DAB bundle generation. @@ -455,6 +809,36 @@ def main(argv: list[str] | None = None) -> int: action="store_true", help="Keep the transient .work/ folder (translation report + IR) instead of pruning it.", ) + parser.add_argument( + "--packaging-mode", + type=str, + choices=PACKAGING_MODES, + default="per-pipeline", + help=( + "How to lay out bundles for a multi-pipeline migration: 'per-pipeline' (default, one " + "bundle per pipeline), 'single' (all pipelines in one bundle), or 'per-group' (group " + "pipelines into bundles — see --group-by)." + ), + ) + parser.add_argument( + "--group-by", + type=str, + choices=("inferred", "spec"), + default="inferred", + help=( + "For --packaging-mode per-group: 'inferred' groups pipelines by their Run Pipeline " + "(ExecutePipeline) call graph; 'spec' uses the mapping in --group-spec." + ), + ) + parser.add_argument( + "--group-spec", + type=Path, + default=None, + help=( + "For --packaging-mode per-group --group-by spec: JSON/YAML file mapping pipelines to " + "group names ({pipeline: group} or {group: [pipelines]})." + ), + ) args = parser.parse_args(argv) if args.report is None: @@ -505,87 +889,142 @@ def main(argv: list[str] | None = None) -> int: print("No translated pipelines found in the report.", file=sys.stderr) return 1 - shared_airflow_bundle = len(workflows) > 1 and all(workflow.source == "airflow" for workflow in workflows) from flowx.validate.bundle_invariants import check_bundle_dir, format_result - # Render and validate away from the destination. This keeps a reconciliation or structural - # failure from leaving a partially-written bundle in the migration directory. - args.output_dir.parent.mkdir(parents=True, exist_ok=True) - with tempfile.TemporaryDirectory(prefix=".flowx-preflight-", dir=args.output_dir.parent) as temporary: - staging_root = Path(temporary) - if shared_airflow_bundle: - write_bundle( - workflow=_combine_airflow_workflows(workflows), + # Airflow migrations with >1 DAG collapse into a single combined bundle (each DAG becomes a job in + # one bundle); ADF and single-DAG runs go through the configurable packaging modes below. + shared_airflow_bundle = len(workflows) > 1 and all(workflow.source == "airflow" for workflow in workflows) + + all_created: list[Path] = [] + bundle_dirs: list[Path] = [] + + if shared_airflow_bundle: + combined_name = args.bundle_name or normalize_task_key(args.output_dir.name) + # Render + validate away from the destination first, so a structural failure never leaves a + # partially-written bundle in the migration directory. + args.output_dir.parent.mkdir(parents=True, exist_ok=True) + with tempfile.TemporaryDirectory(prefix=".flowx-preflight-", dir=args.output_dir.parent) as temporary: + staging_root = Path(temporary) + write_bundle_group( + [_combine_airflow_workflows(workflows)], output_dir=staging_root, catalog=args.catalog, schema=args.schema, - bundle_name=args.bundle_name or normalize_task_key(args.output_dir.name), + bundle_name=combined_name, ) - staged_dirs = [staging_root] - else: - staged_dirs = [] - for workflow in workflows: - workflow_dir = staging_root / normalize_task_key(workflow.name) if len(workflows) > 1 else staging_root - write_bundle( - workflow=workflow, - output_dir=workflow_dir, - catalog=args.catalog, - schema=args.schema, - bundle_name=args.bundle_name if len(workflows) == 1 else None, - ) - staged_dirs.append(workflow_dir) - preflight_violations = 0 - for bundle_dir in staged_dirs: - result = check_bundle_dir(bundle_dir) + result = check_bundle_dir(staging_root) if not result.ok or result.warnings: print(format_result(result), file=sys.stderr) - preflight_violations += len(result.violations) - if preflight_violations: - print( - f"Error: package preflight found {preflight_violations} bundle-invariant violation(s); " - "no bundle files were written.", - file=sys.stderr, - ) - return 1 - - all_created: list[Path] = [] - if shared_airflow_bundle: - combined = _combine_airflow_workflows(workflows) + if result.violations: + print( + f"Error: package preflight found {len(result.violations)} bundle-invariant " + "violation(s); no bundle files were written.", + file=sys.stderr, + ) + return 1 all_created.extend( - write_bundle( - workflow=combined, + write_bundle_group( + [_combine_airflow_workflows(workflows)], output_dir=args.output_dir, catalog=args.catalog, schema=args.schema, - bundle_name=args.bundle_name or normalize_task_key(args.output_dir.name), + bundle_name=combined_name, skipped_pipelines=skipped_pipelines, ) ) + bundle_dirs = [args.output_dir] print(f" [1/1] {len(workflows)} Airflow DAG jobs: {len(all_created)} files") else: - for index, workflow in enumerate(workflows): - workflow_dir = ( - args.output_dir / normalize_task_key(workflow.name) if len(workflows) > 1 else args.output_dir - ) - effective_bundle_name = args.bundle_name if len(workflows) == 1 else None - created = write_bundle( - workflow=workflow, - output_dir=workflow_dir, - catalog=args.catalog, - schema=args.schema, - bundle_name=effective_bundle_name, - skipped_pipelines=skipped_pipelines, + group_spec: dict[str, str] | None = None + if args.group_spec is not None: + if not args.group_spec.exists(): + print(f"Error: group spec file not found: {args.group_spec}", file=sys.stderr) + return 1 + group_spec = _load_group_spec(args.group_spec) + + # Build the Run Pipeline dependency graph once and thread it into grouping and DEPLOY.md. + # write_bundle_group works on a deep copy, so the graph is valid regardless of call order; + # computing it here just scans the task trees once. + from flowx.bundler.pipeline_graph import build_pipeline_dependencies + + pipeline_deps = build_pipeline_dependencies(workflows) + + try: + groups = _group_workflows( + workflows, + mode=args.packaging_mode, + group_by=args.group_by, + group_spec=group_spec, + bundle_name=args.bundle_name, + pipeline_deps=pipeline_deps, ) - all_created.extend(created) - print(f" [{index + 1}/{len(workflows)}] {workflow.name}: {len(created)} files") + except ValueError as exc: + print(f"Error: {exc}", file=sys.stderr) + return 1 + + # A single bundle (single mode, or only one pipeline) lands at the output dir root; multiple + # bundles each get their own subdirectory named after the group. + single_bundle = len(groups) == 1 + + def _write_groups(target_root: Path, *, announce: bool) -> tuple[list[Path], list[Path]]: + """Writes every group bundle under *target_root*; returns (created_files, bundle_dirs).""" + created_all: list[Path] = [] + dirs: list[Path] = [] + for index, (group_name, group_workflows) in enumerate(groups): + # Name the bundle after group_name (e.g. "flowx_bundle"), not the first pipeline, so the + # workspace path is .bundle//dev for a bundle holding many pipelines. + b_dir = target_root if single_bundle else target_root / group_name + created = write_bundle_group( + group_workflows, + output_dir=b_dir, + catalog=args.catalog, + schema=args.schema, + bundle_name=group_name, + skipped_pipelines=skipped_pipelines, + ) + created_all.extend(created) + dirs.append(b_dir) + if announce: + names = ", ".join(wf.name for wf in group_workflows) + print(f" [{index + 1}/{len(groups)}] {group_name} ({names}): {len(created)} files") + return created_all, dirs + + # Render + validate in a temp dir first, so a structural failure never leaves a partially + # written bundle in the migration directory. + args.output_dir.parent.mkdir(parents=True, exist_ok=True) + with tempfile.TemporaryDirectory(prefix=".flowx-preflight-", dir=args.output_dir.parent) as temporary: + _, staged_dirs = _write_groups(Path(temporary), announce=False) + preflight_violations = 0 + for staged_dir in staged_dirs: + result = check_bundle_dir(staged_dir) + if not result.ok or result.warnings: + print(format_result(result), file=sys.stderr) + preflight_violations += len(result.violations) + if preflight_violations: + print( + f"Error: package preflight found {preflight_violations} bundle-invariant " + "violation(s); no bundle files were written.", + file=sys.stderr, + ) + return 1 + + group_created, bundle_dirs = _write_groups(args.output_dir, announce=True) + all_created.extend(group_created) + + # Write a top-level DEPLOY.md documenting every bundle, its cross-bundle deps, and deploy order. + deploy_md = _render_deploy_md_for_run( + [(group_name, group_workflows) for group_name, group_workflows in groups], + pipeline_deps, + single_bundle=single_bundle, + packaging_mode=args.packaging_mode, + ) + deploy_path = args.output_dir / "DEPLOY.md" + deploy_path.write_text(deploy_md, encoding="utf-8") + all_created.append(deploy_path.resolve()) + print(f" DEPLOY.md: deploy order for {len(groups)} bundle(s)") # Tier-0 structural check over the emitted bundle(s): duplicate task keys / job params, # dangling depends_on, undeclared {{job.parameters.X}}, leaked YAML anchors. Source-agnostic. - bundle_dirs = ( - [args.output_dir] - if shared_airflow_bundle or len(workflows) == 1 - else [args.output_dir / normalize_task_key(workflow.name) for workflow in workflows] - ) invariant_violations = 0 for bundle_dir in bundle_dirs: result = check_bundle_dir(bundle_dir) @@ -780,6 +1219,18 @@ def _infer_bundle_cluster_defaults(workflow: PreparedWorkflow) -> tuple[str, str Args: workflow: The prepared workflow being written. + Returns: + ``(spark_version, node_type_id)`` strings. + """ + return _infer_cluster_defaults_from_hints(workflow.cluster_hints) + + +def _infer_cluster_defaults_from_hints(cluster_hints: list[dict[str, Any]]) -> tuple[str, str]: + """Derive ``spark_version`` / ``node_type_id`` defaults from a (possibly multi-workflow) hint list. + + Split out from :func:`_infer_bundle_cluster_defaults` so a bundle holding several pipelines can + pass the union of every member's ``cluster_hints`` and get one consensus default pair. + Returns: ``(spark_version, node_type_id)`` strings. """ @@ -788,11 +1239,9 @@ def _infer_bundle_cluster_defaults(workflow: PreparedWorkflow) -> tuple[str, str # C-29 (NB-ITER4-002): filter out unparseable spark_version / node_type_id hints before Counter so # unresolved ADF expressions don't land as the bundle default and break ``databricks bundle deploy``. spark_versions = [ - hint["spark_version"] for hint in workflow.cluster_hints if _is_valid_spark_version(hint.get("spark_version")) - ] - node_types = [ - hint["node_type_id"] for hint in workflow.cluster_hints if _is_valid_node_type_id(hint.get("node_type_id")) + hint["spark_version"] for hint in cluster_hints if _is_valid_spark_version(hint.get("spark_version")) ] + node_types = [hint["node_type_id"] for hint in cluster_hints if _is_valid_node_type_id(hint.get("node_type_id"))] spark_version = Counter(spark_versions).most_common(1)[0][0] if spark_versions else _DEFAULT_SPARK_VERSION node_type_id = Counter(node_types).most_common(1)[0][0] if node_types else _DEFAULT_NODE_TYPE_ID @@ -1554,9 +2003,6 @@ def visit(task: dict[str, Any]) -> None: return neutralized -_CROSS_BUNDLE_JOB_ID_REF = re.compile(r"\$\{resources\.jobs\.([^.]+)\.id\}") - - def _rewrite_cross_bundle_run_job_refs( tasks: list[dict[str, Any]], known_bundle_jobs: set[str], @@ -1710,6 +2156,10 @@ def _build_job_resource( # field keeps both bundle paths byte-identical and matches the Databricks job-parameter schema. seen_param_names: set[str | None] = set() normalized_parameters: list[dict[str, Any]] = [] + # Names that got a synthetic "" default this pass. Deferred to _synthetic_default_parameters + # until AFTER any trigger parameter_overrides run below, so a param a trigger pins to a real + # value is not still reported in SETUP.md as "receives an empty string". + synthetic_default_names: list[str] = [] for parameter in workflow.parameters: name = parameter.get("name") if name in seen_param_names: @@ -1717,7 +2167,18 @@ def _build_job_resource( seen_param_names.add(name) entry: dict[str, Any] = {"name": name} default = parameter.get("default") - entry["default"] = default if isinstance(default, str) else json.dumps(default) + if default is None: + # A DAB job parameter requires a ``default``; ADF params without one (defaulted by the + # caller at ExecutePipeline/trigger time) get an empty string so `bundle deploy` doesn't + # reject the missing field. Record it so SETUP.md surfaces the synthetic default rather + # than silently substituting "" — a run that forgets to override it will otherwise see "" + # instead of failing fast. + entry["default"] = "" + synthetic_default_names.append(str(name)) + else: + # Databricks job-parameter defaults are strings; pass strings through and JSON-encode + # everything else (numbers / bools / arrays / objects) so the YAML carries a valid default. + entry["default"] = default if isinstance(default, str) else json.dumps(default) normalized_parameters.append(entry) job_def["parameters"] = normalized_parameters @@ -1734,6 +2195,14 @@ def _build_job_resource( override = overrides[entry["name"]] entry["default"] = override if isinstance(override, str) else json.dumps(override) + # Now that trigger overrides have been applied, surface only the params still carrying a synthetic + # "" default (a trigger-pinned param now has a real default and no longer needs operator attention). + if workflow.parameters and synthetic_default_names: + pinned = set((getattr(workflow, "schedule", None) or {}).get("parameter_overrides") or {}) + for name in synthetic_default_names: + if name not in pinned: + _synthetic_default_parameters.append({"job": resource_key, "name": name}) + return { "resources": { "jobs": { diff --git a/src/flowx/bundler/deploy_writer.py b/src/flowx/bundler/deploy_writer.py new file mode 100644 index 0000000..1ed58cc --- /dev/null +++ b/src/flowx/bundler/deploy_writer.py @@ -0,0 +1,159 @@ +"""Renders the top-level ``DEPLOY.md`` describing bundle layout and deploy order. + +A multi-pipeline migration can emit several bundles that must be deployed in dependency order: a +callee pipeline's job must exist before a caller's ``run_job_task`` can reference its numeric id. +``DEPLOY.md`` is the human-readable companion to the automated +:mod:`flowx.bundler.deployer` (``flowx.adapter deploy``): it lists each bundle, the pipelines it +contains, the cross-bundle ``${var.X}`` dependencies it has, and a suggested callees-first deploy +order — collapsed from the pipeline-level Run Pipeline graph to bundle granularity. + +This module only *renders* text; it reads no filesystem state and shells out to nothing, so it works +identically on local CLI and Databricks serverless. +""" + +from __future__ import annotations + +from flowx.bundler.pipeline_graph import PipelineCycleError, topo_order + + +def _bundle_of_pipeline(groups: list[tuple[str, list[str]]]) -> dict[str, str]: + """Returns a ``pipeline_key -> bundle_dir`` map from ``(bundle_dir, [pipeline_keys])`` groups.""" + mapping: dict[str, str] = {} + for bundle_dir, pipeline_keys in groups: + for key in pipeline_keys: + mapping[key] = bundle_dir + return mapping + + +def _bundle_dependencies( + groups: list[tuple[str, list[str]]], + pipeline_deps: dict[str, set[str]], +) -> dict[str, set[str]]: + """Collapses the pipeline-level dep graph to a ``bundle_dir -> {dependency bundle_dirs}`` graph. + + A bundle depends on another when any of its pipelines calls a pipeline that lives in the other + bundle. Intra-bundle calls and calls to pipelines outside the migration are dropped. + """ + bundle_of = _bundle_of_pipeline(groups) + graph: dict[str, set[str]] = {bundle_dir: set() for bundle_dir, _ in groups} + for caller_pipeline, callees in pipeline_deps.items(): + caller_bundle = bundle_of.get(caller_pipeline) + if caller_bundle is None: + continue + for callee_pipeline in callees: + callee_bundle = bundle_of.get(callee_pipeline) + if callee_bundle is None or callee_bundle == caller_bundle: + continue + graph[caller_bundle].add(callee_bundle) + return graph + + +def _bundle_deploy_order(bundle_graph: dict[str, set[str]]) -> tuple[list[str], bool]: + """Returns ``(ordered_bundle_dirs, ok)``. Callees first; ``ok`` is False on a cyclic graph. + + Falls back to a stable sorted order when the graph is cyclic so DEPLOY.md still renders (the + cycle is called out in the prose). + """ + try: + return topo_order({node: set(deps) for node, deps in bundle_graph.items()}), True + except PipelineCycleError: + return sorted(bundle_graph), False + + +def render_deploy_md( + groups: list[tuple[str, list[str]]], + pipeline_deps: dict[str, set[str]], + *, + single_bundle: bool, + packaging_mode: str = "per-pipeline", +) -> str: + """Renders ``DEPLOY.md`` for a packaging run. + + Args: + groups: ``(bundle_dir_name, [pipeline_keys])`` for every bundle written, in write order. + pipeline_deps: ``pipeline_key -> {callee pipeline keys}`` from + :func:`flowx.bundler.pipeline_graph.build_pipeline_dependencies`. + single_bundle: True when the run produced one bundle at the output root (no subdirectory). + packaging_mode: The ``--packaging-mode`` used, surfaced for context. + + Returns: + The full Markdown document. + """ + bundle_graph = _bundle_dependencies(groups, pipeline_deps) + order, acyclic = _bundle_deploy_order(bundle_graph) + + lines: list[str] = [ + "# Deploy", + "", + f"This migration produced **{len(groups)} bundle(s)** with packaging mode `{packaging_mode}`.", + "", + ] + + if single_bundle: + bundle_dir, pipeline_keys = groups[0] + lines += [ + "All pipelines are packaged into a **single bundle** at the migration output root. Deploy it " + "directly — there is no cross-bundle ordering to worry about:", + "", + "```bash", + "databricks bundle validate -t dev", + "databricks bundle deploy -t dev", + "```", + "", + f"Pipelines in this bundle: {', '.join(sorted(pipeline_keys))}.", + "", + ] + return "\n".join(lines) + + if not acyclic: + lines += [ + "> **Warning:** the Run Pipeline call graph between these bundles is **cyclic**, so no valid " + "deploy order exists. The list below is sorted by name, not by dependency. Break the cycle " + "(or package the cyclic pipelines into one bundle with `--packaging-mode single`/`per-group`) " + "before deploying.", + "", + ] + + lines += [ + "## Suggested deploy order", + "", + "Deploy callees before their callers so each caller's `${var.}` job-id reference can be " + "resolved. The automated deployer does this for you (see below); this order is for manual " + "deploys.", + "", + ] + for position, bundle_dir in enumerate(order, start=1): + deps = sorted(bundle_graph.get(bundle_dir, set())) + suffix = f" — depends on: {', '.join(deps)}" if deps else "" + lines.append(f"{position}. `{bundle_dir}/`{suffix}") + lines.append("") + + lines += ["## Bundles", ""] + pipelines_by_bundle = dict(groups) + for bundle_dir in order: + pipeline_keys = pipelines_by_bundle.get(bundle_dir, []) + lines.append(f"### `{bundle_dir}/`") + lines.append("") + lines.append(f"Pipelines: {', '.join(sorted(pipeline_keys))}") + deps = sorted(bundle_graph.get(bundle_dir, set())) + if deps: + lines.append("") + lines.append(f"Cross-bundle dependencies (`${{var.…}}`): {', '.join(deps)}") + lines.append("") + + lines += [ + "## Automated ordered deploy", + "", + "Rather than deploying each bundle by hand, use the ordered deployer, which discovers these " + "bundles, deploys callees first, and injects each callee's deployed job id into its callers " + "automatically:", + "", + "```bash", + "python -m flowx.adapter deploy --output-dir . --target dev", + "```", + "", + "> Local CLI only — `databricks bundle deploy`/`summary` are unavailable on Databricks " + "serverless / Genie Code. Run this from a local CLI session or the web terminal.", + "", + ] + return "\n".join(lines) diff --git a/src/flowx/bundler/deployer.py b/src/flowx/bundler/deployer.py new file mode 100644 index 0000000..d2bf546 --- /dev/null +++ b/src/flowx/bundler/deployer.py @@ -0,0 +1,394 @@ +"""Ordered deploy of the per-pipeline bundles a multi-pipeline migration produces. + +flowx emits **one Databricks Asset Bundle per ADF pipeline**. When pipeline A calls pipeline B via +``ExecutePipeline``, the generated ``run_job_task.job_id`` references B — a job that lives in B's own +bundle. ``_rewrite_cross_bundle_run_job_refs`` (see :mod:`flowx.bundler.dab_writer`) rewrites those +out-of-bundle references to ``${var.}`` and declares a matching bundle variable, so each bundle is +deploy-valid on its own; the operator otherwise has to discover B's numeric job id and pass it by hand. + +This module automates that. It **discovers** the bundles under an output directory (no manifest +needed), reads each bundle's job resource keys and its ``${var.}`` cross-bundle dependencies +straight from the generated YAML, topologically orders them (callees first), and deploys each with +``databricks bundle deploy``. After every deploy it reads the deployed job id from +``databricks bundle summary -o json`` and injects it into callers via ``--var "="``. + +Numeric ids (not names) are captured and injected, so dev-mode ``[dev ]`` job-name prefixes are +irrelevant — this works identically for ``dev`` and ``prod`` targets. + +This is a **local CLI** operation: it shells out to the ``databricks`` CLI and needs a real profile. +``databricks bundle deploy`` is not available on Databricks serverless / Genie Code, so it does not +run on the hosted MCP server. +""" + +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +from pathlib import Path +from typing import Any + +import yaml + +# The cross-bundle variable form dab_writer writes into a run_job_task.job_id: ${var.}, where +# is `_job_id` (with a `_` disambiguation suffix on the rare collision). +_VAR_REF = re.compile(r"\$\{var\.([A-Za-z0-9_]+)\}") +# Recover the callee's job resource key from the variable name by stripping the `_job_id[_]` suffix. +# Names without that suffix are returned unchanged (tolerates the older bare `${var.}` form). +_VAR_JOB_ID_SUFFIX = re.compile(r"_job_id(?:_\d+)?$") + + +def _callee_key_from_var(var_name: str) -> str: + """Maps a cross-bundle variable name (``_job_id``) back to the callee's job resource key.""" + return _VAR_JOB_ID_SUFFIX.sub("", var_name) + + +class CycleError(Exception): + """Raised when the bundle dependency graph contains a cycle (cannot be ordered).""" + + +class MissingDependencyError(Exception): + """Raised when a bundle depends on a callee no discovered bundle provides.""" + + +class DiscoveredBundle: + """One per-pipeline bundle found under the output directory. + + Attributes: + bundle_dir: Directory name (relative to the output dir), e.g. ``ingest_sales``. + resource_keys: Job resource keys this bundle defines (parent + inner ForEach jobs). + depends_on: Maps each cross-bundle callee's job resource key -> the ``${var.}`` variable + name that references it (``_job_id``). Deploy resolves ordering on the callee key + and injects ``--var =`` using the variable name. + """ + + __slots__ = ("bundle_dir", "resource_keys", "depends_on") + + def __init__(self, bundle_dir: str, resource_keys: set[str], depends_on: dict[str, str]) -> None: + self.bundle_dir = bundle_dir + self.resource_keys = resource_keys + self.depends_on = depends_on + + +def _iter_run_job_ids(node: Any) -> Any: + """Yields every ``run_job_task.job_id`` string anywhere in a parsed resource YAML tree.""" + if isinstance(node, dict): + run_job = node.get("run_job_task") + if isinstance(run_job, dict) and "job_id" in run_job: + yield str(run_job["job_id"]) + for value in node.values(): + yield from _iter_run_job_ids(value) + elif isinstance(node, list): + for item in node: + yield from _iter_run_job_ids(item) + + +def _read_bundle_dir(bundle_dir: Path, bundle_name: str) -> DiscoveredBundle: + """Reads one bundle's job resource keys and ``${var.}`` deps from its ``resources/*.yml``.""" + resource_keys: set[str] = set() + depends_on: dict[str, str] = {} + resources_dir = bundle_dir / "resources" + if resources_dir.is_dir(): + for resource_yml in sorted(resources_dir.glob("*.yml")): + try: + doc = yaml.safe_load(resource_yml.read_text()) or {} + except yaml.YAMLError: + continue + jobs = ((doc.get("resources") or {}).get("jobs")) or {} + resource_keys.update(jobs.keys()) + for job_id in _iter_run_job_ids(doc): + match = _VAR_REF.fullmatch(job_id) + if match: + var_name = match.group(1) + depends_on[_callee_key_from_var(var_name)] = var_name + return DiscoveredBundle(bundle_name, resource_keys, depends_on) + + +def _discover_bundles(output_dir: Path) -> list[DiscoveredBundle]: + """Finds every bundle under *output_dir* and reads its jobs + cross-bundle deps. + + A bundle is any immediate subdirectory containing a ``databricks.yml`` (the ``per-pipeline`` / + ``per-group`` layout). When *output_dir* itself holds a ``databricks.yml`` (the ``single`` mode / + single-pipeline layout, where the sole bundle sits at the root), that root bundle is returned + instead — a single root bundle has no siblings to order against, so it is deployed directly. Its + ``bundle_dir`` is ``"."`` so ``run`` shells out in *output_dir* itself. + """ + if (output_dir / "databricks.yml").exists(): + return [_read_bundle_dir(output_dir, ".")] + + bundles: list[DiscoveredBundle] = [] + for child in sorted(output_dir.iterdir()): + if not child.is_dir() or not (child / "databricks.yml").exists(): + continue + bundles.append(_read_bundle_dir(child, child.name)) + return bundles + + +def _build_graph(bundles: list[DiscoveredBundle], *, allow_missing_deps: bool = False) -> dict[str, list[str]]: + """Builds a ``bundle_dir -> [dependency bundle_dirs]`` adjacency map. + + A ``${var.}`` dependency names a job resource key, which may be a bundle's parent job or an + inner ForEach job — resolve it to the owning bundle's directory. + + Raises: + MissingDependencyError: when a callee resource key belongs to no discovered bundle and + ``allow_missing_deps`` is False. + """ + key_to_dir: dict[str, str] = {} + for bundle in bundles: + for key in bundle.resource_keys: + key_to_dir[key] = bundle.bundle_dir + + graph: dict[str, list[str]] = {} + for bundle in bundles: + deps: list[str] = [] + for callee in sorted(bundle.depends_on): + owner = key_to_dir.get(callee) + if owner is None: + if not allow_missing_deps: + raise MissingDependencyError( + f"Bundle '{bundle.bundle_dir}' references ${{var.{callee}}}, but no bundle under " + "the output directory defines a job named '" + f"{callee}'. Pass --allow-missing-deps to deploy anyway (set ${{var.{callee}}} " + "manually per SETUP.md)." + ) + continue + if owner != bundle.bundle_dir and owner not in deps: + deps.append(owner) + graph[bundle.bundle_dir] = deps + return graph + + +def _topo_sort(graph: dict[str, list[str]]) -> list[str]: + """Returns bundle dirs in dependency-first order (Kahn's algorithm). + + Raises: + CycleError: when the graph has a cycle. + """ + in_degree = {node: len(deps) for node, deps in graph.items()} + dependents: dict[str, list[str]] = {node: [] for node in graph} + for node, deps in graph.items(): + for dep in deps: + dependents[dep].append(node) + + queue = sorted(node for node, deg in in_degree.items() if deg == 0) + ordered: list[str] = [] + while queue: + node = queue.pop(0) + ordered.append(node) + for dependent in sorted(dependents[node]): + in_degree[dependent] -= 1 + if in_degree[dependent] == 0: + queue.append(dependent) + queue.sort() + + if len(ordered) != len(graph): + remaining = sorted(node for node in graph if node not in ordered) + raise CycleError( + "Cyclic dependency between bundles: " + ", ".join(remaining) + ". " + "Cyclic factories cannot be deployed as ordered separate bundles." + ) + return ordered + + +def _run_cli(cmd: list[str], *, cwd: Path) -> subprocess.CompletedProcess[str]: + """Runs a CLI command, capturing output. Isolated so tests can monkeypatch it.""" + return subprocess.run(cmd, cwd=str(cwd), capture_output=True, text=True) + + +def _capture_job_ids( + bundle_dir: Path, + resource_keys: set[str], + *, + target: str, + profile: str | None, + var_pairs: dict[str, int], +) -> dict[str, int]: + """Reads deployed job ids for *resource_keys* from ``databricks bundle summary -o json``. + + Args: + var_pairs: The same ``{"": }`` cross-bundle vars this bundle was deployed with. + ``bundle summary`` re-resolves the config and errors on any unset required variable, so + these must be re-passed. + + Returns: + ``{"": }`` for every key whose id was found. Keys without a deployed + id (e.g. a pipeline resource, not a job) are skipped so no empty ``--var`` is ever injected. + """ + cmd = ["databricks", "bundle", "summary", "-o", "json", "-t", target] + if profile: + cmd += ["-p", profile] + for var_name, job_id in sorted(var_pairs.items()): + cmd += ["--var", f"{var_name}={job_id}"] + result = _run_cli(cmd, cwd=bundle_dir) + if result.returncode != 0: + print(f" warning: `bundle summary` failed for {bundle_dir.name}: {result.stderr.strip()}", file=sys.stderr) + return {} + try: + summary = json.loads(result.stdout) + except json.JSONDecodeError: + print(f" warning: could not parse `bundle summary` JSON for {bundle_dir.name}", file=sys.stderr) + return {} + + jobs = ((summary.get("resources") or {}).get("jobs")) or {} + captured: dict[str, int] = {} + for key in resource_keys: + job: dict[str, Any] = jobs.get(key) or {} + deployed_id: Any = job.get("id") + if deployed_id is not None: + captured[key] = int(deployed_id) + return captured + + +def run( + output_dir: Path, + *, + target: str = "dev", + profile: str | None = None, + dry_run: bool = False, + allow_missing_deps: bool = False, +) -> int: + """Deploys every bundle under *output_dir* in dependency order. Returns a process exit code.""" + output_dir = Path(output_dir) + if not output_dir.is_dir(): + print(f"Error: output directory not found: {output_dir}", file=sys.stderr) + return 1 + + bundles = _discover_bundles(output_dir) + if not bundles: + print( + f"No bundles found under {output_dir} (looked for immediate subdirectories with a " + "databricks.yml). Package with the multi-pipeline output first.", + file=sys.stderr, + ) + return 1 + + by_dir = {bundle.bundle_dir: bundle for bundle in bundles} + try: + graph = _build_graph(bundles, allow_missing_deps=allow_missing_deps) + order = _topo_sort(graph) + except (CycleError, MissingDependencyError) as exc: + print(f"Error: {exc}", file=sys.stderr) + return 1 + + print(f"Deploy order ({len(order)} bundle(s), target={target}): {' -> '.join(order)}") + + # Every captured "": from bundles deployed so far. + deployed_ids: dict[str, int] = {} + + for position, bundle_dir in enumerate(order, start=1): + bundle = by_dir[bundle_dir] + # Only pass the vars this bundle references. Keyed by the actual bundle-variable name + # (``_job_id``) so ``--var =`` matches the declaration in databricks.yml, with + # the id looked up by the callee's job resource key. + needed = { + var_name: deployed_ids[callee] + for callee, var_name in sorted(bundle.depends_on.items()) + if callee in deployed_ids + } + + # Every in-migration callee this bundle depends on was deployed earlier in topological order, so + # its id should be in deployed_ids. A missing one means the callee's `bundle summary` id-capture + # failed; deploying now would abort cryptically on an unset required ${var.}. Fail here + # with an actionable message instead. (allow_missing_deps skips callees that no bundle provides; + # those were already dropped from bundle.depends_on's resolved deps in _build_graph.) + if not dry_run: + uncaptured = sorted( + callee + for callee in bundle.depends_on + if callee not in deployed_ids and any(callee in b.resource_keys for b in bundles) + ) + if uncaptured: + print(f" [{position}/{len(order)}] {bundle_dir}: BLOCKED", file=sys.stderr) + print( + f"Cannot deploy '{bundle_dir}': its cross-bundle job id(s) {', '.join(uncaptured)} were " + "not captured from the callee bundle's `databricks bundle summary` (deploy may have " + "succeeded but the summary read failed). Re-run the deploy, or set the " + f"${{var.}} value(s) manually and deploy '{bundle_dir}' with " + "`databricks bundle deploy --var =`.", + file=sys.stderr, + ) + return 1 + + cmd = ["databricks", "bundle", "deploy", "-t", target] + if profile: + cmd += ["-p", profile] + for var_name, job_id in sorted(needed.items()): + cmd += ["--var", f"{var_name}={job_id}"] + + if dry_run: + shown = " ".join(cmd) + if bundle.depends_on and not needed: + shown += " # + --var =" + print(f" [{position}/{len(order)}] {bundle_dir}: {shown}") + continue + + result = _run_cli(cmd, cwd=output_dir / bundle_dir) + if result.returncode != 0: + print(f" [{position}/{len(order)}] {bundle_dir}: FAILED", file=sys.stderr) + print(result.stderr.strip(), file=sys.stderr) + print(f"Stopped: {bundle_dir} failed to deploy; dependent bundles were not deployed.", file=sys.stderr) + return 1 + + captured = _capture_job_ids( + output_dir / bundle_dir, + bundle.resource_keys, + target=target, + profile=profile, + var_pairs=needed, + ) + deployed_ids.update(captured) + ids_note = ", ".join(f"{k}={v}" for k, v in sorted(captured.items())) or "no job ids captured" + print(f" [{position}/{len(order)}] {bundle_dir}: deployed ({ids_note})") + + if dry_run: + print("\nDry run — no bundles were deployed.") + else: + print(f"\nDeployed {len(order)} bundle(s) to target '{target}'.") + return 0 + + +def main(argv: list[str] | None = None) -> int: + """CLI entry point for ordered multi-bundle deploy.""" + parser = argparse.ArgumentParser( + description="Deploy per-pipeline flowx bundles in dependency order, wiring cross-bundle job ids.", + ) + parser.add_argument( + "--output-dir", + type=Path, + default=Path("./flowx_output"), + help="Directory holding the per-pipeline bundle subdirectories.", + ) + parser.add_argument("--target", type=str, default="dev", help="Bundle target to deploy (default: dev).") + parser.add_argument("--profile", type=str, default=None, help="Databricks CLI profile for deploy and summary.") + parser.add_argument( + "--dry-run", + action="store_true", + help="Print the dependency order and deploy commands without deploying.", + ) + parser.add_argument( + "--allow-missing-deps", + action="store_true", + help=( + "Order and attempt to deploy even when a bundle references a callee absent from the output " + "dir. The missing ${var.} is declared without a default, so that bundle's deploy " + "still fails until you supply the value manually (edit its databricks.yml default or " + "`databricks bundle deploy --var =` per SETUP.md); this flag only unblocks " + "the ordering, not the deploy." + ), + ) + args = parser.parse_args(argv) + + return run( + args.output_dir, + target=args.target, + profile=args.profile, + dry_run=args.dry_run, + allow_missing_deps=args.allow_missing_deps, + ) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/flowx/bundler/pipeline_graph.py b/src/flowx/bundler/pipeline_graph.py new file mode 100644 index 0000000..65ead8c --- /dev/null +++ b/src/flowx/bundler/pipeline_graph.py @@ -0,0 +1,175 @@ +"""Pipeline-level Run Pipeline (ExecutePipeline) dependency graph. + +An ADF ``ExecutePipeline`` activity is prepared as a ``run_job_task`` whose ``job_id`` is +``${resources.jobs..id}`` (see :func:`flowx.preparer.activity_preparers.execute_pipeline`), +where ```` is the normalized name of the called pipeline. This module reads those refs back +off the prepared workflow task trees to reconstruct the caller -> callee graph *before* any bundle +is written. + +The graph is the single source of truth for two packaging decisions: + +- **Grouping** (``per-group`` inferred mode): pipelines that call one another belong in the same + bundle. :func:`connected_components` returns those clusters. +- **Deploy order** (top-level ``DEPLOY.md``): :func:`topo_order` returns callees-before-callers so + the generated ``DEPLOY.md`` can suggest the order the operator (or ``flowx.adapter deploy``) + should deploy in. + +Keys throughout are the normalized pipeline resource keys (:func:`flowx.utils.normalize_task_key`), +matching the job resource keys emitted into ``resources/*.yml`` and the ``${resources.jobs.X.id}`` / +``${var.X}`` refs, so they line up 1:1 with what :mod:`flowx.bundler.deployer` discovers at deploy +time. +""" + +from __future__ import annotations + +import re +from typing import TYPE_CHECKING, Any + +from flowx.utils import normalize_task_key + +if TYPE_CHECKING: + from flowx.preparer.workflow_preparer import PreparedWorkflow + +# The ref shape execute_pipeline.prepare emits: ${resources.jobs..id}. Shared with +# dab_writer._rewrite_cross_bundle_run_job_refs (imported there) so the two never drift. +CROSS_BUNDLE_JOB_ID_REF = re.compile(r"\$\{resources\.jobs\.([^.]+)\.id\}") + + +class PipelineCycleError(Exception): + """Raised when the Run Pipeline dependency graph contains a cycle (cannot be ordered).""" + + +def _iter_run_job_targets(tasks: list[dict[str, Any]]) -> list[str]: + """Returns the callee key of every ``run_job_task`` in *tasks*, descending into ForEach bodies.""" + targets: list[str] = [] + + def visit(task: dict[str, Any]) -> None: + run_job = task.get("run_job_task") + if isinstance(run_job, dict): + match = CROSS_BUNDLE_JOB_ID_REF.fullmatch(str(run_job.get("job_id", ""))) + if match: + targets.append(match.group(1)) + for_each = task.get("for_each_task") + if isinstance(for_each, dict) and isinstance(for_each.get("task"), dict): + visit(for_each["task"]) + + for task in tasks: + visit(task) + return targets + + +def _workflow_own_keys(workflow: PreparedWorkflow) -> set[str]: + """Returns the job resource keys a workflow owns: its own key plus every inner ForEach job key.""" + keys = {normalize_task_key(workflow.name)} + keys.update(normalize_task_key(inner.name) for inner in workflow.inner_workflows) + return keys + + +def build_pipeline_dependencies(workflows: list[PreparedWorkflow]) -> dict[str, set[str]]: + """Builds a ``pipeline_key -> {callee pipeline keys}`` map from Run Pipeline refs. + + Scans each workflow's task tree (and its inner ForEach jobs') for ``run_job_task`` refs of the + form ``${resources.jobs.X.id}`` and records ``X`` as a dependency of the *owning* pipeline. Refs + to a workflow's own keys (its inner ForEach jobs) are self-edges and dropped — they are within + one bundle and never affect grouping or deploy order. Refs to keys no workflow provides are kept + (an ExecutePipeline to a pipeline outside this migration); callers decide how to treat them. + + Returns: + A dict with one entry per workflow (keyed by its normalized name), whose value is the set of + other pipeline keys it calls. Every workflow appears as a key, even with no dependencies. + """ + key_by_workflow = {normalize_task_key(wf.name): wf for wf in workflows} + deps: dict[str, set[str]] = {key: set() for key in key_by_workflow} + + for workflow in workflows: + owner = normalize_task_key(workflow.name) + own_keys = _workflow_own_keys(workflow) + callees = list(_iter_run_job_targets(workflow.tasks)) + for inner in workflow.inner_workflows: + callees.extend(_iter_run_job_targets(inner.tasks)) + for callee in callees: + if callee in own_keys: + continue + deps[owner].add(callee) + return deps + + +def connected_components(deps: dict[str, set[str]]) -> list[list[str]]: + """Returns clusters of pipelines connected (in either direction) by Run Pipeline calls. + + Dependencies are treated as **undirected** edges: a pipeline and everything it transitively + calls or is called by land in one component. Only keys present in *deps* participate; callee + keys not in *deps* (calls to pipelines outside the migration) are ignored for grouping. + + Returns: + A list of components, each a sorted list of pipeline keys. Components are ordered by their + smallest member so the result is deterministic. + """ + nodes = set(deps) + adjacency: dict[str, set[str]] = {node: set() for node in nodes} + for node, callees in deps.items(): + for callee in callees: + if callee not in nodes: + continue + adjacency[node].add(callee) + adjacency[callee].add(node) + + seen: set[str] = set() + components: list[list[str]] = [] + for start in sorted(nodes): + if start in seen: + continue + stack = [start] + component: set[str] = set() + while stack: + node = stack.pop() + if node in component: + continue + component.add(node) + stack.extend(adjacency[node] - component) + seen |= component + components.append(sorted(component)) + + components.sort(key=lambda members: members[0]) + return components + + +def topo_order(deps: dict[str, set[str]]) -> list[str]: + """Returns pipeline keys callees-first (a called pipeline precedes its caller). + + Uses Kahn's algorithm over only the keys present in *deps* (callee keys outside the migration + are ignored, matching :func:`connected_components`). Deterministic: ready nodes are drained in + sorted order. + + Raises: + PipelineCycleError: when the Run Pipeline graph has a cycle (cyclic factories cannot be + ordered into a deploy sequence). + """ + nodes = set(deps) + # in_degree[node] = number of callees node waits on (within the migration). + in_degree = {node: len({c for c in callees if c in nodes}) for node, callees in deps.items()} + # callers[callee] = nodes that depend on callee (edges to decrement once callee is placed). + callers: dict[str, list[str]] = {node: [] for node in nodes} + for node, callees in deps.items(): + for callee in callees: + if callee in nodes: + callers[callee].append(node) + + queue = sorted(node for node, degree in in_degree.items() if degree == 0) + ordered: list[str] = [] + while queue: + node = queue.pop(0) + ordered.append(node) + for caller in sorted(callers[node]): + in_degree[caller] -= 1 + if in_degree[caller] == 0: + queue.append(caller) + queue.sort() + + if len(ordered) != len(nodes): + remaining = sorted(node for node in nodes if node not in ordered) + raise PipelineCycleError( + "Cyclic Run Pipeline dependency between: " + ", ".join(remaining) + ". " + "A cyclic call graph cannot be ordered into a deploy sequence." + ) + return ordered diff --git a/src/flowx/bundler/prereqs_writer.py b/src/flowx/bundler/prereqs_writer.py index 3641b66..cc03ad9 100644 --- a/src/flowx/bundler/prereqs_writer.py +++ b/src/flowx/bundler/prereqs_writer.py @@ -142,6 +142,10 @@ class Prereqs: # Each entry is a human-readable identifier (the pipeline name, or ``index N`` when unnamed) so a # skipped pipeline is surfaced rather than silently missing from the bundle. skipped_pipelines: list[str] = field(default_factory=list) + # Job parameters emitted with a synthetic ``default: ""`` because the ADF pipeline parameter had no + # default (it was supplied by the caller at ExecutePipeline/trigger time). Each entry is + # ``{job, name}``; the operator must override these at run/deploy time or the run gets "". + synthetic_default_parameters: list[dict[str, str]] = field(default_factory=list) def is_empty(self) -> bool: """Return ``True`` when nothing needs to happen before ``bundle run``.""" @@ -165,6 +169,7 @@ def is_empty(self) -> bool: and not self.pydabs_dbt_factories and not self.airflow_backfills and not self.skipped_pipelines + and not self.synthetic_default_parameters ) @@ -380,6 +385,7 @@ def build_prereqs( pydabs_dbt_factories: list[dict[str, Any]] | None = None, airflow_backfills: list[dict[str, Any]] | None = None, skipped_pipelines: list[str] | None = None, + synthetic_default_parameters: list[dict[str, str]] | None = None, ) -> Prereqs: """Assemble a :class:`Prereqs` from the bundle's generated artifacts. @@ -431,6 +437,7 @@ def build_prereqs( pydabs_dbt_factories=list(pydabs_dbt_factories or []), airflow_backfills=list(airflow_backfills or []), skipped_pipelines=list(skipped_pipelines or []), + synthetic_default_parameters=list(synthetic_default_parameters or []), ) @@ -588,6 +595,24 @@ def render_setup_md(prereqs: Prereqs, *, bundle_name: str) -> str: lines.append(f"| `{empty_parameter.task_key}` | `{empty_parameter.widget_name}` |") lines.append("") + if prereqs.synthetic_default_parameters: + lines.append("## Job parameters without an ADF default") + lines.append("") + lines.append( + "The job parameters below had no default in the source ADF pipeline — they were supplied " + "by the caller at ExecutePipeline (or trigger) time. A DAB job parameter must declare a " + '`default`, so flowx emitted `default: ""` to keep the bundle deploy-valid. **Override ' + "each one** at run time (`databricks bundle run --params '{:}'`) or set " + "a real default in the job YAML — a run that omits it receives an empty string rather than " + "failing fast." + ) + lines.append("") + lines.append("| Job | Parameter |") + lines.append("|---|---|") + for parameter in prereqs.synthetic_default_parameters: + lines.append(f"| `{parameter.get('job', '')}` | `{parameter.get('name', '')}` |") + lines.append("") + if prereqs.compute_notes: lines.append("## Compute configuration") lines.append("") diff --git a/src/flowx/sources/adf/loader.py b/src/flowx/sources/adf/loader.py index af9917a..4248108 100644 --- a/src/flowx/sources/adf/loader.py +++ b/src/flowx/sources/adf/loader.py @@ -59,6 +59,8 @@ "ExecuteDataFlow", "Until", "SqlServerStoredProcedure", + # ADF exports the type as "AzureFunctionActivity"; the bare alias covers pre-normalized inputs. + "AzureFunctionActivity", "AzureFunction", "WebHook", "Custom", diff --git a/tests/unit/test_adapter.py b/tests/unit/test_adapter.py index 910ec7a..5b0f83d 100644 --- a/tests/unit/test_adapter.py +++ b/tests/unit/test_adapter.py @@ -700,6 +700,30 @@ def test_inputs_writes_to_file(self, tmp_path: Path): class TestCli: + def test_deploy_dispatches_to_deployer(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """`adapter deploy` routes to the deployer with parsed flags and returns its exit code.""" + import flowx.bundler.deployer as deployer + + captured: dict[str, Any] = {} + + def fake_run(output_dir, *, target, profile, dry_run, allow_missing_deps): + captured.update( + output_dir=output_dir, + target=target, + profile=profile, + dry_run=dry_run, + allow_missing_deps=allow_missing_deps, + ) + return 0 + + monkeypatch.setattr(deployer, "run", fake_run) + exit_code = adapter_cli_main(["deploy", "--output-dir", str(tmp_path), "--target", "prod", "--dry-run"]) + assert exit_code == 0 + assert captured["output_dir"] == tmp_path + assert captured["target"] == "prod" + assert captured["dry_run"] is True + assert captured["allow_missing_deps"] is False + def test_inspect_emits_pending_options(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]): from flowx.ir_serde import pipeline_to_dict diff --git a/tests/unit/test_deploy_writer.py b/tests/unit/test_deploy_writer.py new file mode 100644 index 0000000..2aaf648 --- /dev/null +++ b/tests/unit/test_deploy_writer.py @@ -0,0 +1,61 @@ +"""Unit tests for DEPLOY.md rendering.""" + +from __future__ import annotations + +from flowx.bundler.deploy_writer import render_deploy_md + + +class TestRenderDeployMd: + def test_single_bundle_has_no_ordering_section(self): + md = render_deploy_md( + [("flowx_bundle", ["a", "b"])], + {"a": {"b"}, "b": set()}, + single_bundle=True, + packaging_mode="single", + ) + assert "single bundle" in md + assert "Suggested deploy order" not in md + assert "databricks bundle deploy -t dev" in md + + def test_multi_bundle_orders_callees_first(self): + groups = [("caller", ["caller"]), ("callee", ["callee"])] + deps = {"caller": {"callee"}, "callee": set()} + md = render_deploy_md(groups, deps, single_bundle=False, packaging_mode="per-pipeline") + assert md.index("`callee/`") < md.index("`caller/`") + # The caller lists its cross-bundle dependency. + assert "depends on: callee" in md + + def test_order_is_dependency_not_alphabetical(self): + # 'aaa_root' sorts first but is the callee of 'zzz_leaf'; it must still deploy first, proving + # the order follows the dependency graph rather than the (alphabetical) group order. + groups = [("zzz_leaf", ["zzz_leaf"]), ("aaa_root", ["aaa_root"])] + deps = {"zzz_leaf": {"aaa_root"}, "aaa_root": set()} + md = render_deploy_md(groups, deps, single_bundle=False, packaging_mode="per-pipeline") + assert md.index("`aaa_root/`") < md.index("`zzz_leaf/`") + + def test_multi_bundle_points_at_automated_deployer(self): + md = render_deploy_md( + [("a", ["a"]), ("b", ["b"])], + {"a": {"b"}, "b": set()}, + single_bundle=False, + packaging_mode="per-pipeline", + ) + assert "python -m flowx.adapter deploy" in md + assert "serverless" in md + + def test_cycle_is_flagged(self): + md = render_deploy_md( + [("a", ["a"]), ("b", ["b"])], + {"a": {"b"}, "b": {"a"}}, + single_bundle=False, + packaging_mode="per-pipeline", + ) + assert "cyclic" in md.lower() + + def test_grouped_bundle_lists_all_member_pipelines(self): + groups = [("grp", ["p1", "p2"]), ("other", ["other"])] + deps = {"p1": {"p2"}, "p2": {"other"}, "other": set()} + md = render_deploy_md(groups, deps, single_bundle=False, packaging_mode="per-group") + assert "p1, p2" in md + # p1->p2 is intra-bundle (both in grp) so grp only depends on 'other'. + assert "depends on: other" in md diff --git a/tests/unit/test_deployer.py b/tests/unit/test_deployer.py new file mode 100644 index 0000000..f5521cc --- /dev/null +++ b/tests/unit/test_deployer.py @@ -0,0 +1,235 @@ +"""Unit tests for the ordered multi-bundle deployer. + +The deploy/summary subprocess (``databricks bundle …``) is always mocked — these tests never touch a +real workspace. +""" + +from __future__ import annotations + +import json +import subprocess + +import pytest +import yaml + +from flowx.bundler import deployer +from flowx.bundler.deployer import ( + CycleError, + MissingDependencyError, + _build_graph, + _discover_bundles, + _topo_sort, + run, +) + + +def _make_bundle(root, bundle_dir, *, jobs, deps=None, var_suffix=""): + """Writes a minimal per-pipeline bundle: databricks.yml + one resource YAML. + + Args: + jobs: resource keys (job names) this bundle defines. + deps: resource keys of sibling jobs to reference via ${var.} in a run_job_task. + var_suffix: appended to the variable name (use "_job_id" to mirror the real dab_writer naming; + default "" uses a bare ${var.}). The deployer strips a _job_id[_] suffix to recover + the callee job key either way. + """ + bdir = root / bundle_dir + (bdir / "resources").mkdir(parents=True) + (bdir / "databricks.yml").write_text("bundle:\n name: " + bundle_dir + "\n") + resources = {"resources": {"jobs": {}}} + for job in jobs: + tasks = [] + for dep in deps or []: + tasks.append({"task_key": f"call_{dep}", "run_job_task": {"job_id": f"${{var.{dep}{var_suffix}}}"}}) + resources["resources"]["jobs"][job] = {"name": job, "tasks": tasks} + (bdir / "resources" / f"{jobs[0]}.yml").write_text(yaml.safe_dump(resources)) + + +# --------------------------------------------------------------------------- +# Discovery +# --------------------------------------------------------------------------- + + +class TestDiscovery: + def test_reads_jobs_and_var_dependencies(self, tmp_path): + _make_bundle(tmp_path, "a", jobs=["a"], deps=["b"]) + _make_bundle(tmp_path, "b", jobs=["b"]) + bundles = {bundle.bundle_dir: bundle for bundle in _discover_bundles(tmp_path)} + assert set(bundles) == {"a", "b"} + assert bundles["a"].resource_keys == {"a"} + # depends_on maps callee job key -> the ${var.} variable name referencing it. The helper + # emits a bare ${var.b} (no _job_id suffix), so key and var name coincide here. + assert bundles["a"].depends_on == {"b": "b"} + assert bundles["b"].depends_on == {} + + def test_job_id_suffixed_var_maps_to_callee_key(self, tmp_path): + # dab_writer emits ${var._job_id}; the deployer must map that back to callee key 'b' + # (so ordering resolves) while remembering the real variable name 'b_job_id' for --var injection. + _make_bundle(tmp_path, "a", jobs=["a"], deps=["b"], var_suffix="_job_id") + _make_bundle(tmp_path, "b", jobs=["b"]) + bundles = {bundle.bundle_dir: bundle for bundle in _discover_bundles(tmp_path)} + assert bundles["a"].depends_on == {"b": "b_job_id"} + + def test_ignores_non_bundle_dirs(self, tmp_path): + _make_bundle(tmp_path, "a", jobs=["a"]) + (tmp_path / "metadata").mkdir() # not a bundle (no databricks.yml) + (tmp_path / "deploy_manifest.json").write_text("{}") # stray file + assert {b.bundle_dir for b in _discover_bundles(tmp_path)} == {"a"} + + def test_inner_job_keys_collected(self, tmp_path): + # A bundle whose resource file defines a parent + an inner ForEach job. + _make_bundle(tmp_path, "b", jobs=["b", "b_foreach_body"]) + (bundle,) = _discover_bundles(tmp_path) + assert bundle.resource_keys == {"b", "b_foreach_body"} + + def test_single_mode_root_bundle_discovered(self, tmp_path): + # single-mode / single-pipeline layout: the sole bundle sits at output_dir root, not a subdir. + _make_bundle(tmp_path, ".", jobs=["combined", "other"]) + (bundle,) = _discover_bundles(tmp_path) + assert bundle.bundle_dir == "." + assert bundle.resource_keys == {"combined", "other"} + assert bundle.depends_on == {} + + +# --------------------------------------------------------------------------- +# Graph + topological sort +# --------------------------------------------------------------------------- + + +class TestTopoSort: + def test_dependency_first_order(self, tmp_path): + _make_bundle(tmp_path, "a", jobs=["a"], deps=["b"]) + _make_bundle(tmp_path, "b", jobs=["b"], deps=["c"]) + _make_bundle(tmp_path, "c", jobs=["c"]) + assert _topo_sort(_build_graph(_discover_bundles(tmp_path))) == ["c", "b", "a"] + + def test_inner_key_resolves_to_owning_bundle(self, tmp_path): + # a depends on b_inner, an inner job of bundle b. + _make_bundle(tmp_path, "a", jobs=["a"], deps=["b_inner"]) + _make_bundle(tmp_path, "b", jobs=["b", "b_inner"]) + assert _topo_sort(_build_graph(_discover_bundles(tmp_path))) == ["b", "a"] + + def test_cycle_raises(self, tmp_path): + _make_bundle(tmp_path, "a", jobs=["a"], deps=["b"]) + _make_bundle(tmp_path, "b", jobs=["b"], deps=["a"]) + with pytest.raises(CycleError): + _topo_sort(_build_graph(_discover_bundles(tmp_path))) + + def test_missing_dependency_raises(self, tmp_path): + _make_bundle(tmp_path, "a", jobs=["a"], deps=["ghost"]) + with pytest.raises(MissingDependencyError): + _build_graph(_discover_bundles(tmp_path)) + + def test_missing_dependency_allowed(self, tmp_path): + _make_bundle(tmp_path, "a", jobs=["a"], deps=["ghost"]) + graph = _build_graph(_discover_bundles(tmp_path), allow_missing_deps=True) + assert graph == {"a": []} + + +# --------------------------------------------------------------------------- +# run(): ordering, --var injection, dry-run, failure handling +# --------------------------------------------------------------------------- + + +class _FakeCli: + """Records commands and returns canned deploy/summary results. + + ``summary_ids`` maps a bundle_dir (cwd name) -> {resource_key: job_id} to synthesize the + ``bundle summary`` JSON. ``fail_on`` names a bundle_dir whose deploy returns non-zero. + """ + + def __init__(self, summary_ids=None, fail_on=None): + self.summary_ids = summary_ids or {} + self.fail_on = fail_on + self.commands = [] + + def __call__(self, cmd, *, cwd): + self.commands.append((cmd, str(cwd))) + bundle_dir = str(cwd).rsplit("/", 1)[-1] + if "summary" in cmd: + jobs = {key: {"id": jid} for key, jid in self.summary_ids.get(bundle_dir, {}).items()} + return subprocess.CompletedProcess(cmd, 0, stdout=json.dumps({"resources": {"jobs": jobs}}), stderr="") + if self.fail_on == bundle_dir: + return subprocess.CompletedProcess(cmd, 1, stdout="", stderr=f"boom in {bundle_dir}") + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + + +def test_run_deploys_in_order_and_injects_captured_id(tmp_path, monkeypatch): + _make_bundle(tmp_path, "parent", jobs=["parent"], deps=["child"]) + _make_bundle(tmp_path, "child", jobs=["child"]) + fake = _FakeCli(summary_ids={"child": {"child": 123}}) + monkeypatch.setattr(deployer, "_run_cli", fake) + + assert run(tmp_path, target="dev", profile="myprofile") == 0 + + deploy_cwds = [cwd for cmd, cwd in fake.commands if "deploy" in cmd] + # child deploys before parent. + assert deploy_cwds.index(str(tmp_path / "child")) < deploy_cwds.index(str(tmp_path / "parent")) + + parent_cmd = next(cmd for cmd, cwd in fake.commands if "deploy" in cmd and cwd.endswith("parent")) + assert "--var" in parent_cmd and "child=123" in parent_cmd + assert "-p" in parent_cmd and "myprofile" in parent_cmd + assert "-t" in parent_cmd and "dev" in parent_cmd + # child (no deps) gets no --var. + child_cmd = next(cmd for cmd, cwd in fake.commands if "deploy" in cmd and cwd.endswith("child")) + assert "--var" not in child_cmd + + +def test_summary_receives_dependency_vars(tmp_path, monkeypatch): + """`bundle summary` for a dependent must re-pass its --var or it errors on the unset variable.""" + _make_bundle(tmp_path, "parent", jobs=["parent"], deps=["child"]) + _make_bundle(tmp_path, "child", jobs=["child"]) + fake = _FakeCli(summary_ids={"child": {"child": 55}, "parent": {"parent": 66}}) + monkeypatch.setattr(deployer, "_run_cli", fake) + + assert run(tmp_path) == 0 + parent_summary = next(cmd for cmd, cwd in fake.commands if "summary" in cmd and cwd.endswith("parent")) + assert "--var" in parent_summary and "child=55" in parent_summary + + +def test_dry_run_makes_no_calls(tmp_path, monkeypatch): + _make_bundle(tmp_path, "parent", jobs=["parent"], deps=["child"]) + _make_bundle(tmp_path, "child", jobs=["child"]) + fake = _FakeCli() + monkeypatch.setattr(deployer, "_run_cli", fake) + + assert run(tmp_path, dry_run=True) == 0 + assert fake.commands == [] + + +def test_deploy_failure_stops_before_dependents(tmp_path, monkeypatch): + _make_bundle(tmp_path, "parent", jobs=["parent"], deps=["child"]) + _make_bundle(tmp_path, "child", jobs=["child"]) + fake = _FakeCli(fail_on="child") # child deploys first and fails + monkeypatch.setattr(deployer, "_run_cli", fake) + + assert run(tmp_path) == 1 + deployed = [cwd for cmd, cwd in fake.commands if "deploy" in cmd] + assert not any(cwd.endswith("parent") for cwd in deployed) + + +def test_uncaptured_dependency_id_blocks_caller_with_clear_error(tmp_path, monkeypatch, capsys): + """If a callee's job id isn't captured, the caller is blocked with an actionable message.""" + _make_bundle(tmp_path, "parent", jobs=["parent"], deps=["child"]) + _make_bundle(tmp_path, "child", jobs=["child"]) + # child deploys fine but its summary yields no id (transient summary failure) -> capture miss. + fake = _FakeCli(summary_ids={}) + monkeypatch.setattr(deployer, "_run_cli", fake) + + assert run(tmp_path) == 1 + err = capsys.readouterr().err + assert "BLOCKED" in err + assert "child" in err + # parent's deploy was never attempted. + assert not any("deploy" in cmd and cwd.endswith("parent") for cmd, cwd in fake.commands) + + +def test_empty_output_dir_returns_error(tmp_path): + assert run(tmp_path) == 1 + + +def test_cycle_returns_error(tmp_path, monkeypatch): + _make_bundle(tmp_path, "a", jobs=["a"], deps=["b"]) + _make_bundle(tmp_path, "b", jobs=["b"], deps=["a"]) + monkeypatch.setattr(deployer, "_run_cli", _FakeCli()) + assert run(tmp_path) == 1 diff --git a/tests/unit/test_packaging_modes.py b/tests/unit/test_packaging_modes.py new file mode 100644 index 0000000..23a634c --- /dev/null +++ b/tests/unit/test_packaging_modes.py @@ -0,0 +1,447 @@ +"""Unit tests for configurable bundle packaging modes (single / per-pipeline / per-group).""" + +from __future__ import annotations + +import json + +import pytest +import yaml + +from flowx.bundler.dab_writer import ( + MalformedReportError, + _group_workflows, + _load_group_spec, + _load_report, + write_bundle_group, +) +from flowx.models.ir import ( + CopyActivity, + ExecutePipelineActivity, + ForEachActivity, + NotebookActivity, + Pipeline, + SparkPythonActivity, + WaitActivity, +) +from flowx.preparer.workflow_preparer import prepare_workflow + + +def _workflow(name: str, calls: list[str] | None = None): + tasks: list = [WaitActivity(name="wait", task_key="wait", wait_time_seconds=1)] + for index, callee in enumerate(calls or []): + tasks.append(ExecutePipelineActivity(name=f"call_{index}", task_key=f"call_{index}", pipeline_name=callee)) + return prepare_workflow(Pipeline(name=name, tasks=tasks)) + + +class TestGroupWorkflows: + def test_per_pipeline_one_group_each(self): + wfs = [_workflow("a"), _workflow("b")] + groups = _group_workflows(wfs, mode="per-pipeline") + assert sorted(name for name, _ in groups) == ["a", "b"] + assert all(len(members) == 1 for _, members in groups) + + def test_single_folds_all_into_one(self): + wfs = [_workflow("a"), _workflow("b")] + groups = _group_workflows(wfs, mode="single") + assert len(groups) == 1 + name, members = groups[0] + assert name == "flowx_bundle" + assert {m.name for m in members} == {"a", "b"} + + def test_single_uses_bundle_name_when_given(self): + groups = _group_workflows([_workflow("a"), _workflow("b")], mode="single", bundle_name="my_bundle") + assert groups[0][0] == "my_bundle" + + def test_per_group_inferred_groups_by_call_graph(self): + # a->b are connected (a calls b, so b is the callee root); c is standalone. + wfs = [_workflow("a", ["b"]), _workflow("b"), _workflow("c")] + groups = _group_workflows(wfs, mode="per-group", group_by="inferred") + members_by_name = {name: sorted(m.name for m in members) for name, members in groups} + # Bundle is named after the callee root 'b' (topo-first), not the alphabetically-first 'a'. + assert members_by_name == {"b": ["a", "b"], "c": ["c"]} + + def test_per_group_inferred_names_bundle_after_callee_root_not_alphabetical(self): + # 'z_root' is the callee (deploys first) but sorts last; it must still name the bundle, + # proving the name follows the dependency graph rather than alphabetical order. + wfs = [_workflow("a_caller", ["z_root"]), _workflow("z_root")] + groups = _group_workflows(wfs, mode="per-group", group_by="inferred") + assert len(groups) == 1 + assert groups[0][0] == "z_root" + + def test_per_group_spec_honors_mapping(self): + wfs = [_workflow("a"), _workflow("b"), _workflow("c")] + groups = _group_workflows( + wfs, + mode="per-group", + group_by="spec", + group_spec={"a": "grp1", "b": "grp1", "c": "grp2"}, + ) + members_by_name = {name: sorted(m.name for m in members) for name, members in groups} + assert members_by_name == {"grp1": ["a", "b"], "grp2": ["c"]} + + def test_single_pipeline_collapses_regardless_of_mode(self): + groups = _group_workflows([_workflow("solo")], mode="per-group") + assert len(groups) == 1 + assert groups[0][1][0].name == "solo" + + def test_colliding_normalized_keys_raise_not_silently_drop(self): + # "Load Sales" and "load-sales" both normalize to "load_sales" -> must fail loudly, since + # keeping only one would ship a bundle set silently missing a pipeline. + wfs = [_workflow("Load Sales"), _workflow("load-sales")] + with pytest.raises(ValueError, match="collide on normalized key"): + _group_workflows(wfs, mode="per-pipeline") + + +class TestLoadGroupSpec: + def test_flat_pipeline_to_group_map(self, tmp_path): + spec = tmp_path / "spec.yml" + spec.write_text(yaml.dump({"Pipeline A": "grp1", "pipeline_b": "grp1"})) + assert _load_group_spec(spec) == {"pipeline_a": "grp1", "pipeline_b": "grp1"} + + def test_group_to_pipelines_map(self, tmp_path): + spec = tmp_path / "spec.json" + spec.write_text('{"grp1": ["Pipeline A", "pipeline_b"], "grp2": ["c"]}') + assert _load_group_spec(spec) == {"pipeline_a": "grp1", "pipeline_b": "grp1", "c": "grp2"} + + +class TestWriteBundleGroup: + def test_single_bundle_holds_multiple_job_resources(self, tmp_path): + wfs = [_workflow("alpha"), _workflow("beta")] + write_bundle_group(wfs, tmp_path, bundle_name="combined") + resources = {p.name for p in (tmp_path / "resources").glob("*.yml")} + assert {"alpha.yml", "beta.yml"} <= resources + databricks_yml = yaml.safe_load((tmp_path / "databricks.yml").read_text()) + assert databricks_yml["bundle"]["name"] == "combined" + # One consolidated SETUP.md for the whole bundle. + assert (tmp_path / "SETUP.md").exists() + + def test_intra_group_call_stays_direct_ref(self, tmp_path): + # alpha calls beta; both in the same bundle -> keep ${resources.jobs.beta.id}, no ${var}. + wfs = [_workflow("alpha", ["beta"]), _workflow("beta")] + write_bundle_group(wfs, tmp_path, bundle_name="combined") + alpha_yaml = (tmp_path / "resources" / "alpha.yml").read_text() + assert "${resources.jobs.beta.id}" in alpha_yaml + assert "${var.beta}" not in alpha_yaml + databricks_yml = yaml.safe_load((tmp_path / "databricks.yml").read_text()) + assert "beta" not in databricks_yml.get("variables", {}) + + def test_cross_group_call_becomes_var(self, tmp_path): + # alpha calls gamma, which is NOT in this bundle -> rewrite to ${var.gamma_job_id}. + wfs = [_workflow("alpha", ["gamma"])] + write_bundle_group(wfs, tmp_path, bundle_name="alpha_only") + alpha_yaml = (tmp_path / "resources" / "alpha.yml").read_text() + assert "${var.gamma_job_id}" in alpha_yaml + databricks_yml = yaml.safe_load((tmp_path / "databricks.yml").read_text()) + assert "gamma_job_id" in databricks_yml["variables"] + + +def _copy_workflow(name: str, source_type: str): + """A one-Copy-activity pipeline; the activity name is shared so notebook paths would collide.""" + return prepare_workflow( + Pipeline( + name=name, + tasks=[ + CopyActivity(name="Copy Data", task_key="copy_data", source_type=source_type, sink_type="DeltaSink") + ], + ) + ) + + +def _foreach_subjob_workflow(name: str): + """A pipeline whose ForEach escalates to an inner sub-job (two children), keyed 'loop_inner_tasks'.""" + foreach = ForEachActivity( + name="Loop", + task_key="loop", + items_expression="@pipeline().parameters.arr", + inner_activities=[ + NotebookActivity(name="One", task_key="one", notebook_path="/Shared/one"), + NotebookActivity(name="Two", task_key="two", notebook_path="/Shared/two"), + ], + ) + return prepare_workflow(Pipeline(name=name, tasks=[foreach])) + + +class TestMultiPipelineArtifactCollisions: + """Regression: co-locating pipelines in one bundle must not let same-named artifacts collide.""" + + def test_notebooks_namespaced_per_pipeline(self, tmp_path): + # Two pipelines, each a Copy named "Copy Data" -> same notebook filename, different content. + write_bundle_group( + [_copy_workflow("pipe_a", "AzureSqlSource"), _copy_workflow("pipe_b", "BlobSource")], + tmp_path, + bundle_name="combined", + ) + notebooks = {str(p.relative_to(tmp_path / "src")) for p in (tmp_path / "src").rglob("*.py")} + # Each pipeline's copy notebook lands under its own subdirectory — no overwrite. + assert "notebooks/pipe_a/copy_data.py" in notebooks + assert "notebooks/pipe_b/copy_data.py" in notebooks + # And each job references its OWN notebook. + assert "../src/notebooks/pipe_a/copy_data.py" in (tmp_path / "resources" / "pipe_a.yml").read_text() + assert "../src/notebooks/pipe_b/copy_data.py" in (tmp_path / "resources" / "pipe_b.yml").read_text() + + def test_inner_foreach_job_keys_namespaced_per_pipeline(self, tmp_path): + write_bundle_group( + [_foreach_subjob_workflow("pipe_a"), _foreach_subjob_workflow("pipe_b")], + tmp_path, + bundle_name="combined", + ) + resources = {p.name for p in (tmp_path / "resources").glob("*.yml")} + # Both inner sub-jobs survive under distinct, pipeline-prefixed keys. + assert "pipe_a_loop_inner_tasks.yml" in resources + assert "pipe_b_loop_inner_tasks.yml" in resources + # Each parent's run_job_task points at its own inner job. + assert "${resources.jobs.pipe_a_loop_inner_tasks.id}" in (tmp_path / "resources" / "pipe_a.yml").read_text() + assert "${resources.jobs.pipe_b_loop_inner_tasks.id}" in (tmp_path / "resources" / "pipe_b.yml").read_text() + + def test_namespacing_rewrites_self_referential_paths_in_notebook_body(self, tmp_path): + # A Spark-Python placeholder body references its own bundle path (`... src/scripts/foo.py`). + # After namespacing, that in-body path must match the notebook's new location, or an operator + # following the download hint would write the script where the task no longer looks. + def spark_wf(name: str): + return prepare_workflow( + Pipeline( + name=name, + tasks=[SparkPythonActivity(name="Run", task_key="run", python_file="dbfs:/scripts/foo.py")], + ) + ) + + write_bundle_group([spark_wf("pipe_a"), spark_wf("pipe_b")], tmp_path, bundle_name="combined") + body = (tmp_path / "src" / "scripts" / "pipe_a" / "foo.py").read_text() + # The download hint points at the namespaced path, not the un-prefixed one. + assert "src/scripts/pipe_a/foo.py" in body + assert "src/scripts/foo.py" not in body + # And the task's python_file points at the same namespaced path. + assert "../src/scripts/pipe_a/foo.py" in (tmp_path / "resources" / "pipe_a.yml").read_text() + + def test_single_pipeline_bundle_paths_unchanged(self, tmp_path): + # Namespacing must NOT fire for a one-pipeline bundle (per-pipeline output stays identical). + write_bundle_group([_copy_workflow("solo", "AzureSqlSource")], tmp_path, bundle_name="solo") + notebooks = {str(p.relative_to(tmp_path / "src")) for p in (tmp_path / "src").rglob("*.py")} + assert "notebooks/copy_data.py" in notebooks + assert not any("/solo/" in n for n in notebooks) + + +def _write_two_pipeline_report(tmp_path): + """Writes an aggregated translation report with two pipelines: 'caller' calls 'callee'.""" + import json + + work = tmp_path / ".work" + work.mkdir() + report = { + "translations": [ + { + "pipeline": "caller", + "status": "translated", + "ir": { + "type": "ExecutePipelineActivity", + "name": "Run Callee", + "task_key": "run_callee", + "pipeline_name": "callee", + }, + }, + { + "pipeline": "callee", + "status": "translated", + "ir": { + "type": "WaitActivity", + "name": "Pause", + "task_key": "pause", + "wait_time_seconds": 5, + }, + }, + ] + } + (work / "translation_report.json").write_text(json.dumps(report)) + + +class TestPackageMainModes: + def _run(self, tmp_path, *extra): + from flowx.bundler.dab_writer import main + + return main( + [ + "--output-dir", + str(tmp_path), + "--no-download-workspace-files", + "--keep-intermediates", + *extra, + ] + ) + + def test_per_pipeline_writes_a_bundle_dir_each_plus_deploy_md(self, tmp_path): + _write_two_pipeline_report(tmp_path) + assert self._run(tmp_path, "--packaging-mode", "per-pipeline") == 0 + assert (tmp_path / "caller" / "databricks.yml").exists() + assert (tmp_path / "callee" / "databricks.yml").exists() + deploy_md = (tmp_path / "DEPLOY.md").read_text() + # callee deploys before caller. + assert deploy_md.index("`callee/`") < deploy_md.index("`caller/`") + + def test_single_mode_one_bundle_at_root(self, tmp_path): + _write_two_pipeline_report(tmp_path) + assert self._run(tmp_path, "--packaging-mode", "single") == 0 + assert (tmp_path / "databricks.yml").exists() + resources = {p.name for p in (tmp_path / "resources").glob("*.yml")} + assert {"caller.yml", "callee.yml"} <= resources + # Intra-bundle call stays a direct ref. + assert "${resources.jobs.callee.id}" in (tmp_path / "resources" / "caller.yml").read_text() + assert "single bundle" in (tmp_path / "DEPLOY.md").read_text() + + def test_per_group_inferred_colocates_connected_pipelines(self, tmp_path): + _write_two_pipeline_report(tmp_path) + assert self._run(tmp_path, "--packaging-mode", "per-group") == 0 + # caller + callee are connected -> a single component, so one bundle at the output root. + resources = {p.name for p in (tmp_path / "resources").glob("*.yml")} + assert {"caller.yml", "callee.yml"} <= resources + assert "single bundle" in (tmp_path / "DEPLOY.md").read_text() + + def test_per_group_inferred_separates_disconnected_pipelines(self, tmp_path): + import json + + work = tmp_path / ".work" + work.mkdir() + # Two independent pipelines with no Run Pipeline edge between them -> two bundles. + report = { + "translations": [ + { + "pipeline": "solo_one", + "status": "translated", + "ir": {"type": "WaitActivity", "name": "W", "task_key": "w", "wait_time_seconds": 1}, + }, + { + "pipeline": "solo_two", + "status": "translated", + "ir": {"type": "WaitActivity", "name": "W", "task_key": "w", "wait_time_seconds": 1}, + }, + ] + } + (work / "translation_report.json").write_text(json.dumps(report)) + assert self._run(tmp_path, "--packaging-mode", "per-group") == 0 + assert (tmp_path / "solo_one" / "databricks.yml").exists() + assert (tmp_path / "solo_two" / "databricks.yml").exists() + + +class TestLoadReportPipelinesShape: + """The convert/modify ``{"pipelines": [...]}`` report shape (real flowx output).""" + + def test_load_report_handles_pipelines_format(self, tmp_path): + """``_load_report`` accepts the ``{"pipelines": [...]}`` aggregated report. + + Regression: ``convert``/``modify`` serialize multi-pipeline reports under a top-level + ``"pipelines"`` key, but ``_load_report`` only understood the single-pipeline and legacy + ``"translations"`` shapes and silently returned ``[]`` for this one — so ``package`` aborted + with "No translated pipelines found" for any real multi-pipeline factory. + """ + report = { + "pipelines": [ + { + "name": "pipeline_a", + "tasks": [ + {"type": "WaitActivity", "name": "Pause", "task_key": "pause", "wait_time_seconds": 5}, + ], + }, + { + "name": "pipeline_b", + "tasks": [ + {"type": "NotebookActivity", "name": "Run NB", "task_key": "run_nb", "notebook_path": "/x"}, + ], + }, + ] + } + report_path = tmp_path / "translation_report.json" + report_path.write_text(json.dumps(report)) + + workflows, skipped = _load_report(report_path) + assert {wf.name for wf in workflows} == {"pipeline_a", "pipeline_b"} + assert skipped == [] + + def test_load_report_skips_malformed_pipelines_entry(self, tmp_path): + """A malformed ``"pipelines"`` entry is skipped (surfaced via the returned skip list, and later + in SETUP.md) rather than dropping every pipeline; the valid pipelines still load.""" + report = { + "pipelines": [ + { + "name": "pipeline_ok", + "tasks": [ + {"type": "WaitActivity", "name": "Pause", "task_key": "pause", "wait_time_seconds": 5}, + ], + }, + {"name": "no_tasks_here"}, # missing "tasks" + {"tasks": []}, # missing "name" + "not-even-a-dict", # not a dict at all + ] + } + report_path = tmp_path / "translation_report.json" + report_path.write_text(json.dumps(report)) + + workflows, skipped = _load_report(report_path) + # The valid pipeline still loads; every offender is recorded in the skip list, not raised. + assert {wf.name for wf in workflows} == {"pipeline_ok"} + assert any("no_tasks_here" in label for label in skipped) + assert any("index 2" in label for label in skipped) + assert any("index 3" in label for label in skipped) + + def test_package_main_aborts_on_malformed_pipelines_entry(self, tmp_path, capsys): + """``package`` aborts (non-zero, nothing written) when a pipeline entry is malformed. + + The source-reconciliation preflight fails closed on a malformed ``pipelines`` report before any + bundle is written, so a corrupt/incomplete entry (here, one missing ``tasks``) is rejected with a + clear message rather than silently emitting a bundle short a pipeline. + """ + from flowx.bundler.dab_writer import main as dab_main + + report = { + "pipelines": [ + { + "name": "pipeline_ok", + "tags": {"source": "adf"}, + "tasks": [ + {"type": "WaitActivity", "name": "Pause", "task_key": "pause", "wait_time_seconds": 5}, + ], + }, + {"name": "no_tasks_here", "tags": {"source": "adf"}}, # missing "tasks" + ] + } + report_path = tmp_path / "translation_report.json" + report_path.write_text(json.dumps(report)) + out_dir = tmp_path / "out" + + exit_code = dab_main( + ["--report", str(report_path), "--output-dir", str(out_dir), "--no-download-workspace-files"] + ) + assert exit_code != 0 + assert "no_tasks_here" in capsys.readouterr().err + # Fail-closed: nothing was written to the destination. + assert not (out_dir / "databricks.yml").exists() + + +class TestSyntheticParameterDefault: + """A job parameter with no ADF default gets default="" (DAB-valid) but is surfaced in SETUP.md.""" + + def test_missing_default_emits_empty_and_lists_in_setup(self, tmp_path): + wf = prepare_workflow( + Pipeline( + name="needs_param", + tasks=[WaitActivity(name="w", task_key="w", wait_time_seconds=1)], + parameters=[{"name": "target_table"}], # no "default" + ) + ) + write_bundle_group([wf], tmp_path, bundle_name="needs_param") + # default:"" is emitted so `bundle validate/deploy` accepts the parameter. + doc = yaml.safe_load((tmp_path / "resources" / "needs_param.yml").read_text()) + assert doc["resources"]["jobs"]["needs_param"]["parameters"] == [{"name": "target_table", "default": ""}] + # ...and it is surfaced in SETUP.md (not silently substituted, and not a separate file). + setup_md = (tmp_path / "SETUP.md").read_text() + assert "Job parameters without an ADF default" in setup_md + assert "target_table" in setup_md + + def test_present_default_not_listed_in_setup(self, tmp_path): + wf = prepare_workflow( + Pipeline( + name="has_param", + tasks=[WaitActivity(name="w", task_key="w", wait_time_seconds=1)], + parameters=[{"name": "region", "default": "eu"}], + ) + ) + write_bundle_group([wf], tmp_path, bundle_name="has_param") + assert "Job parameters without an ADF default" not in (tmp_path / "SETUP.md").read_text() diff --git a/tests/unit/test_pipeline_graph.py b/tests/unit/test_pipeline_graph.py new file mode 100644 index 0000000..3b7b87e --- /dev/null +++ b/tests/unit/test_pipeline_graph.py @@ -0,0 +1,76 @@ +"""Unit tests for the pipeline-level Run Pipeline dependency graph.""" + +from __future__ import annotations + +import pytest + +from flowx.bundler.pipeline_graph import ( + PipelineCycleError, + build_pipeline_dependencies, + connected_components, + topo_order, +) +from flowx.models.ir import ExecutePipelineActivity, Pipeline, WaitActivity +from flowx.preparer.workflow_preparer import prepare_workflow + + +def _workflow(name: str, calls: list[str] | None = None): + """Builds a PreparedWorkflow named *name* that ExecutePipeline-calls each pipeline in *calls*.""" + tasks: list = [WaitActivity(name="wait", task_key="wait", wait_time_seconds=1)] + for index, callee in enumerate(calls or []): + tasks.append( + ExecutePipelineActivity( + name=f"call_{index}", + task_key=f"call_{index}", + pipeline_name=callee, + ) + ) + return prepare_workflow(Pipeline(name=name, tasks=tasks)) + + +class TestBuildPipelineDependencies: + def test_extracts_run_pipeline_edges(self): + workflows = [_workflow("a", ["b"]), _workflow("b", []), _workflow("c", ["a", "b"])] + deps = build_pipeline_dependencies(workflows) + assert deps == {"a": {"b"}, "b": set(), "c": {"a", "b"}} + + def test_every_workflow_is_a_key_even_without_deps(self): + deps = build_pipeline_dependencies([_workflow("solo", [])]) + assert deps == {"solo": set()} + + def test_call_to_pipeline_outside_migration_is_retained(self): + # 'b' is not a workflow in the migration; the edge is still recorded (deploy-time concern). + deps = build_pipeline_dependencies([_workflow("a", ["b"])]) + assert deps == {"a": {"b"}} + + +class TestConnectedComponents: + def test_splits_disjoint_pipelines(self): + deps = {"a": {"b"}, "b": set(), "c": set()} + assert connected_components(deps) == [["a", "b"], ["c"]] + + def test_transitive_calls_form_one_component(self): + deps = {"a": {"b"}, "b": {"c"}, "c": set()} + assert connected_components(deps) == [["a", "b", "c"]] + + def test_components_are_deterministic_and_sorted(self): + deps = {"z": set(), "m": {"n"}, "n": set()} + assert connected_components(deps) == [["m", "n"], ["z"]] + + def test_edge_to_outside_pipeline_ignored_for_grouping(self): + deps = {"a": {"external"}} + assert connected_components(deps) == [["a"]] + + +class TestTopoOrder: + def test_callees_before_callers(self): + deps = {"a": {"b"}, "b": set(), "c": {"a"}} + order = topo_order(deps) + assert order.index("b") < order.index("a") < order.index("c") + + def test_independent_nodes_sorted(self): + assert topo_order({"y": set(), "x": set()}) == ["x", "y"] + + def test_cycle_raises(self): + with pytest.raises(PipelineCycleError): + topo_order({"a": {"b"}, "b": {"a"}}) From 82541921457e0c774363f3f7359f4565517b34f3 Mon Sep 17 00:00:00 2001 From: Zanita Rahimi Date: Fri, 7 Aug 2026 15:40:35 +0200 Subject: [PATCH 4/9] address review: mode-switch safety, non-mutating writer, cleaner errors --- src/flowx/bundler/dab_writer.py | 67 +++++++++++++++++++++++++----- src/flowx/bundler/deployer.py | 38 +++++++++++++---- tests/unit/test_deployer.py | 35 ++++++++++++++++ tests/unit/test_packaging_modes.py | 67 ++++++++++++++++++++++++++++++ 4 files changed, 190 insertions(+), 17 deletions(-) diff --git a/src/flowx/bundler/dab_writer.py b/src/flowx/bundler/dab_writer.py index cdd539d..7b74e53 100644 --- a/src/flowx/bundler/dab_writer.py +++ b/src/flowx/bundler/dab_writer.py @@ -78,7 +78,15 @@ class _BundleYamlDumper(yaml.SafeDumper): """YAML dumper that leaves keys unquoted and only quotes values when needed.""" -# Module-level warnings collector — reset per write_bundle call. +# Module-level accumulators (_bundle_warnings, _cross_bundle_variables, _neutralized_conditions, +# _synthetic_default_parameters) are reset at the top of every write_bundle_group call and read at the +# end of the same call. single / per-group modes invoke the writer in a loop over groups, but each +# iteration resets first, so a prior group's state — even after a mid-write exception — never leaks +# into the next: the reset, not the previous run's cleanup, guarantees a clean slate. This is safe only +# for sequential writing; parallelising bundle writing would require threading this state through +# instead. Kept module-level (a pre-existing pattern) to avoid re-plumbing every helper's signature. +# +# Module-level warnings collector — reset per write_bundle_group call. _bundle_warnings: list[str] = [] # Cross-bundle ExecutePipeline refs seen while translating (variable_name -> target pipeline). Reset per @@ -162,6 +170,12 @@ def _rewrite_task_string_values(tasks: list[dict[str, Any]], replacements: dict[ swaps any string equal to a key of *replacements* for its mapped value. Used to keep notebook / python-file paths and ``run_job_task`` job-id refs in sync after a namespacing rename, without hard-coding every field name (``notebook_path``, ``python_file``, ``job_id``, …). + + Trade-off: matching is by exact string value, not by field, so a non-path field whose value + happens to equal a renamed notebook path (e.g. a ``base_parameters`` entry that genuinely passes a + notebook path) is rewritten too. That is intentional — if a parameter carries the path, it should + track the rename — and it only fires on an exact whole-string match, so free text merely mentioning + a path (``"see ../src/notebooks/x.py"``) is left untouched. """ if not replacements: return @@ -204,10 +218,22 @@ def _namespace_bundle_artifacts(workflow: PreparedWorkflow, prefix: str) -> None replacements: dict[str, str] = {} # 1. Inner ForEach job keys: rename inner.name, map old resources.jobs ref -> new. + seen_new_keys: dict[str, str] = {} for inner in workflow.inner_workflows: old_key = normalize_task_key(inner.name) inner.name = f"{prefix}__{inner.name}" new_key = normalize_task_key(inner.name) + # normalize_task_key collapses the "__" separator to "_", so a pipeline name containing "__" + # could make two inner jobs land on the same namespaced key (e.g. prefix "a" + inner "b__c" vs + # prefix "a__b" + inner "c" both -> "a_b_c"), silently overwriting one resource file. Refuse + # rather than ship a corrupt bundle — this requires pathological ADF names but is cheap to catch. + if new_key in seen_new_keys: + raise ValueError( + f"Namespacing inner ForEach jobs under prefix '{prefix}' produced a duplicate resource " + f"key '{new_key}' (from inner jobs '{seen_new_keys[new_key]}' and '{inner.name}'). " + "Rename the offending pipeline/activity so keys don't collide after normalization." + ) + seen_new_keys[new_key] = inner.name if old_key != new_key: replacements[f"${{resources.jobs.{old_key}.id}}"] = f"${{resources.jobs.{new_key}.id}}" @@ -233,11 +259,21 @@ def _namespace_bundle_artifacts(workflow: PreparedWorkflow, prefix: str) -> None def _prefixed_notebook_relative_path(relative_path: str, prefix: str) -> str: """Inserts *prefix* as a subdirectory under the top-level segment of a notebook relative path. - ``notebooks/copy_data.py`` -> ``notebooks//copy_data.py``; a path with no ``/`` is just - prefixed. Idempotent when the prefix segment is already present. + ``notebooks/copy_data.py`` -> ``notebooks//copy_data.py``. Idempotent for these real + (slashed) paths: a double-apply is a no-op (``tail`` already starts with ``/``) rather than + nesting ``notebooks///…``. + + A path with no ``/`` (``x.py`` -> ``/x.py``) is a defensive fallback — every notebook this + codebase emits is under a category dir (``notebooks/``, ``lib/``, ``src/…``), so it does not occur + in practice. Only the trivial ``relative_path == prefix`` re-apply is guarded there; nesting on a + slash-less re-apply is not, because distinguishing a once-prefixed ``/x.py`` from a genuine + first-time slashed path whose head equals the prefix (a pipeline literally named ``notebooks``) is + ambiguous — and skipping a genuine path would defeat the namespacing this function exists to do. """ head, sep, tail = relative_path.partition("/") if not sep: + if relative_path == prefix: + return relative_path return f"{prefix}/{relative_path}" if tail.startswith(f"{prefix}/"): return relative_path @@ -286,10 +322,13 @@ def write_bundle_group( if not workflows: raise ValueError("write_bundle_group requires at least one workflow") - # Deep-copy the input so writing a bundle never mutates the caller's PreparedWorkflows: several steps - # below rewrite the task dicts in place (namespacing, ${resources.jobs.X.id} -> ${var.X}), and leaking - # that back would erase the Run Pipeline edges pipeline_graph reads. Copying makes the writer a pure - # sink so callers can build the dependency graph before or after writing, in any order. + # Deep-copy the input so writing a bundle never mutates the caller's PreparedWorkflows. This + # matters because several steps below rewrite the task dicts in place — namespacing and the + # cross-bundle rewrite that turns ${resources.jobs.X.id} into ${var.X_job_id} — which would erase + # the Run Pipeline edges pipeline_graph reads. Without the copy, correctness of the caller's + # dependency graph / DEPLOY.md order would silently depend on it being computed before this call + # (main() does, but that is an unenforceable ordering trap). Copying makes the writer a pure sink: + # callers can build the graph before or after writing, in any order. workflows = [copy.deepcopy(workflow) for workflow in workflows] output_dir = Path(output_dir) @@ -724,8 +763,9 @@ def _render_deploy_md_for_run( ) -> str: """Builds DEPLOY.md from the workflows written this run (bridges to :mod:`deploy_writer`). - *pipeline_deps* must be computed **before** write_bundle_group mutates the task dicts (it rewrites - cross-bundle ``${resources.jobs.X.id}`` refs to ``${var.X}`` in place, which erases the edges). + *pipeline_deps* is the Run Pipeline graph from :func:`build_pipeline_dependencies`. write_bundle_group + works on a deep copy of its input, so the graph can be built before or after writing; passing it in + (rather than re-deriving it here from *written_groups*) just avoids scanning the task trees twice. """ from flowx.bundler.deploy_writer import render_deploy_md @@ -993,7 +1033,14 @@ def _write_groups(target_root: Path, *, announce: bool) -> tuple[list[Path], lis # written bundle in the migration directory. args.output_dir.parent.mkdir(parents=True, exist_ok=True) with tempfile.TemporaryDirectory(prefix=".flowx-preflight-", dir=args.output_dir.parent) as temporary: - _, staged_dirs = _write_groups(Path(temporary), announce=False) + try: + _, staged_dirs = _write_groups(Path(temporary), announce=False) + except ValueError as exc: + # e.g. the inner-ForEach-key collision guard in _namespace_bundle_artifacts. Surface it + # as a clean message + exit code rather than a raw traceback. Caught here in the temp-dir + # preflight so nothing is written to the destination. + print(f"Error: {exc}", file=sys.stderr) + return 1 preflight_violations = 0 for staged_dir in staged_dirs: result = check_bundle_dir(staged_dir) diff --git a/src/flowx/bundler/deployer.py b/src/flowx/bundler/deployer.py index d2bf546..9e1e464 100644 --- a/src/flowx/bundler/deployer.py +++ b/src/flowx/bundler/deployer.py @@ -106,6 +106,10 @@ def _read_bundle_dir(bundle_dir: Path, bundle_name: str) -> DiscoveredBundle: return DiscoveredBundle(bundle_name, resource_keys, depends_on) +class AmbiguousLayoutError(Exception): + """Raised when the output dir holds both a root bundle and subdirectory bundles.""" + + def _discover_bundles(output_dir: Path) -> list[DiscoveredBundle]: """Finds every bundle under *output_dir* and reads its jobs + cross-bundle deps. @@ -114,16 +118,32 @@ def _discover_bundles(output_dir: Path) -> list[DiscoveredBundle]: single-pipeline layout, where the sole bundle sits at the root), that root bundle is returned instead — a single root bundle has no siblings to order against, so it is deployed directly. Its ``bundle_dir`` is ``"."`` so ``run`` shells out in *output_dir* itself. + + Raises: + AmbiguousLayoutError: when *both* a root ``databricks.yml`` and subdirectory bundles are + present. ``package`` never clears the output dir (it only prunes ``.work/``), so + re-packaging into the same dir with a different ``--packaging-mode`` leaves both layouts on + disk. Silently deploying the root bundle would ignore the freshly-written subdirectory + bundles (or vice versa). Refuse and tell the operator to clear the dir, rather than deploy a + stale layout. """ + subdir_bundles = [ + child for child in sorted(output_dir.iterdir()) if child.is_dir() and (child / "databricks.yml").exists() + ] + if (output_dir / "databricks.yml").exists(): + if subdir_bundles: + names = ", ".join(child.name for child in subdir_bundles) + raise AmbiguousLayoutError( + f"{output_dir} holds both a root-level databricks.yml (a 'single'-mode bundle) and " + f"subdirectory bundle(s): {names}. This usually means the dir was packaged more than " + "once with different --packaging-mode values (package does not clear the output dir). " + "Deploying would use only one layout and silently ignore the other. Delete the stale " + "layout (or re-package into a clean directory) and retry." + ) return [_read_bundle_dir(output_dir, ".")] - bundles: list[DiscoveredBundle] = [] - for child in sorted(output_dir.iterdir()): - if not child.is_dir() or not (child / "databricks.yml").exists(): - continue - bundles.append(_read_bundle_dir(child, child.name)) - return bundles + return [_read_bundle_dir(child, child.name) for child in subdir_bundles] def _build_graph(bundles: list[DiscoveredBundle], *, allow_missing_deps: bool = False) -> dict[str, list[str]]: @@ -256,7 +276,11 @@ def run( print(f"Error: output directory not found: {output_dir}", file=sys.stderr) return 1 - bundles = _discover_bundles(output_dir) + try: + bundles = _discover_bundles(output_dir) + except AmbiguousLayoutError as exc: + print(f"Error: {exc}", file=sys.stderr) + return 1 if not bundles: print( f"No bundles found under {output_dir} (looked for immediate subdirectories with a " diff --git a/tests/unit/test_deployer.py b/tests/unit/test_deployer.py index f5521cc..0a1cfe6 100644 --- a/tests/unit/test_deployer.py +++ b/tests/unit/test_deployer.py @@ -14,6 +14,7 @@ from flowx.bundler import deployer from flowx.bundler.deployer import ( + AmbiguousLayoutError, CycleError, MissingDependencyError, _build_graph, @@ -233,3 +234,37 @@ def test_cycle_returns_error(tmp_path, monkeypatch): _make_bundle(tmp_path, "b", jobs=["b"], deps=["a"]) monkeypatch.setattr(deployer, "_run_cli", _FakeCli()) assert run(tmp_path) == 1 + + +class TestAmbiguousLayout: + """A root databricks.yml *and* subdirectory bundles both present (a mode-switch on one output dir, + since package never clears the dir) must be refused, not silently resolved to one layout.""" + + def test_root_and_subdir_bundles_raise(self, tmp_path): + # A stale single-mode bundle at the root, then a per-pipeline re-package writing subdir bundles. + (tmp_path / "databricks.yml").write_text("bundle:\n name: stale_single\n") + _make_bundle(tmp_path, "a", jobs=["a"]) + _make_bundle(tmp_path, "b", jobs=["b"]) + with pytest.raises(AmbiguousLayoutError) as excinfo: + _discover_bundles(tmp_path) + # The message names both stale subdirs so the operator knows what to clear. + assert "a" in str(excinfo.value) and "b" in str(excinfo.value) + + def test_run_reports_ambiguous_layout_and_deploys_nothing(self, tmp_path, monkeypatch): + (tmp_path / "databricks.yml").write_text("bundle:\n name: stale_single\n") + _make_bundle(tmp_path, "a", jobs=["a"]) + fake = _FakeCli() + monkeypatch.setattr(deployer, "_run_cli", fake) + assert run(tmp_path) == 1 + # No deploy was attempted — the ambiguity is caught before any CLI call. + assert fake.commands == [] + + def test_root_only_still_deploys_directly(self, tmp_path): + # Only a root bundle (clean single-mode dir) is unambiguous and returns the '.' bundle. + (tmp_path / "databricks.yml").write_text("bundle:\n name: only_single\n") + (tmp_path / "resources").mkdir() + (tmp_path / "resources" / "j.yml").write_text( + yaml.safe_dump({"resources": {"jobs": {"j": {"name": "j", "tasks": []}}}}) + ) + bundles = _discover_bundles(tmp_path) + assert [b.bundle_dir for b in bundles] == ["."] diff --git a/tests/unit/test_packaging_modes.py b/tests/unit/test_packaging_modes.py index 23a634c..4a502d7 100644 --- a/tests/unit/test_packaging_modes.py +++ b/tests/unit/test_packaging_modes.py @@ -12,6 +12,7 @@ _group_workflows, _load_group_spec, _load_report, + _prefixed_notebook_relative_path, write_bundle_group, ) from flowx.models.ir import ( @@ -286,6 +287,24 @@ def test_single_mode_one_bundle_at_root(self, tmp_path): # Intra-bundle call stays a direct ref. assert "${resources.jobs.callee.id}" in (tmp_path / "resources" / "caller.yml").read_text() assert "single bundle" in (tmp_path / "DEPLOY.md").read_text() + # bundle.name is the group name (flowx_bundle), NOT the first pipeline — so the dev workspace + # path is .bundle/flowx_bundle/dev, not .bundle/caller/dev for a bundle holding many pipelines. + databricks_yml = yaml.safe_load((tmp_path / "databricks.yml").read_text()) + assert databricks_yml["bundle"]["name"] == "flowx_bundle" + + def test_writer_valueerror_surfaces_cleanly(self, tmp_path, monkeypatch, capsys): + """A ValueError from write_bundle_group (e.g. the inner-key collision guard) must be reported as + a clean 'Error: ...' + exit 1, not escape main() as a traceback.""" + import flowx.bundler.dab_writer as dab_writer + + _write_two_pipeline_report(tmp_path) + + def _boom(*_args, **_kwargs): + raise ValueError("simulated collision") + + monkeypatch.setattr(dab_writer, "write_bundle_group", _boom) + assert self._run(tmp_path, "--packaging-mode", "per-pipeline") == 1 + assert "Error: simulated collision" in capsys.readouterr().err def test_per_group_inferred_colocates_connected_pipelines(self, tmp_path): _write_two_pipeline_report(tmp_path) @@ -445,3 +464,51 @@ def test_present_default_not_listed_in_setup(self, tmp_path): ) write_bundle_group([wf], tmp_path, bundle_name="has_param") assert "Job parameters without an ADF default" not in (tmp_path / "SETUP.md").read_text() + + +class TestWriterDoesNotMutateCaller: + """write_bundle_group must not mutate the PreparedWorkflows it is handed. It rewrites + ${resources.jobs.X.id} -> ${var.X} in place internally; if that leaked back to the caller it would + erase the Run Pipeline edges pipeline_graph reads, silently breaking grouping / DEPLOY.md order for + any caller that builds the graph after writing.""" + + def test_run_pipeline_graph_survives_writing(self, tmp_path): + from flowx.bundler.pipeline_graph import build_pipeline_dependencies + + # 'a' calls 'b'; write them as separate per-pipeline bundles (so b is cross-bundle for a). + wfs = [_workflow("a", ["b"]), _workflow("b")] + write_bundle_group([wfs[0]], tmp_path / "a", bundle_name="a") + write_bundle_group([wfs[1]], tmp_path / "b", bundle_name="b") + + # Graph built AFTER writing must still see the a->b edge (writer worked on copies). + deps = build_pipeline_dependencies(wfs) + assert deps == {"a": {"b"}, "b": set()} + + def test_caller_task_dicts_unchanged(self, tmp_path): + wf = _workflow("caller", ["callee"]) + before = [dict(t) for t in wf.tasks] + write_bundle_group([wf], tmp_path, bundle_name="caller") + # The caller's own task dicts are untouched — no ${var.X} rewrite bled back. + assert wf.tasks == before + + +class TestPrefixedNotebookRelativePath: + """Every notebook this codebase emits is under a category dir (notebooks/, lib/, src/…), so the + slashed path is the real case and must be idempotent (double-apply is a no-op). The function must + NOT special-case head==prefix, which would silently skip namespacing a genuine path whose top + segment equals the pipeline key (e.g. a pipeline literally named 'notebooks').""" + + def test_slashed_path_prefixed_once(self): + assert _prefixed_notebook_relative_path("notebooks/x.py", "pre") == "notebooks/pre/x.py" + + def test_slashed_path_idempotent(self): + once = _prefixed_notebook_relative_path("notebooks/x.py", "pre") + assert _prefixed_notebook_relative_path(once, "pre") == once + + def test_genuine_path_with_head_equal_to_prefix_is_still_namespaced(self): + # A pipeline named 'notebooks' -> prefix 'notebooks'; its path notebooks/x.py must still be + # namespaced to notebooks/notebooks/x.py, not skipped because head == prefix. + assert _prefixed_notebook_relative_path("notebooks/x.py", "notebooks") == "notebooks/notebooks/x.py" + + def test_slashless_path_prefixed_once(self): + assert _prefixed_notebook_relative_path("x.py", "pre") == "pre/x.py" From a83d36148af7f558489ab5a5a06bcc01d1621e63 Mon Sep 17 00:00:00 2001 From: Zanita Rahimi Date: Tue, 1 Sep 2026 16:55:13 +0200 Subject: [PATCH 5/9] Address review: drop dead cross-bundle rewriter, guard missing CLI - Remove _rewrite_cross_bundle_run_job_refs from dab_writer: it was replaced by _rewrite_cross_bundle_job_references during the main rebase and had no remaining callers. Drop its now-unused CROSS_BUNDLE_JOB_ID_REF import. - Fix stale comments that named the removed function and claimed the old bare ${var.X} scheme (pipeline_graph.py, deployer.py docstring); the live scheme is ${var.X_job_id}. - deployer.run(): check for the `databricks` CLI on PATH up front and return an actionable error instead of an uncaught FileNotFoundError. Skipped for --dry-run, which never shells out. Add tests. --- src/flowx/bundler/dab_writer.py | 41 ----------------------------- src/flowx/bundler/deployer.py | 26 +++++++++++++----- src/flowx/bundler/pipeline_graph.py | 5 ++-- tests/unit/test_deployer.py | 16 +++++++++++ 4 files changed, 38 insertions(+), 50 deletions(-) diff --git a/src/flowx/bundler/dab_writer.py b/src/flowx/bundler/dab_writer.py index 7b74e53..6e5f512 100644 --- a/src/flowx/bundler/dab_writer.py +++ b/src/flowx/bundler/dab_writer.py @@ -24,7 +24,6 @@ ) from flowx.bundler.inner_job_params import normalize_value from flowx.bundler.notebook_writer import write_notebooks -from flowx.bundler.pipeline_graph import CROSS_BUNDLE_JOB_ID_REF as _CROSS_BUNDLE_JOB_ID_REF from flowx.bundler.prereqs_writer import ManualParameter, build_prereqs, render_setup_md from flowx.bundler.setup_generator import generate_setup_tasks from flowx.models.dab import DabNotebook, SecretInstruction, SetupTask @@ -2050,46 +2049,6 @@ def visit(task: dict[str, Any]) -> None: return neutralized -def _rewrite_cross_bundle_run_job_refs( - tasks: list[dict[str, Any]], - known_bundle_jobs: set[str], - cross_bundle_variables: dict[str, str], -) -> None: - """Rewrites ``run_job_task`` refs to jobs outside this bundle into ``${var.X}``. - - An ExecutePipeline activity is emitted as ``run_job_task.job_id = - ${resources.jobs.X.id}`` (see ``execute_pipeline.prepare``). That resolves - only when job ``X`` is a resource in *this* bundle. In a multi-pipeline - migration each ADF pipeline becomes its **own** bundle, so a reference to a - sibling pipeline points at a resource node that does not exist here and - ``bundle deploy`` fails with ``no such node "resources.jobs.X"``. - - For every ``run_job_task.job_id`` whose target is not in *known_bundle_jobs*, - rewrite it to ``${var.X}`` and register ``X`` in *cross_bundle_variables* so - the ``databricks.yml`` builder declares a matching bundle variable (the user - supplies the numeric job id at deploy time, per SETUP.md). Recurses into - ``for_each_task.task`` bodies. The rewritten refs are surfaced to the - operator via *cross_bundle_variables* (declared in ``databricks.yml`` and - listed in SETUP.md), so this mutates in place and returns nothing. - """ - - def visit(task: dict[str, Any]) -> None: - run_job = task.get("run_job_task") - if isinstance(run_job, dict): - match = _CROSS_BUNDLE_JOB_ID_REF.fullmatch(str(run_job.get("job_id", ""))) - if match: - target = match.group(1) - if target not in known_bundle_jobs: - run_job["job_id"] = f"${{var.{target}}}" - cross_bundle_variables[target] = target - for_each = task.get("for_each_task") - if for_each and isinstance(for_each.get("task"), dict): - visit(for_each["task"]) - - for task in tasks: - visit(task) - - def _collect_all_task_keys(tasks: list[dict[str, Any]]) -> set[str]: """Collects every task_key reachable from the job's top-level task list.""" keys: set[str] = set() diff --git a/src/flowx/bundler/deployer.py b/src/flowx/bundler/deployer.py index 9e1e464..960a432 100644 --- a/src/flowx/bundler/deployer.py +++ b/src/flowx/bundler/deployer.py @@ -2,15 +2,15 @@ flowx emits **one Databricks Asset Bundle per ADF pipeline**. When pipeline A calls pipeline B via ``ExecutePipeline``, the generated ``run_job_task.job_id`` references B — a job that lives in B's own -bundle. ``_rewrite_cross_bundle_run_job_refs`` (see :mod:`flowx.bundler.dab_writer`) rewrites those -out-of-bundle references to ``${var.}`` and declares a matching bundle variable, so each bundle is -deploy-valid on its own; the operator otherwise has to discover B's numeric job id and pass it by hand. +bundle. ``_rewrite_cross_bundle_job_references`` (see :mod:`flowx.bundler.dab_writer`) rewrites those +out-of-bundle references to ``${var._job_id}`` and declares a matching bundle variable, so each +bundle is deploy-valid on its own; the operator otherwise has to discover B's numeric job id by hand. This module automates that. It **discovers** the bundles under an output directory (no manifest -needed), reads each bundle's job resource keys and its ``${var.}`` cross-bundle dependencies -straight from the generated YAML, topologically orders them (callees first), and deploys each with -``databricks bundle deploy``. After every deploy it reads the deployed job id from -``databricks bundle summary -o json`` and injects it into callers via ``--var "="``. +needed), reads each bundle's job resource keys and its ``${var._job_id}`` cross-bundle +dependencies straight from the generated YAML, topologically orders them (callees first), and deploys +each with ``databricks bundle deploy``. After every deploy it reads the deployed job id from +``databricks bundle summary -o json`` and injects it into callers via ``--var "_job_id="``. Numeric ids (not names) are captured and injected, so dev-mode ``[dev ]`` job-name prefixes are irrelevant — this works identically for ``dev`` and ``prod`` targets. @@ -25,6 +25,7 @@ import argparse import json import re +import shutil import subprocess import sys from pathlib import Path @@ -276,6 +277,17 @@ def run( print(f"Error: output directory not found: {output_dir}", file=sys.stderr) return 1 + # A real deploy shells out to the `databricks` CLI; fail with an actionable message rather than an + # uncaught FileNotFoundError if it isn't on PATH. Skipped for --dry-run, which never invokes it. + if not dry_run and shutil.which("databricks") is None: + print( + "Error: the `databricks` CLI was not found on PATH. This is a local-CLI operation " + "(`databricks bundle deploy`/`summary`); install the CLI, or use --dry-run to preview the " + "deploy order without deploying.", + file=sys.stderr, + ) + return 1 + try: bundles = _discover_bundles(output_dir) except AmbiguousLayoutError as exc: diff --git a/src/flowx/bundler/pipeline_graph.py b/src/flowx/bundler/pipeline_graph.py index 65ead8c..d4baf6d 100644 --- a/src/flowx/bundler/pipeline_graph.py +++ b/src/flowx/bundler/pipeline_graph.py @@ -30,8 +30,9 @@ if TYPE_CHECKING: from flowx.preparer.workflow_preparer import PreparedWorkflow -# The ref shape execute_pipeline.prepare emits: ${resources.jobs..id}. Shared with -# dab_writer._rewrite_cross_bundle_run_job_refs (imported there) so the two never drift. +# The ref shape execute_pipeline.prepare emits: ${resources.jobs..id}. Used here to read the +# Run Pipeline call graph off the prepared task trees *before* dab_writer rewrites cross-bundle refs +# to ${var._job_id} (see _rewrite_cross_bundle_job_references). CROSS_BUNDLE_JOB_ID_REF = re.compile(r"\$\{resources\.jobs\.([^.]+)\.id\}") diff --git a/tests/unit/test_deployer.py b/tests/unit/test_deployer.py index 0a1cfe6..b4449cf 100644 --- a/tests/unit/test_deployer.py +++ b/tests/unit/test_deployer.py @@ -229,6 +229,22 @@ def test_empty_output_dir_returns_error(tmp_path): assert run(tmp_path) == 1 +def test_missing_databricks_cli_reports_cleanly(tmp_path, monkeypatch, capsys): + # A real deploy needs the `databricks` CLI on PATH; if it's absent, run() must return 1 with an + # actionable message rather than an uncaught FileNotFoundError from subprocess. + _make_bundle(tmp_path, "a", jobs=["a"]) + monkeypatch.setattr(deployer.shutil, "which", lambda _name: None) + assert run(tmp_path) == 1 + assert "databricks` CLI was not found" in capsys.readouterr().err + + +def test_missing_databricks_cli_does_not_block_dry_run(tmp_path, monkeypatch): + # --dry-run never shells out, so a missing CLI must not stop it. + _make_bundle(tmp_path, "a", jobs=["a"]) + monkeypatch.setattr(deployer.shutil, "which", lambda _name: None) + assert run(tmp_path, dry_run=True) == 0 + + def test_cycle_returns_error(tmp_path, monkeypatch): _make_bundle(tmp_path, "a", jobs=["a"], deps=["b"]) _make_bundle(tmp_path, "b", jobs=["b"], deps=["a"]) From 22f45e50833a6fe0c86745fe673db3fd8dcdc2bc Mon Sep 17 00:00:00 2001 From: Lorenzo Rubio Date: Thu, 10 Sep 2026 14:59:47 +0200 Subject: [PATCH 6/9] minor reformatting --- tests/unit/test_packaging_modes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_packaging_modes.py b/tests/unit/test_packaging_modes.py index 4a502d7..8cb95bc 100644 --- a/tests/unit/test_packaging_modes.py +++ b/tests/unit/test_packaging_modes.py @@ -8,7 +8,6 @@ import yaml from flowx.bundler.dab_writer import ( - MalformedReportError, _group_workflows, _load_group_spec, _load_report, From b9a83cfd668d5dc2a49f95df42d6fae569f04b8b Mon Sep 17 00:00:00 2001 From: Zanita Rahimi Date: Fri, 11 Sep 2026 11:00:06 +0200 Subject: [PATCH 7/9] Fix global-param hoisting across grouped workflows Declare the union of every workflow's hoisted globals in databricks.yml + SETUP.md, but pass each job only its own workflow's globals. Single-workflow bundles unchanged. Adds TestHoistedGlobalsAcrossGroupedWorkflows. --- src/flowx/bundler/dab_writer.py | 24 +++++++++--- tests/unit/test_bundler.py | 69 ++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/flowx/bundler/dab_writer.py b/src/flowx/bundler/dab_writer.py index 6e5f512..fd7548e 100644 --- a/src/flowx/bundler/dab_writer.py +++ b/src/flowx/bundler/dab_writer.py @@ -378,8 +378,18 @@ def write_bundle_group( "warehouse_id", {"description": "SQL warehouse id for sql_task queries"} ) - hoisted_global_variables = _collect_hoisted_global_variables(workflow) - extra_variable_declarations = {**pipeline_variable_declarations, **hoisted_global_variables} + # Hoisted factory globals become bundle variables. Collected per workflow (each call also merges the + # workflow's inner ForEach jobs): the bundle-scoped surfaces — databricks.yml ``variables:`` and + # SETUP.md — declare the UNION across every workflow in the group, while each individual job resource + # (in the loop below) gets only its own workflow's globals. For a single-workflow bundle the union + # equals that one workflow's set, so per-pipeline output is unchanged. + hoisted_globals_by_workflow: dict[int, set[str]] = {} + hoisted_global_union: dict[str, Any] = {} + for workflow in workflows: + wf_hoisted = _collect_hoisted_global_variables(workflow) + hoisted_globals_by_workflow[id(workflow)] = set(wf_hoisted) + hoisted_global_union.update(wf_hoisted) + extra_variable_declarations = {**pipeline_variable_declarations, **hoisted_global_union} # dbt-factory PyDABs hooks: each `resources._dbt_job:load_resources` module must be # registered under the `python.resources` block so `bundle deploy` runs it to build the dbt job. @@ -424,15 +434,17 @@ def write_bundle_group( manual_parameters: list[ManualParameter] = [] resources_dir = output_dir / "resources" resources_dir.mkdir(parents=True, exist_ok=True) - hoisted_global_names = set(hoisted_global_variables) for workflow in workflows: resource_key = normalize_task_key(workflow.name) + # Each job resource (parent + its inner ForEach jobs) gets only this workflow's hoisted globals, + # not the group-wide union, so a widget is bound to ${var.X} only in the pipelines that declare X. + wf_hoisted_globals = hoisted_globals_by_workflow[id(workflow)] manual_parameters.extend(_extract_manual_parameters_from_existing_notebook_tasks(workflow.tasks)) for inner in workflow.inner_workflows: manual_parameters.extend(_extract_manual_parameters_from_existing_notebook_tasks(inner.tasks)) job_yml_path = resources_dir / f"{resource_key}.yml" - job_resource = _build_job_resource(workflow, resource_key, hoisted_globals=hoisted_global_names) + job_resource = _build_job_resource(workflow, resource_key, hoisted_globals=wf_hoisted_globals) job_yml_path.write_text( yaml.dump( job_resource, default_flow_style=False, sort_keys=False, allow_unicode=True, Dumper=_BundleYamlDumper @@ -447,7 +459,7 @@ def write_bundle_group( inner_key = normalize_task_key(inner.name) inner_yml_path = resources_dir / f"{inner_key}.yml" inner_resource = _build_job_resource( - inner, inner_key, extra_notebooks_for_augment=workflow.notebooks, hoisted_globals=hoisted_global_names + inner, inner_key, extra_notebooks_for_augment=workflow.notebooks, hoisted_globals=wf_hoisted_globals ) inner_yml_path.write_text( yaml.dump( @@ -562,7 +574,7 @@ def _write_generated(notebooks: list[DabNotebook]) -> None: manual_schedule_time_of_day=manual_schedule_time_of_day_configs, manual_credentials=manual_credential_configs, neutralized_conditions=list(_neutralized_conditions), - hoisted_global_variables=hoisted_global_variables, + hoisted_global_variables=hoisted_global_union, pydabs_dbt_factories=pydabs_dbt_factory_configs, airflow_backfills=airflow_backfill_configs, skipped_pipelines=list(skipped_pipelines or []), diff --git a/tests/unit/test_bundler.py b/tests/unit/test_bundler.py index 5d50144..0957488 100644 --- a/tests/unit/test_bundler.py +++ b/tests/unit/test_bundler.py @@ -9,9 +9,10 @@ from flowx.bundler.dab_writer import ( _load_report, write_bundle, + write_bundle_group, ) from flowx.bundler.dab_writer import main as dab_main -from flowx.models.dab import SecretInstruction, SetupTask +from flowx.models.dab import DabNotebook, SecretInstruction, SetupTask from flowx.models.ir import ( CopyActivity, IfConditionActivity, @@ -1590,3 +1591,69 @@ def test_prereqs_not_empty_when_only_hoisted_globals(self): assert "Factory global parameters (bundle variables)" in md assert "Security note" in md assert "env=" in md + + +class TestHoistedGlobalsAcrossGroupedWorkflows: + """When several workflows share one bundle (single / per-group modes), global-parameter hoisting + must be UNION at the bundle level (databricks.yml variables + SETUP.md declare every workflow's + globals) but PER-WORKFLOW at each job (a notebook widget binds to ${var.X} only in the pipelines + that actually declare X). Regression: the multi-workflow merge previously applied a single leaked + workflow's hoisted set (the last one) to every job and to databricks.yml.""" + + def _hoisted_workflow(self, name: str, global_name: str) -> PreparedWorkflow: + # A generated notebook that reads BOTH globals as widgets, so per-job binding is observable: + # the owning workflow's global binds to ${var.X}, the other stays "". + notebook = DabNotebook( + relative_path=f"notebooks/{name}.py", + content=("env_a = dbutils.widgets.get('env_a')\nenv_b = dbutils.widgets.get('env_b')\n"), + ) + return PreparedWorkflow( + name=name, + tasks=[{"task_key": "run", "notebook_task": {"notebook_path": f"../src/notebooks/{name}.py"}}], + notebooks=[notebook], + secrets=[], + setup_tasks=[], + bundle_variables={global_name: {"description": f"Factory global '{global_name}'.", "default": "prod"}}, + ) + + def test_bundle_variables_are_the_union_across_workflows(self, tmp_path): + wfs = [self._hoisted_workflow("pl_a", "env_a"), self._hoisted_workflow("pl_b", "env_b")] + write_bundle_group(wfs, tmp_path, bundle_name="grouped") + variables = yaml.safe_load((tmp_path / "databricks.yml").read_text())["variables"] + # Union: BOTH pipelines' globals are declared, not just the last workflow's. + assert "env_a" in variables + assert "env_b" in variables + + def test_setup_md_lists_every_workflows_globals(self, tmp_path): + wfs = [self._hoisted_workflow("pl_a", "env_a"), self._hoisted_workflow("pl_b", "env_b")] + write_bundle_group(wfs, tmp_path, bundle_name="grouped") + setup = (tmp_path / "SETUP.md").read_text() + assert "env_a" in setup + assert "env_b" in setup + + def test_each_job_binds_only_its_own_global(self, tmp_path): + wfs = [self._hoisted_workflow("pl_a", "env_a"), self._hoisted_workflow("pl_b", "env_b")] + write_bundle_group(wfs, tmp_path, bundle_name="grouped") + + job_a = yaml.safe_load((tmp_path / "resources" / "pl_a.yml").read_text())["resources"]["jobs"]["pl_a"] + base_a = job_a["tasks"][0]["notebook_task"]["base_parameters"] + # pl_a declares only env_a -> its widget binds to the var; env_b is not its global -> "". + assert base_a["env_a"] == "${var.env_a}" + assert base_a["env_b"] == "" + + job_b = yaml.safe_load((tmp_path / "resources" / "pl_b.yml").read_text())["resources"]["jobs"]["pl_b"] + base_b = job_b["tasks"][0]["notebook_task"]["base_parameters"] + assert base_b["env_b"] == "${var.env_b}" + assert base_b["env_a"] == "" + + def test_single_workflow_group_unchanged(self, tmp_path): + # With one workflow the union equals that workflow's set -> byte-identical to per-pipeline. + wf = self._hoisted_workflow("pl_solo", "env_a") + write_bundle_group([wf], tmp_path, bundle_name="pl_solo") + variables = yaml.safe_load((tmp_path / "databricks.yml").read_text())["variables"] + assert "env_a" in variables + assert "env_b" not in variables + job = yaml.safe_load((tmp_path / "resources" / "pl_solo.yml").read_text())["resources"]["jobs"]["pl_solo"] + base = job["tasks"][0]["notebook_task"]["base_parameters"] + assert base["env_a"] == "${var.env_a}" + assert base["env_b"] == "" From c57bfaa69e5e6c238db5fa8ef94bdd1f22e30e68 Mon Sep 17 00:00:00 2001 From: Zanita Rahimi Date: Fri, 11 Sep 2026 16:08:08 +0200 Subject: [PATCH 8/9] Fix missing-CLI guard breaking deployer tests without databricks on PATH --- skills/flowx-deploy/SKILL.md | 2 +- src/flowx/bundler/deployer.py | 29 +++++++++++++++-------------- tests/unit/test_deployer.py | 20 ++++++++++++++------ 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/skills/flowx-deploy/SKILL.md b/skills/flowx-deploy/SKILL.md index 45fe8f5..e0900f9 100644 --- a/skills/flowx-deploy/SKILL.md +++ b/skills/flowx-deploy/SKILL.md @@ -1,7 +1,7 @@ --- name: flowx-deploy description: > - Deploy the per-pipeline Databricks Asset Bundles from a multi-pipeline flowx + Deploy the per-pipeline Declarative Automation Bundles from a multi-pipeline flowx migration in dependency order, resolving cross-bundle job ids automatically. Local CLI only. triggers: diff --git a/src/flowx/bundler/deployer.py b/src/flowx/bundler/deployer.py index 960a432..4ac66bc 100644 --- a/src/flowx/bundler/deployer.py +++ b/src/flowx/bundler/deployer.py @@ -25,7 +25,6 @@ import argparse import json import re -import shutil import subprocess import sys from pathlib import Path @@ -215,8 +214,21 @@ def _topo_sort(graph: dict[str, list[str]]) -> list[str]: def _run_cli(cmd: list[str], *, cwd: Path) -> subprocess.CompletedProcess[str]: - """Runs a CLI command, capturing output. Isolated so tests can monkeypatch it.""" - return subprocess.run(cmd, cwd=str(cwd), capture_output=True, text=True) + """Runs a CLI command, capturing output. Isolated so tests can monkeypatch it. + + If the executable is missing (this is a local-CLI operation and ``databricks`` is not on PATH), + surface an actionable message as a non-zero CompletedProcess instead of letting an uncaught + FileNotFoundError escape as a traceback. Callers already branch on ``returncode``. + """ + try: + return subprocess.run(cmd, cwd=str(cwd), capture_output=True, text=True) + except FileNotFoundError: + message = ( + f"`{cmd[0]}` was not found on PATH. Ordered deploy is a local-CLI operation " + "(`databricks bundle deploy`/`summary`); install the databricks CLI, or use --dry-run to " + "preview the deploy order without deploying." + ) + return subprocess.CompletedProcess(cmd, returncode=127, stdout="", stderr=message) def _capture_job_ids( @@ -277,17 +289,6 @@ def run( print(f"Error: output directory not found: {output_dir}", file=sys.stderr) return 1 - # A real deploy shells out to the `databricks` CLI; fail with an actionable message rather than an - # uncaught FileNotFoundError if it isn't on PATH. Skipped for --dry-run, which never invokes it. - if not dry_run and shutil.which("databricks") is None: - print( - "Error: the `databricks` CLI was not found on PATH. This is a local-CLI operation " - "(`databricks bundle deploy`/`summary`); install the CLI, or use --dry-run to preview the " - "deploy order without deploying.", - file=sys.stderr, - ) - return 1 - try: bundles = _discover_bundles(output_dir) except AmbiguousLayoutError as exc: diff --git a/tests/unit/test_deployer.py b/tests/unit/test_deployer.py index b4449cf..d973ffc 100644 --- a/tests/unit/test_deployer.py +++ b/tests/unit/test_deployer.py @@ -230,18 +230,26 @@ def test_empty_output_dir_returns_error(tmp_path): def test_missing_databricks_cli_reports_cleanly(tmp_path, monkeypatch, capsys): - # A real deploy needs the `databricks` CLI on PATH; if it's absent, run() must return 1 with an - # actionable message rather than an uncaught FileNotFoundError from subprocess. + # When `databricks` is not on PATH, subprocess.run raises FileNotFoundError; _run_cli must turn that + # into a non-zero result with an actionable message so run() stops cleanly rather than tracebacks. _make_bundle(tmp_path, "a", jobs=["a"]) - monkeypatch.setattr(deployer.shutil, "which", lambda _name: None) + + def _boom(*_args, **_kwargs): + raise FileNotFoundError(2, "No such file or directory", "databricks") + + monkeypatch.setattr(deployer.subprocess, "run", _boom) assert run(tmp_path) == 1 - assert "databricks` CLI was not found" in capsys.readouterr().err + assert "was not found on PATH" in capsys.readouterr().err def test_missing_databricks_cli_does_not_block_dry_run(tmp_path, monkeypatch): - # --dry-run never shells out, so a missing CLI must not stop it. + # --dry-run never shells out, so even a totally broken subprocess.run must not stop it. _make_bundle(tmp_path, "a", jobs=["a"]) - monkeypatch.setattr(deployer.shutil, "which", lambda _name: None) + + def _boom(*_args, **_kwargs): + raise FileNotFoundError(2, "No such file or directory", "databricks") + + monkeypatch.setattr(deployer.subprocess, "run", _boom) assert run(tmp_path, dry_run=True) == 0 From 0eaab1d788f1dd1a09374e446132e60e8d121755 Mon Sep 17 00:00:00 2001 From: Zanita Rahimi Date: Mon, 14 Sep 2026 18:49:03 +0200 Subject: [PATCH 9/9] add documentation and remove unrelated changes --- README.md | 2 +- docs/content/docs/guide.mdx | 26 ++++++++++++++++++++++++++ src/flowx/bundler/dab_writer.py | 10 ---------- src/flowx/sources/adf/loader.py | 2 -- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 577e064..d9c78df 100644 --- a/README.md +++ b/README.md @@ -209,7 +209,7 @@ Parses the source into typed nodes and classifies each activity/operator as dete Applies deterministic translators (ADF activity registry / Airflow operator mapping), resolves dependencies, and records unresolved gaps. ADF supports its guided agentic translation workflow. Airflow supports a fingerprint-bound, explicitly reviewed leaf-gap workflow whose constrained provider output is replayed against an immutable deterministic baseline before packaging. Produces the shared Pipeline IR consumed unchanged by the package phase. ### Phase 3: Package -Converts Pipeline IR into a deployable DABs project: `databricks.yml`, per-job YAML resource files, generated Python notebooks, and setup scripts for UC volumes, secrets, and connections. +Converts Pipeline IR into a deployable DABs project: `databricks.yml`, per-job YAML resource files, generated Python notebooks, and setup scripts for UC volumes, secrets, and connections. A multi-pipeline factory can be laid out into bundles with `--packaging-mode` (`per-pipeline` default / `single` / `per-group`); a top-level `DEPLOY.md` records the suggested callees-first deploy order, and `python -m flowx.adapter deploy` deploys the bundles in that order, wiring cross-bundle job ids automatically. ## Output Format diff --git a/docs/content/docs/guide.mdx b/docs/content/docs/guide.mdx index 821e9da..5bfb374 100644 --- a/docs/content/docs/guide.mdx +++ b/docs/content/docs/guide.mdx @@ -120,6 +120,32 @@ databricks bundle deploy --target Bundles created by flowx are standard Declarative Automation Bundles. You can target different environments, integrate with CI/CD, or further customize the YAML before deploying. See the [Declarative Automation Bundles documentation](https://docs.databricks.com/aws/en/dev-tools/bundles/) for more information. + +### Packaging modes + +A multi-pipeline migration can be laid out into bundles in three ways, selected with `--packaging-mode` on `package`: + +| Mode | Layout | +| --- | --- | +| `per-pipeline` (default) | One bundle per pipeline, each in its own subdirectory. | +| `single` | Every pipeline in one bundle at the output root. | +| `per-group` | Pipelines grouped into bundles — `--group-by inferred` (default) groups by the Run Pipeline (`ExecutePipeline`) call graph; `--group-by spec` reads an explicit mapping from `--group-spec`. | + +When a pipeline calls another (`ExecutePipeline`) that lands in a **different** bundle, the caller's `run_job_task` job id is rewritten to a bundle variable (`${var._job_id}`) and wired at deploy time. A top-level `DEPLOY.md` lists every bundle, its cross-bundle dependencies, and the suggested **callees-first** deploy order. + +### Deploying multiple bundles in order + +For a multi-bundle migration, the ordered deployer resolves the cross-bundle job ids for you rather than deploying each bundle by hand: + +```bash +python -m flowx.adapter deploy --output-dir ./flowx_output --target +``` + +It discovers the bundles, deploys callees before callers, reads each deployed job's numeric id from `databricks bundle summary`, and injects it into callers via `--var _job_id=`. Add `--dry-run` to print the deploy order without deploying. + + +`databricks bundle deploy` / `summary` are not available on Databricks serverless / Genie Code. Run the deploy step from a local CLI session or the web terminal. + diff --git a/src/flowx/bundler/dab_writer.py b/src/flowx/bundler/dab_writer.py index fd7548e..ba8883c 100644 --- a/src/flowx/bundler/dab_writer.py +++ b/src/flowx/bundler/dab_writer.py @@ -63,16 +63,6 @@ from flowx.utils import normalize_task_key -class MalformedReportError(Exception): - """A translation report contained an entry flowx could not have produced. - - Reports are machine-generated by ``engine.py`` (``_pipeline_to_dict`` always - emits ``name`` + ``tasks``), so a non-conforming entry signals corruption or - an internal bug. Raising -- rather than silently dropping the entry -- keeps - ``package`` from emitting a bundle that is quietly missing a pipeline. - """ - - class _BundleYamlDumper(yaml.SafeDumper): """YAML dumper that leaves keys unquoted and only quotes values when needed.""" diff --git a/src/flowx/sources/adf/loader.py b/src/flowx/sources/adf/loader.py index 4248108..af9917a 100644 --- a/src/flowx/sources/adf/loader.py +++ b/src/flowx/sources/adf/loader.py @@ -59,8 +59,6 @@ "ExecuteDataFlow", "Until", "SqlServerStoredProcedure", - # ADF exports the type as "AzureFunctionActivity"; the bare alias covers pre-normalized inputs. - "AzureFunctionActivity", "AzureFunction", "WebHook", "Custom",