From 1ce332fa1baf8be45c8fa74901108696e3153c91 Mon Sep 17 00:00:00 2001 From: ILay Date: Wed, 26 Aug 2026 19:21:41 +0200 Subject: [PATCH] [SIMPL-30451] Correct stale references in the guide and verify both images build The rename of process_work_units and _results_from_pipes during the fan-out refactor left three references behind, in the guide, the checklist and the loosely coupled module docstring. The guide also showed loosely_coupled_k8s_job without its executor_def and claimed the pipes client replaced the executor, which stopped being true once the graph fanned out. Both images were built and smoke tested: the code location image loads its definitions, and the payload image ships dagster_pipes without dagster. That check is now recorded as L11 in the readiness checklist, so the isolation argument is verified at image level and not only in the source. Changelog: fixed --- README.md | 4 ++++ .../user-guide/distributed-execution-guide.md | 13 ++++++++++--- documents/user-guide/readiness-checklist.md | 17 ++++++++++------- .../loosely_coupled/jobs.py | 2 +- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 55e2cb6..30e4103 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,10 @@ docker build -t distributed-execution:0.1.0 . docker build -f payload/Dockerfile -t distributed-execution-payload:0.1.0 payload/ ``` +Both images build and have been smoke tested locally: the code location image +loads its definitions, and the payload image contains `dagster_pipes` without +`dagster` — check L11 in the readiness checklist. + ## Status Both execution targets are implemented. The tightly coupled jobs and the loosely diff --git a/documents/user-guide/distributed-execution-guide.md b/documents/user-guide/distributed-execution-guide.md index f51c13a..09ffefe 100644 --- a/documents/user-guide/distributed-execution-guide.md +++ b/documents/user-guide/distributed-execution-guide.md @@ -312,16 +312,22 @@ def dispatch_external_work_k8s( return _result_from_pipes(context, completed) ``` -and the job supplies the client as a resource instead of an executor: +and the job supplies the client as a resource: ```python loosely_coupled_k8s_job = loosely_coupled_k8s_reference.to_job( name="loosely_coupled_k8s_job", + executor_def=multiprocess_executor.configured({"max_concurrent": 2}), resource_defs={"pipes_k8s_client": PipesK8sClient()}, tags={**COMMON_TAGS, "transport": "k8s_pod_logs"}, ) ``` +The executor is still present, but its meaning has changed. It no longer decides +where the work runs — the payload image and `namespace` do that. It only bounds +how many dispatching ops wait on the message channel at once. The two decisions +that were fused in the tightly coupled setup are now independent. + #### What changes operationally | | Tightly coupled | Loosely coupled | @@ -366,7 +372,7 @@ distinct external workers, which is the loosely coupled equivalent of the reference uses. It needs no extra infrastructure, but it couples message delivery to the pod log stream — a log shipper that intercepts or truncates stdout will break reporting while the workload still exits 0. That is why -`_results_from_pipes` raises on an empty message list rather than returning an +`_result_from_pipes` raises on an empty message list rather than returning an empty result. If your cluster's logging setup makes that transport unreliable, switch to an object-storage message reader, which needs a bucket plus credentials on both sides. @@ -462,7 +468,8 @@ Tracked under SIMPL-30451. | Before-and-after example, switching to loosely coupled | AC4 | Complete | | Runnable tightly coupled reference implementations | AC4 | Complete | | Runnable loosely coupled reference implementation | AC4 | Complete (subprocess verified, K8s not cluster-run) | -| Payload image published to the registry | Tech details | **Pending** | +| Both images build; payload image passes the isolation check (L11) | Tech details | Complete | +| Payload image published to the container registry | Tech details | **Pending** — needs the CI build job | | End-to-end run of `loosely_coupled_k8s_job` on a cluster | Tech details | **Pending** | | Screenshots of UI surfaces | Tech details | **Pending** | | Platform architecture document update | Tech details | **Pending** | diff --git a/documents/user-guide/readiness-checklist.md b/documents/user-guide/readiness-checklist.md index 7681b2c..d6e5956 100644 --- a/documents/user-guide/readiness-checklist.md +++ b/documents/user-guide/readiness-checklist.md @@ -9,8 +9,9 @@ location image. > **NOT CLUSTER-VERIFIED.** Checks L4–L9 and the Kubernetes rows of section 4.3 > are derived from the implemented reference but have not yet been run against a -> Simpl cluster. Checks L1–L3 and L10, which exercise the payload contract, the -> message-parsing path and per-unit dispatch, are covered by the test suite. +> Simpl cluster. Checks L1–L3, L10 and L11, which exercise the payload contract, +> the message-parsing path, per-unit dispatch and image-level isolation, are +> verified locally. --- @@ -52,9 +53,10 @@ location image. | L8 | Payload has **no** orchestration connectivity | Same run; inspect run logs | No `Payload could see orchestration runtime credentials` warning. This is a positive check — absence of errors is not sufficient | | L9 | Work ran off-platform | Same run | `summarise_results` metadata shows `contributing_hosts` containing the payload pod names, not the run worker's hostname | | L10 | One workload dispatched per unit | Same run, or `uv run pytest -k dispatched_to_its_own` locally | `contributing_workers` has one entry per unit; the local test asserts four distinct external workers | +| L11 | Payload **image** carries no orchestration dependency | `docker run --rm python -c "import importlib.util; print(importlib.util.find_spec('dagster') is not None)"` | Prints `False`. L1 proves the *source* does not import `dagster`; this proves the shipped image does not contain it either | -Checks L4–L9 require a cluster. L1–L3 and L10 run on a laptop and should gate -every change to the payload or the dispatching op. +Checks L4–L9 require a cluster. L1–L3, L10 and L11 run on a laptop and should +gate every change to the payload or the dispatching op. --- @@ -90,7 +92,7 @@ every change to the payload or the dispatching op. | Op hangs until `pod_wait_timeout` (default 24 h) | Payload Job never scheduled — quota, node selector or image pull | Check `kubectl -n describe job `; lower `pod_wait_timeout` so the failure surfaces quickly | | `403 Forbidden` creating the Job | Dispatcher service account lacks Job create permission | Apply `yaml/loosely-coupled/rbac-pipes-dispatch.yaml` in the **payload** namespace | | Warning: `Payload could see orchestration runtime credentials` | Payload pod inherited run-pod env or a Vault annotation | Remove the inherited env; the payload should receive only what `extras` and explicit `env` pass it | -| Payload exits non-zero but the run reports success | Exit status not being checked, or messages read before failure | Confirm the dispatching op returns through `_results_from_pipes`; do not swallow `PipesClientCompletedInvocation` errors | +| Payload exits non-zero but the run reports success | Exit status not being checked, or messages read before failure | Confirm the dispatching op returns through `_result_from_pipes`; do not swallow `PipesClientCompletedInvocation` errors | | Payload receives no `units` | `extras` key mismatch between dispatcher and `pipes.get_extra()` | Both sides must use the same key; a typo yields a `KeyError` inside the payload | | Run cancelled in the UI, payload pod keeps running | Cancellation is not propagated to dispatched workloads automatically | `delete_pod_on_completion` handles the normal path; for cancellation, verify orphaned Jobs and add a cleanup sensor | @@ -108,8 +110,9 @@ change record: 1. The run ID and its final status. 2. The `report_execution_target` output metadata block (pod identity, namespace, env var presence). -3. The `summarise_results` metadata block (`contributing_hosts`), which proves - which execution target was actually used. +3. The `summarise_results` metadata block (`contributing_hosts` and + `contributing_workers`), which proves which execution target was actually + used and that the fan-out reached it. 4. For `k8s_job_executor`, the output of `kubectl get jobs -l dagster/run-id=`. 5. For the loosely coupled target, the payload image digest and the run log line emitted by `pipes.log` — together they prove which payload version ran and diff --git a/src/distributed_execution/loosely_coupled/jobs.py b/src/distributed_execution/loosely_coupled/jobs.py index ac1687a..1e9c760 100644 --- a/src/distributed_execution/loosely_coupled/jobs.py +++ b/src/distributed_execution/loosely_coupled/jobs.py @@ -6,7 +6,7 @@ dagster-pipes protocol, which Dagster materialises as logs and metadata. The payload never connects to the metadata database. Two transports are provided. Both dispatch the *same* payload script and produce -the same output shape as the tightly coupled ``process_work_units`` op, so the +the same output shape as the tightly coupled ``process_work_unit`` op, so the graphs differ by exactly one node - see the guide's section 5.3. """