Skip to content

orchestration: run the pipeline on Argo, one tile per pod - #19

Closed
danohu wants to merge 7 commits into
cornerstone-data:zarr-cache-complete-markerfrom
AdAstraEco:orchestration-core
Closed

danohu wants to merge 7 commits into
cornerstone-data:zarr-cache-complete-markerfrom
AdAstraEco:orchestration-core

Conversation

@danohu

@danohu danohu commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Stacked on #16 (zarr .complete marker): the docs here rely on a killed pod leaving no zarr that passes for a result. Review #16 first; this PR's diff is only the orchestration work.

What

An optional layer for running the pipeline on Kubernetes with Argo Workflows, one 10° tile per pod, so that a country or a global backfill takes hours rather than days. jdluc runs unmodified inside the pods; Argo takes over only sequencing, fan-out, retries and visibility. docs/orchestration.md has the design and setup.

  • orchestration package, one subcommand per workflow task: plan, ingest-tile, compute, trace
  • jdluc/export.py + trace.export_workflow: write the emissions-factor table, with provenance, to a new EXPORT_ROOT
  • orchestration/workflows/cornerstone.example.yaml: an Argo WorkflowTemplate, one DAG per tile, with cloud-neutral placeholders
  • Dockerfile, .dockerignore
  • an import-linter contract (jdluc never imports orchestration), and mypy over orchestration

Design notes

  • orchestration may not change a result. Every cache-key argument goes straight through to jdluc, with jdluc's own defaults.
  • Ingest is separate from compute. Only ingest pods carry source-API keys; compute runs harmonize with skip_ingest=True from a warm INGEST_ROOT. That needs the list of datasets a run reads, which orchestration/plan.py keeps by hand.
  • A failed tile fails only itself, and trace runs only once every tile has succeeded. argo retry resumes from the cache.
  • Extensible without edits: __main__.build_parser() returns the parser and its subparsers, plan.get_task_to_items() returns the plan lists without writing them, and the template's tasks can be called with templateRef.

Heads-up

EXPORT_ROOT is a new .env key, and Config.from_dot_env() requires every key, so an existing .env needs it added (see .env.example).

Verification

  • pytest passes, including 18 new tests in orchestration
  • pre-commit run --all-files passes (ruff, import-linter, mypy, mdformat)
  • argo lint --offline on the example template
  • Not yet: a full Argo run of this template, and a fresh docker build

For zarr files, set up a .complete marker file to check when data is
fully written.

This replaces relying on zarr.json -- that doesn't work because
zarr.json is the first to be written. This might happen from a failed or
aborted calculation, or because multiple processes are trying to write
the same file (e.g. running 2 countries which share a tile)

A partially-written zarr might be not cause any warning signs later,
since it might be techncially valid in spite of missing data.

Now an empty .complete marker is written beside the zarr after its last chunk, and the
entry counts as cached only once the marker exists. A zarr found without its marker is a
write that was killed part-way or is still running: serialize raises FileExistsError and
leaves it for someone to resolve by hand, usually by deleting the zarr, rather than
writing over it and risking a race with a live writer.

Entries written before this change have no marker, so each raises once until deleted.
An autouse fixture in jdluc/__tests__/conftest.py stubs Config.from_dot_env
for every test not marked integration. Each field gets its type's empty value,
with two dask workers, so a unit test that reaches for a storage root fails
instead of touching a bucket. Integration tests still read the real .env.

This replaces the fixture storage_test carried for itself, so the next unit
test that writes a zarr needs no copy of it, and a new Config field needs no
edit to the tests.
An optional `orchestration` package with one subcommand per workflow task
(plan, ingest-tile, compute, trace), and an example Argo WorkflowTemplate
that runs each tile's ingest -> compute as a DAG of its own, so a failed
tile stops only itself. jdluc runs unmodified in the pods; every
cache-key argument passes straight through, and an import-linter contract
keeps jdluc from importing orchestration.

trace writes the emissions-factor table to a new EXPORT_ROOT, through a
small `export.write_table` utility.

The package is built to be wrapped rather than edited: `build_parser`
and `run` let a wrapper add subcommands, `plan.get_task_to_items` returns
the lists for it to add to before `write_task_to_items`, and the
template's tasks can be called by templateRef. See docs/orchestration.md.

Also adds the Dockerfile the workflow's image is built from.
get_name no longer mentions mosaics, get_task_to_items describes the flat lists
without a flat workflow to point at, and trace's --help is a whole sentence.
Comment thread .env.example
# EXPORT_ROOT is where terminal deliverables land (the emissions-factor table): not a cache
# the pipeline reads back. Any fsspec URI, like the roots below.
# EXPORT_ROOT = '/absolute/path/to/export'
EXPORT_ROOT = 'gs://cornerstone-emissions-us-central1'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

EXPORT_ROOT is the active line here and it points at the real bucket. Anyone copying this file and running trace would write straight into it. Could we make the placeholder the active line and keep the bucket commented out?

resources: { requests: { storage: 150Gi } }

# harmonize -> emit -> attribute for one tile, from the warm ingest cache, without keys
- name: compute

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No retryStrategy on any template, so a reclaimed spot node means a manual argo retry. Maybe retryStrategy: {limit: 2, retryPolicy: OnError} on ingest-tile and compute? The cache makes retries cheap.

Comment thread Dockerfile
RUN uv sync --frozen --no-dev

# .git in its own last layer, so a commit that changes no source rebuilds only this layer.
COPY .git .git

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This copies the whole .git into the image. If it's built from a clone with a token in the remote URL, the token ends up in the image (get_git_remote_url reads that URL). Could we pass the SHA and remote as build args instead, or strip the credentials? Also, the container runs as root, and the safe.directory line above suggests a non-root user was intended.

deliverable_name becomes product_name in write_table and get_table_output_uri,
matching the cornerstone-product-name tag that get_metadata already writes from it.
danohu added a commit to AdAstraEco/cornerstone_luc that referenced this pull request Oct 19, 2026
orchestration: run the pipeline on Argo, one tile per pod (cornerstone-data/luc#19): write_table names its product

# Conflicts:
#	jdluc/export.py
danohu added a commit to AdAstraEco/cornerstone_luc that referenced this pull request Oct 19, 2026
Brings in cornerstone-data/luc#19's write_table product_name rename.
@qudratk

qudratk commented Oct 20, 2026

Copy link
Copy Markdown

@danohu, One more thing building on @tankoyarik comment about .git/credentials in the Dockerfile: export.get_metadata() stamps utils.get_git_remote_url() into cornerstone-remote-url on every exported table's attrs, and the new test confirms attrs survives the to_parquet/read_parquet round-trip. So if the image is ever built from a clone whose remote URL carries a token, that credential doesn't just sit in the image. It propagates into the metadata of every emissions-factors parquet handed downstream. Worth stripping credentials from the URL before it's used as provenance, not just before it's baked into the image.

@danohu
danohu marked this pull request as draft October 22, 2026 05:19
@danohu danohu closed this Nov 11, 2026
@danohu
danohu force-pushed the zarr-cache-complete-marker branch from d033289 to f9daffd Compare November 11, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants