Skip to content

Add tus upload backend via bfabricpy - #25

Draft
Caushi wants to merge 4 commits into
6-project-tomlfrom
feat-tus-upload
Draft

Add tus upload backend via bfabricpy#25
Caushi wants to merge 4 commits into
6-project-tomlfrom
feat-tus-upload

Conversation

@Caushi

@Caushi Caushi commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Draft until a bfabricpy release containing the linkable handling is out. This branch needs
bfabric[transfer] from fgcz/bfabricPy#615; the pin below must be raised to that version before merge.

What this adds

tool="tus" as a fourth transfer backend alongside robocopy, scp and sftp. It uploads over
HTTP(S) to a tus endpoint via bfabricpy, which means nothing has to be mounted (no UNC mapping,
no SSH keys) and the workunit is created at upload time, so data lands in B-Fabric registered
rather than needing a separate import.

Additive and opt-in per host: existing robocopy/scp/sftp hosts are untouched.

Design decisions

  • One workunit per acquisition folder. upload_files is batch (one call = one workunit), so tus
    branches above the per-file copy loop in copy_files_with_tool; routing it through would have
    created one workunit per file. Bruker .d / Waters .PRO bundles are uploaded as a single
    directory entry so nested structure survives as resource names (sub/deep.bin).
  • application_id comes from <b-fabric><applicationID>, which has been in the XSD since the
    beginning and was never read by any code — the parser only walked i.attrib, never children.
  • container_id is parsed from the file path, with no fallback. One instrument writes data for
    many projects, so the container cannot be a per-host constant. Filing data under the wrong project
    is worse than a failed run, so an unrecognisable path is a hard error naming both the path and the
    accepted formats; nothing is uploaded. The legal formats are an operational contract, documented in
    the README in wording identical to the runtime error.
  • Credentials never touch argv. The client secret is read from BFABRIC_CLIENT_SECRET only, so it
    cannot leak into logs or /proc. This also fixes a pre-existing leak where --password was written
    verbatim into the launcher's INFO log.

Verified end to end

Against a live local stack (tusd + bfabric-tus-storage-service + B-Fabric):

  • two acquisition folders → two workunits, 5 resources, nested sub/deep.bin prefix intact
  • re-run uploads nothing (local ledger + server-side MD5 dedup)
  • unparseable container → exit 2, correct message, empty ledger, nothing uploaded
  • a deliberately failed transfer leaves pending resources; the retry uploads them rather than
    failing to link (needs the bfabricpy change above)

Unit tests: 29 new in tests/test_tusupload.py, 71 passing overall (42 before this branch).

Storage is verified before a source file is deleted

A completed tus transfer is not confirmed storage. The storage service runs its virus scan,
checksum verification and disk checks in a post-finish hook — after the transfer is already
complete — and that hook reports to B-Fabric rather than to BioBeamer, so it cannot fail the
transfer that produced it. A file bfabricpy reports in summary.uploads can therefore end up with
its resource marked failed, holding no usable bytes.

That matters here because max_time_delete deletes source files once they are old enough and
recorded as copied. Trusting the ledger alone would eventually delete the only copy of data
B-Fabric rejected. So for tool="tus":

  • the resource id of every uploaded file is recorded in tus_resources.json beside the ledger
  • deletion re-reads those statuses and deletes only what B-Fabric reports available
  • the next run drops rejected files from the ledger so they are uploaded again rather than skipped
    for ever

Only available authorises a delete. Everything else is withheld — pending (the server has not
ruled yet), a status that could not be read, and any file not accounted for. A failure to check
is never mistaken for confirmation, and equally never triggers a mass re-upload.

The check is at deletion time rather than just after upload because verification runs on the
server's schedule: measured against the live stack, resources read pending ~1s after upload and
available ~25s later. Checking immediately would tell us nothing, and the state that matters is
the one when the delete decision is made.

This is tus-specific on purpose. For robocopy/scp/sftp the target is a staging area and a separate
downstream process moves and registers the files, so a completed transfer is the whole job and
the existing destination comparison already covers it. tus collapses transfer and registration into
one step, which is what creates the unobservable window.

Verified against the live stack

tests/integration/tus_verification_walkthrough.sh walks it through and prints B-Fabric's verdict
per file at each step. Using an EICAR test file — which transfers normally and is then rejected by
ClamAV in post-finish — the run shows the upload reporting success, the resource going failed, the
local copy correctly kept, and the ledger repaired on the following run.

Known gap, for discussion: #26. A file that will never pass verification currently retries
indefinitely, creating a new workunit and resource each run. No data is at risk (the local copy is
always kept) but nothing escalates it to a human, and a transient rejection is indistinguishable
from a deterministic one. Options are laid out in that issue.

Files

