docs: ADR for migration - #4
Conversation
21809ca to
f14ca7d
Compare
amaltaro
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
This is just a very initial review. I will review again the updated version.
| 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. | ||
|
|
There was a problem hiding this comment.
You might as well add DIRAC.Resources.Computing ?
| - **One contract, many backends**, with *combinations* (`SSH + Slurm`, `SSH + HTCondor`, | ||
| `Local + HTCondor`) as first-class. |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.** |
There was a problem hiding this comment.
I would reference https://typing.python.org/en/latest/spec/protocol.html#protocols
|
|
||
| ### 2. The contract — capability-segmented Protocols | ||
|
|
||
| The caller-facing contract is a set of small `typing.Protocol`s, and **every operation works on |
There was a problem hiding this comment.
You mean
| 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 |
There was a problem hiding this comment.
"input" and "output" here are only "input sandbox" and "output sandbox". Just make sure there's no way it's interpreted differently
| @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 |
| - **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. |
There was a problem hiding this comment.
| - **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. |
There was a problem hiding this comment.
"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 | ||
|
|
There was a problem hiding this comment.
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). |
There was a problem hiding this comment.
FileRef and OutputMember are not the best names, IMHO. Especially if they could be called simply InputFile and OutputFile.
| class Submission(BaseModel): | ||
| handles: Sequence[JobHandle] # handles[i] belongs to copies[i] | ||
| failures: Mapping[int, str] # copy index -> reason, for copies the backend refused |
There was a problem hiding this comment.
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. |
| cpus: int = 1 | ||
| memory_mb: int | None = None | ||
| wall_time_s: int | None = None | ||
| queue: str | None = None |
| - **`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. |
There was a problem hiding this comment.
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. |
| 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. |
There was a problem hiding this comment.
In general, this phrase is very difficult to understand.
What's a "narrow protocol"?
What's a "task" in this context?
...
No description provided.