Skip to content

docs: ADR for migration - #4

Open
aldbr wants to merge 1 commit into
mainfrom
adr_migration
Open

docs: ADR for migration#4
aldbr wants to merge 1 commit into
mainfrom
adr_migration

Conversation

@aldbr

@aldbr aldbr commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@amaltaro amaltaro left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hello, as we discussed last week during the hackathon, I wanted to leave some feedback on "IC-ADR-002_integration_tests.md" file provided in this PR:

  • do we already have a unit test that tests HTCondor >= 25.8, ensuring it is catching the double fetch issue?
  • for the credentials setup, is it already in place? should it be a pre-job and then each backend job just uses that file, which is destroyed at the tear down of the full suite of tests?
  • how to deal with backend change of APIs (imagine that HTCondor 27 accepts a new parameter, while 26 does not)?
  • I might have missed the definition of "leading-edge" and "anchor". It would be helpful to explicitly define it, if not yet.
  • I would suggest making a table with minimal version supported for each backend (is it the "anchor" concept?)

I am just getting started with Dirac workflows, so I might have missed things that are already defined and/or that I should know.
However, I have to say that it was hard to follow this document, it is full of (important) details. Perhaps a summary section would be positive (or having it directly in the README or so.

Please let me know if you would like feedback on anything else in specific. Thanks for this thorough document!

@fstagni fstagni left a comment

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 is just a very initial review. I will review again the updated version.

Comment thread docs/notes/reuse-survey.md Outdated
closest HEP analogue and does *both* grid-CE and direct-batch — but it is Java, never released as
an artifact (CVMFS-only), coupled to JAliEn's LDAP/central/token machinery, and of unconfirmed
licence.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You might as well add DIRAC.Resources.Computing ?

Comment on lines +112 to +113
- **One contract, many backends**, with *combinations* (`SSH + Slurm`, `SSH + HTCondor`,
`Local + HTCondor`) as first-class.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can combinations be something else then 2-tiered?

- **Out of scope — execute-here-and-now:** running a payload *in this process* on a worker node
(DIRAC's `InProcess`/`Singularity`/`Pool`). This is the pilot/worker-node domain (see §6).
- **Out of scope — orchestration:** pull vs push, matching, pilot lifecycle. interCEde exposes
mechanism; DiracX/DIRAC decides policy.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

DiracX/DIRAC or just "DiracX"?
If we think this library will ever be used outside of DiracX we can also say "DiracX or alternative users/clients".

providers, validated against containerised backends. This ADR fixes the shape of that interface with
five decisions:

1. **The contract is a set of small typed interfaces (`typing.Protocol`s), not a base class.**

Copy link
Copy Markdown

Choose a reason for hiding this comment

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


### 2. The contract — capability-segmented Protocols

The caller-facing contract is a set of small `typing.Protocol`s, and **every operation works on

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You mean

Suggested change
The caller-facing contract is a set of small `typing.Protocol`s, and **every operation works on
The caller-facing contract is a small set of `typing.Protocol`s, and **every operation works on

?

The contract is sized by its consumers. DiracX splits the old monolithic SiteDirector into
independent, separately-scheduled **tasks** — one submits, one polls status, one fetches outputs
(§9 sketches them). The **essential** capabilities of a CE are two — *submit a payload (with
inputs)* and *get the jobs' status* — each driven by its own task. *Retrieving outputs* is a

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"input" and "output" here are only "input sandbox" and "output sandbox". Just make sure there's no way it's interpreted differently

Comment thread docs/adr/IC-ADR-001_computing_elements.md
@runtime_checkable
class OutputRetriever(Protocol): # the (on-demand) output task — OPTIONAL
# Bulk: materialise each job's whole output sandbox (incl. the CE/scheduler log) into
# `dest`. No separate log fetch — the log is a manifest member. Retrieval is IDEMPOTENT

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what is an example of dest?

Comment thread docs/adr/IC-ADR-001_computing_elements.md Outdated
- **Bulk and asynchronous everywhere.** Submit, status, kill, purge and retrieval all take many jobs and return a result per job.
- **One contract, many backends**, with combinations treated as first-class.
- **Per-entity values at submission time.** One submission produces many jobs, and the caller has to be able to give each of them its own identity and its own secret.
- **The caller says where the data goes.** A backend that cannot move data to storage has to say so before the job runs, not after it finishes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
- **The caller says where the data goes.** A backend that cannot move data to storage has to say so before the job runs, not after it finishes.
- **The backend advertise data movement capability.** A backend that cannot move data to storage has to say so before the job runs, not after it finishes.


### 1. Scope: what interCEde is and is not

interCEde is the delegate-and-poll side of running work on a resource. You submit a payload to a scheduler or a resource, you get a handle, and then you poll it, fetch its sandbox, or kill it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"scheduler or a resource". Before "backend" was used. Are they all the same thing? I can guess not, but what about starting with an explanation of what is what?

from pathlib import Path

from pydantic import BaseModel

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What are these? Examples? Why they are here?

tag: str | None = None # consumer token for correlation and debugging only
```

**What "inputs" and "outputs" mean here.** They are the files the job reads and writes in its own working directory, and nothing else. `FileRef.name` and `OutputMember.pattern` always name a file in that directory. What may reach outside it is where a file comes from and where it goes: a source can be a URL the resource downloads, and a destination can be a URL the resource uploads to. So the inputs and outputs are the job's sandbox, while their sources and destinations may be remote storage. Neither field describes the payload's own data management: interCEde receives URLs the caller has already resolved (§1).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FileRef and OutputMember are not the best names, IMHO. Especially if they could be called simply InputFile and OutputFile.

Comment on lines +129 to +131
class Submission(BaseModel):
handles: Sequence[JobHandle] # handles[i] belongs to copies[i]
failures: Mapping[int, str] # copy index -> reason, for copies the backend refused

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what's a "copy"?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see it explained later, but while reading in order this is not clear.


#### 2.2 stdout and stderr

stdout and stderr are named members of the output sandbox. The caller may name them, and when it does not the backend picks a name that is unique per job.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"name" == "file name"?

Comment on lines +78 to +81
cpus: int = 1
memory_mb: int | None = None
wall_time_s: int | None = None
queue: str | None = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

these are insufficient

- **`destination` is a URL** means the resource uploads the file itself, to a location the caller has already resolved. The bytes never pass through the consumer, so no temporary directory is needed for them.
- `inputs` works the same way in reverse. A `FileRef` whose source is a local path is staged by interCEde. A `FileRef` whose source is a URL is fetched by the resource.

Not every backend can do both. A batch system reached over SSH has no data stager of its own, so it can only keep files for collection. A cloud provider that boots a VM can do neither. The type system cannot express "this spec is compatible with this backend", so the backend validates the spec when `submit()` is called and refuses it there. That is a runtime check by necessity, and refusing at submit is the whole point: a job that cannot deliver its output should never start.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we actually have use cases when we want the resource to move the output or get the inputs?


### 3. The contract: capability-segmented protocols

The rule of this ADR is never to force a backend to stub a capability it does not have.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What does it mean?

class JobBackend(Submitter, StatusReporter, Protocol): ...
```

`JobBackend` names a complete backend, and it is what the registry returns and validates. Consumers depend on the narrow protocol they use rather than on `JobBackend`: the submission task takes a `Submitter`, the status task a `StatusReporter`, and the output task an `OutputRetriever`, which it has to confirm structurally because a backend may not have one.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what is a "Registry" here?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In general, this phrase is very difficult to understand.
What's a "narrow protocol"?
What's a "task" in this context?
...

Comment on lines +16 to +18
interCEde's contract is a set of structural protocols, and its unit tests exercise them against fakes. A structural protocol and a fake agree with each other forever, so the bugs interCEde exists to absorb are the ones unit tests cannot see.

This ADR therefore adds integration tests that run real backend daemons, in the combinations interCEde supports, on every pull request, inside a standard CI runner. The unit of everything is a **stack**: a named, self-contained backend environment described by one container-compose file plus versioned configuration, declared in a single manifest from which the CI matrix is generated. One backend-agnostic test suite is dispatched across stacks by capability. Every version is pinned and tracked automatically, so an upstream release that breaks a contract arrives as a red, bisectable pull request.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For my curiosity, which LLM writes like this? Can you tweak it to be more... understandable?


### What we have to support

1. **Sites run versions we do not control, and not the same ones.** A site may run a long-term-support release years after it shipped, or a version that is not the newest. Testing only against the newest daemon answers a question nobody asked.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"versions of backends", I guess.

### What we have to support

1. **Sites run versions we do not control, and not the same ones.** A site may run a long-term-support release years after it shipped, or a version that is not the newest. Testing only against the newest daemon answers a question nobody asked.
2. **Some bugs exist only against a real daemon.** One HTCondor release changed output retrieval for spooled jobs so that collecting the files removed the job from the queue. Nothing about the client changed, and no unit test could have noticed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
2. **Some bugs exist only against a real daemon.** One HTCondor release changed output retrieval for spooled jobs so that collecting the files removed the job from the queue. Nothing about the client changed, and no unit test could have noticed.
2. **Some bugs surfaces only against a real daemon.** As an example from a real use case: an HTCondor release changed output retrieval for spooled jobs so that collecting the files removed the job from the queue. Nothing about the client changed, and no unit test could have noticed.


**Unpinned canary.** A scheduled workflow runs the same matrix against floating tags. It cannot block a merge. It exists to catch upstream changes before they reach the stable tags, and failures should open a deduplicated issue. This lane is planned rather than implemented.

### 7. Backend version support window

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How new versions of backends are discovered?

- an object store beside the grid stacks, so that resource-side staging is exercised for real, in both directions, with the harness never touching the bytes;
- a container variant, asserting that a backend either runs the payload in the requested image or refuses at submit.

## Rationale

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It still makes more sense to me that this and the next section are brought at the beginning.

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.

[Feature]: Adopt the CE-migration ADRs and build the integration-test scaffold (Phase1)

3 participants