File Change
src/biobeamer/tusupload.py new — container resolution, folder grouping, upload, comparison
src/biobeamer/cli.py batch tus branch; tus in the destination-comparison dispatch; non-secret bfabric flags; deletion gated on verified storage; ledger repair
src/biobeamer/tusregistry.py new — records which resource each file became, and classifies stored / rejected / unknown
tests/test_tusregistry.py new — 22 tests covering the registry and both gates
tests/integration/tus_verification_walkthrough.sh new — live walkthrough incl. the EICAR rejection case
src/biobeamer/parser.py read <b-fabric><applicationID>; per-instance parameters copy
src/biobeamer/configs/BioBeamer2.xsd tus_* attributes, xs:anyAttribute, applicationIDxs:int
pyproject.toml optional [tus] extra; requires-python raised to a truthful 3.9
README.md tool="tus", the tus_* attributes, the legal source path formats, and the storage-verification contract

Rollout note

xs:anyAttribute is the load-bearing part of the XSD change. The config XML is shared by the whole
fleet while the XSD ships per BioBeamer version, so without it any new attribute breaks every host
still pinned to an older version — including robocopy hosts unrelated to tus. Verified empirically:
a tus_endpoint attribute fails validation against the old XSD and passes with the one-line
anyAttribute addition. Back-port that line and roll it out before the config change lands.

The [tus] extra carries a python_version >= '3.11' marker because bfabric requires 3.11+ while
BioBeamer must stay installable on the 3.8/3.9 instrument PCs. On those hosts the extra resolves to
nothing and tool="tus" fails at startup with a message saying so.

@wolski

wolski commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Thanks for putting this together. I agree with the central separation: completing a tus transfer is not sufficient to delete the instrument copy, and checking the individual B-Fabric resource status at deletion time is the right foundation. FCC processing does not need an acquisition/workunit-level “done” state; the relevant invariant here is strictly per source file/resource because BioBeamer owns source cleanup.

I do not think the current implementation is safe to merge for production deletion yet:

  1. The deletion receipt is bound only to a pathname, not to the uploaded bytes. record_uploads() stores source_path -> resource_id, and classify() authorizes that pathname when the resource is available. If the instrument rewrites or recreates the pathname, the old resource can therefore authorize deletion of the new bytes. I reproduced this locally: after replacing a registered file, cleanup deleted the replacement when the old resource returned available. The registry needs to store a file identity (ideally MD5; at minimum size + mtime_ns) and cleanup must revalidate it immediately before deletion. This is related to Discuss: two ledger/retry gaps in the copy pipeline #26, but it directly limits the deletion guarantee made by this PR.

  2. on_duplicate="skip" produces a permanent copied-but-undeletable state. Skips are added to done_names/the copied ledger, but resource_by_name contains only uploads and links. The registry therefore has no entry, classify() returns unknown, and the source is neither retried nor ever deleted. bfabricpy 1.22 exposes FileSkip.existing_resource_id; record that ID when present. If it is absent, the skip cannot satisfy BioBeamer's removal contract.

  3. bfabricpy 1.22 successful results are lost on WorkunitCompletionError. That exception means the transfers landed but the final workunit status flip failed, and it carries the successful UploadSummary. The generic exception handler currently discards the summary and reports no copied files, causing incorrect accounting and potentially another upload/workunit next run. This is not about FCC caring whether the workunit is complete; BioBeamer must preserve the per-file resource IDs from error.summary. The dependency should also be raised from bfabric[transfer]>=1.21.0 to >=1.22.0, as anticipated in the PR description.

  4. This is a replacement backend, not a parallel Data2San+tus migration path. One tool is selected and, for tus, destination mapping/Data2San copying is bypassed. If the rollout requirement is to keep Data2San (and its normal pfiles.txt/FCC behavior) operational alongside tus, two independent BioBeamer runs are unsafe: a shared ledger makes one path suppress the other, while separate ledgers allow either path to delete before the other succeeds. Cleanup needs to require receipts from every destination enabled for that migration phase.

The repeated if tool == "tus" branches in upload dispatch, destination comparison, mapping, ledger repair, and cleanup are consequently a design smell. Tus is no longer merely another copy command: it has batch grouping, a remote receipt, asynchronous verification, and different deletion rules. I would introduce a small backend boundary that returns per-file transfer receipts and answers whether the exact local file is safe to remove. The orchestrator can then run one or several configured backends and delete only when every required receipt is satisfied.

Suggested state shape:

source: path + size + mtime_ns/checksum
destinations:
  data2san: copied/verified + destination path
  tus: resource_id + storage status

Deletion would require: unchanged local identity, every required destination complete, tus resource available, and max_time_delete elapsed.

Verification on this branch with bfabricpy 1.22.0: all 58 tus-specific tests pass. The full suite produced 99 passed, 1 skipped, and one environmental SFTP integration failure because no SSH server was running on localhost. There are currently no GitHub checks/workflows on the PR, so I would also add CI before enabling the backend on instrument hosts.

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.

2 participants