docs: ADR for migration - #4
Conversation
21809ca to
f14ca7d
Compare
amaltaro
left a comment
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
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
| backend to stub a capability it lacks*. So `OutputRetriever` sits with the optional capabilities, | ||
| and the output task narrows to it structurally. | ||
|
|
||
| ```python |
There was a problem hiding this comment.
I would add also, for clarity:
from typing import runtime_checkable, Protocol
from collections.abc import Mapping
| @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 |
| async def fetch_output(self, ids: Sequence[JobID], dest: Path) -> Mapping[JobID, JobOutput]: ... | ||
|
|
||
| @runtime_checkable | ||
| class Cancellable(Protocol): |
There was a problem hiding this comment.
| class Cancellable(Protocol): | |
| class Killable(Protocol): |
?
3 participants
No description provided.