Skip to content

Bundle packaging modes + ordered multi-bundle deploy - #73

Open
zanitarahimi wants to merge 10 commits into
mainfrom
port/pr32-ordered-deploy
Open

zanitarahimi wants to merge 10 commits into
mainfrom
port/pr32-ordered-deploy

Conversation

@zanitarahimi

@zanitarahimi zanitarahimi commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Bundle packaging modes + ordered multi-bundle deploy

Implements the bundle-packaging design and the ordered auto-deploy follow-up.

Packaging modes

  • --packaging-mode: per-pipeline (default) / single / per-group; per-group supports --group-by inferred (Run Pipeline call graph) and --group-by spec.
  • Run Pipeline dependencies tracked in bundler/pipeline_graph.py.
  • A single top-level DEPLOY.md records the suggested callees-first deploy order for every mode.
  • Within each bundle: generated code + setup scripts de-duped, one SETUP.md.

Ordered deploy

  • flowx deploy (bundler/deployer.py) discovers the bundles, orders them callees-first, deploys each with databricks bundle deploy, reads each deployed job's numeric id from bundle summary, and injects it into callers via --var <callee>_job_id=<id> — no manual job-id wiring.

Global-param hoisting across grouped workflows

  • When several workflows share one bundle (single / per-group), global-parameter hoisting is union at the bundle level (databricks.yml variables: + SETUP.md) and per-workflow at each job (a widget binds to ${var.X} only in pipelines that declare X). Single-workflow bundles are byte-identical to before. Adds TestHoistedGlobalsAcrossGroupedWorkflows.

Testing

  • make fmt (ruff + mypy) clean; make test and make integration green.
  • Ordered-deploy validated on a real multi-pipeline factory: callees deploy first and each caller's ${var.<callee>_job_id} resolves to the deployed job id.

matthewmoorcroft and others added 8 commits September 7, 2026 17:55
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
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
- 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.
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.

@ghanse ghanse left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks good. I left a few comments. It would be good to add documentation for this feature.

Comment thread src/flowx/bundler/dab_writer.py Outdated
Comment thread src/flowx/sources/adf/loader.py Outdated
Comment thread src/flowx/bundler/dab_writer.py
Comment thread src/flowx/bundler/dab_writer.py
Comment thread src/flowx/bundler/pipeline_graph.py
Comment thread src/flowx/bundler/deploy_writer.py Outdated
Comment thread src/flowx/bundler/deployer.py
Comment thread tests/unit/test_packaging_modes.py
Comment thread src/flowx/bundler/deployer.py

@ghanse ghanse left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Strong, well-tested PR overall. One change I'd like before approving:

deploy_writer.py — single-bundle DEPLOY.md. It tells the operator there's no cross-bundle ordering to worry about and to just run databricks bundle deploy, but a single bundle can still carry a ${var.<callee>_job_id} reference to a pipeline outside the migration (declared with no default). In that case a plain deploy fails on the unset var, and flowx deploy errors with MissingDependencyError unless --allow-missing-deps. Please add a note covering that external-reference case (see the inline comment).

The other inline comments — the duplicate topo sort / CycleError, and the loud-failure test gaps — are non-blocking.

@zanitarahimi
zanitarahimi requested a review from ghanse September 23, 2026 12:26

@ghanse ghanse left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks very good overall. One finding:

dab_writer.py _namespace_bundle_artifacts breaks dbt-factory pipelines in grouped modes. Step 2 prefixes every notebook path including the resources/*.py PyDABs hook modules, but _collect_pydabs_resource_entries still registers the old resources.<module> path. The file lands at resources/<prefix>/<module>.py while python.resources points at resources.<module>, and bundle deploy can't import the hook.

Confirmed this with a small repro: a dbt pipeline + any second pipeline in a single/per-group bundle is enough because namespacing fires whenever there's more than 1 workflow. The airflow path (_namespace_workflow_assets) skips resources/ for this reason. Needs a fix + a test.

@zanitarahimi

Copy link
Copy Markdown
Contributor Author

Looks very good overall. One finding:

dab_writer.py _namespace_bundle_artifacts breaks dbt-factory pipelines in grouped modes. Step 2 prefixes every notebook path including the resources/*.py PyDABs hook modules, but _collect_pydabs_resource_entries still registers the old resources.<module> path. The file lands at resources/<prefix>/<module>.py while python.resources points at resources.<module>, and bundle deploy can't import the hook.

Confirmed this with a small repro: a dbt pipeline + any second pipeline in a single/per-group bundle is enough because namespacing fires whenever there's more than 1 workflow. The airflow path (_namespace_workflow_assets) skips resources/ for this reason. Needs a fix + a test.

Hey @ghanse on the dbt hook namespacing fix: do you want me to just leave the hook module where it is (so resources/.py still matches python.resources), or namespace it like _namespace_workflow_assets does?

@ghanse

ghanse commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Looks very good overall. One finding:
dab_writer.py _namespace_bundle_artifacts breaks dbt-factory pipelines in grouped modes. Step 2 prefixes every notebook path including the resources/*.py PyDABs hook modules, but _collect_pydabs_resource_entries still registers the old resources.<module> path. The file lands at resources/<prefix>/<module>.py while python.resources points at resources.<module>, and bundle deploy can't import the hook.
Confirmed this with a small repro: a dbt pipeline + any second pipeline in a single/per-group bundle is enough because namespacing fires whenever there's more than 1 workflow. The airflow path (_namespace_workflow_assets) skips resources/ for this reason. Needs a fix + a test.

Hey @ghanse on the dbt hook namespacing fix: do you want me to just leave the hook module where it is (so resources/.py still matches python.resources), or namespace it like _namespace_workflow_assets does?

Namespace it. We may want to reuse _namespace_workflow_assets or promote it to a shared helper so that we don't maintain 2 separate entry-points for this.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants