Skip to content

feat(transfer): exclude_names + duplicate linking for uploads, plus instrument folder uploader - #581

Draft
Caushi wants to merge 5 commits into
fix/auth-register-uses-loginfrom
feat/upload-exclude-names
Draft

feat(transfer): exclude_names + duplicate linking for uploads, plus instrument folder uploader#581
Caushi wants to merge 5 commits into
fix/auth-register-uses-loginfrom
feat/upload-exclude-names

Conversation

@Caushi

@Caushi Caushi commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Three related upload changes, plus an example that exercises them.

Stacked on #579#573. Review those first; this retargets to main once they land.

1. exclude_names on upload_files / collect_file_infos

Drops files by basename at any depth during the folder expansion, for callers that keep a sentinel file inside the directory they upload (or want to skip .DS_Store and friends).

Filtering has to happen inside collect_file_infos rather than in the caller's path list. compute_file_info only applies base_dir on the directory branch, so pre-filtering into a flat file list silently changes resource names:

whole folder      : ['.marker', 'sub/a.raw', 'top.raw']
children filtered : ['a.raw', 'top.raw']        # sub/ lost, silently

It also breaks folders with same-named files in different subdirectories, which _pair_resources_to_files rejects as duplicate resource names.

A directory whose contents are entirely excluded raises ValueError through the existing empty-directory branch, rather than creating a workunit with no resources.

2. Linking content-duplicates instead of skipping them

Opt-in via UploadFilesParams.link_duplicates / bfabric-cli workunit upload --link-duplicates. Any duplicate verdict naming an existingResourceId has that id passed back as linkFromResourceId; the server creates an AVAILABLE resource pointing at the existing bytes with no transfer. The workunit then holds a resource for every input file rather than silently omitting duplicates.

Linking is deliberately the caller's decision, not the server's: this server reports skip for every duplicate category, so the trigger is "a verdict carrying an existingResourceId", not an explicit link action. Without the flag, behaviour is unchanged.

UploadSummary reports linked (a count, alongside uploaded/skipped/failed) with the detail in links, mirroring uploads / failures. A workunit whose files were all linked now completes instead of tripping the "nothing uploaded" check.

3. Instrument folder uploader (example)

A cron-friendly uploader: each subfolder of a watched directory becomes one workunit, uploaded once its marker file appears. One YAML config per machine, so a fleet shares the script and differs only in config.

Three load-bearing details:

Sidecar state file. The folder → workunit-id memory lives outside the watched tree. Inside the run folder it would be uploaded as a resource — and since its content changes after every upload, its md5 would change too, re-uploading it on every scan and defeating the dedup the design depends on. A config guard rejects a state_dir inside watch_dir (resolved, so symlinks and .. cannot smuggle it back in).

Atomic state write. write_text truncates before writing, so a crash mid-write leaves an empty file — which reads back as "not yet uploaded" and creates a duplicate workunit, exactly the failure the state file exists to prevent. Written temp-then-os.replace.

A new run uploads with force=True. B-Fabric's duplicate check is container-wide, not per-run. An unrelated run that produced byte-identical content (a calibration file, a blank) would otherwise suppress this run's copy, leaving the folder with no workunit of its own — and with no id to remember, stranded on the create path forever. Instrument runs are events, not content. The reuse path keeps dedup, which is what makes repeated scans cheap: force=True on an unchanged folder 409s on the server's per-workunit path uniqueness.

The uploader deliberately does not use linking — for automatic instrument capture, each run wants its own resources.

The marker filename is configurable (marker_name, default .bfabric_upload). Sites whose acquisition software already drops a done-flag can point the uploader at that instead of training operators to create ours — the difference between an operator doing nothing and an operator opening a terminal. Validated at load, since a name containing / would never match the per-folder check and would silently skip every run forever.

--help is operator-facing rather than a dump of the module docstring: it names the marker file, shows the touch command, and points at the secret env var.

Verified end to end

Against a live B-Fabric + tus instance (application 588, container 403).

Nested folder through the uploader:

Scan Result Workunit
initial uploaded 3, skipped 0, failed 0 created, AVAILABLE
unchanged re-scan uploaded 0, skipped 3 reused
new file in a new subdir uploaded 1, skipped 3 reused

Resources kept their relative names (sub/deeper/deep.raw), the marker never appeared as a resource, and exactly one workunit existed per run.

Linking, on a folder mixing duplicates with new content:

uploaded 1, linked 2, skipped 0, failed 0
dup_a.raw  available  ->  p403/w346619/top.raw          (linked, no transfer)
dup_b.raw  available  ->  p403/w346619/sub/nested.raw   (linked, no transfer)
fresh.raw  available  ->  p403/w346637/fresh.raw        (transferred)

Also checked directly against the REST layer: mixed link+plain batches flag linked per file, and a batch containing one bad id registers nothing (all-or-nothing).

Full tests/bfabric suite passes (868); basedpyright clean on bfabric and bfabric_scripts.

Note

Uploading a folder with sub-directories required a server-side fix: /rest/upload/* previously echoed the basename, so nested names failed the duplicate-check and resource-pairing guards, and stored paths were flattened. That is fixed and verified; the guards were right and needed no change. A nested re-upload now reports renamed_duplicate rather than exact_duplicate (name matching misses on a subpath, so detection falls back to MD5) — both carry skip, so branch on action, not category.

Open question for reviewers

The uploader's marker stays inside the run folder — that is where the operator is when the run ends, and a done-flag in a distant directory is one that gets skipped or misspelled. It is excluded via exclude_names, so it is never uploaded. Say the word if you would rather it lived in state_dir too.


The lazy-polars change that was briefly on this branch now lives in #585, since it touches core entity modules unrelated to uploads.

@Caushi Caushi changed the title feat(transfer): add exclude_names to upload_files and collect_file_infos feat(transfer): exclude_names for uploads, plus instrument folder uploader example Aug 10, 2026
Claudio Cannizzaro added 2 commits August 10, 2026 14:02
Drop files by basename at any depth during the folder expansion, for callers
that keep a sentinel file inside the directory they upload (or want to skip
.DS_Store and friends).

Filtering has to happen inside collect_file_infos rather than in the caller's
path list: base_dir is only applied on the directory branch, so pre-filtering
into a flat file list silently renames "sub/file.raw" to "file.raw" and makes
two same-named files in different subdirectories collide in
_pair_resources_to_files.

A directory whose contents are entirely excluded raises ValueError via the
existing empty-directory branch, rather than creating a workunit with no
resources.
A cron-friendly uploader for instrument output: each finished run folder
becomes one workunit, uploaded once its marker file appears.

Three details are load-bearing:

- The folder -> workunit-id memory is a sidecar state file outside the watched
  tree. Keeping it inside the run folder would upload it as a resource, and
  since its content changes after every upload its md5 would change too,
  re-uploading it on every scan. A config guard rejects a state_dir inside
  watch_dir.

- The state file is written atomically (temp + os.replace). A bare write_text
  truncates first, so a crash mid-write would leave an empty file, which reads
  as "not yet uploaded" and creates a duplicate workunit for the run.

- A new run uploads with force=True. B-Fabric's duplicate check is
  container-wide, so an unrelated run that produced byte-identical content
  would otherwise suppress this run's copy and leave it with no workunit at
  all -- and with no id to remember, stranded on the create path forever.
  Instrument runs are events, not content. The reuse path keeps dedup, which
  is what makes repeated scans cheap.

The operator's marker stays in the run folder (that is where the operator is
when the run ends) and is excluded via exclude_names.
@Caushi
Caushi force-pushed the feat/upload-exclude-names branch from 1eba9fc to 4ef5e88 Compare August 10, 2026 12:02
@Caushi
Caushi changed the base branch from main to fix/auth-register-uses-login August 10, 2026 12:02
Claudio Cannizzaro added 2 commits August 10, 2026 14:50
The upload REST API gained a `linked` field on create-resources responses and
accepts `linkFromResourceId` on requests, letting a resource point at bytes the
instance already stores instead of transferring them again.

Handle both directions:

- `CreatedResource.linked` marks a resource created AVAILABLE with no bytes to
  send. Such resources are excluded from the ids passed to /upload/initiate and
  never handed to the mover; a workunit whose files were all linked completes
  instead of tripping the "nothing uploaded" failure branch.
- `FileInfo.link_from_resource_id` drives the request side, emitted only for
  create-resources (never on check-duplicates, and omitted rather than sent as
  null for an ordinary upload).
- New opt-in `UploadFilesParams.link_duplicates` / `--link-duplicates` links a
  duplicate rather than skipping it, so the workunit holds a resource for every
  input file. The server reports a content-duplicate as action "skip" with an
  existingResourceId (category exact_duplicate/renamed_duplicate), so linking
  keys off that, not off action "link"; a duplicate with no existingResourceId
  stays a plain skip. Off by default, since linking registers bytes this caller
  never uploaded.
- `UploadSummary.linked` / `linked_count` report linked files separately: a
  linked file has a resource but transferred nothing, whereas a skipped one has
  no resource at all.

Nested resource names now round-trip verbatim, so the duplicate-check and
resource-pairing guards no longer fire on subdirectory uploads and force=True is
no longer needed to work around them. Tests cover that, since the fix is
server-side and nothing here changed.
linked was a list[FileUpload] while uploaded/skipped/failed were ints, so
f"linked {summary.linked}" dumped a list of objects instead of a number, and
callers needed the separate linked_count property to get the count.

Every counter is now an int with a matching plural detail list:

    uploaded -> uploads
    failed   -> failures
    linked   -> links
    skipped

linked_count is dropped, since linked is now the count it provided.
@Caushi Caushi changed the title feat(transfer): exclude_names for uploads, plus instrument folder uploader example feat(transfer): exclude_names + duplicate linking for uploads, plus instrument folder uploader Aug 10, 2026
@Caushi Caushi changed the title feat(transfer): exclude_names + duplicate linking for uploads, plus instrument folder uploader feat(transfer): exclude_names + duplicate linking, lazy polars, instrument folder uploader Aug 10, 2026
The marker filename was hardcoded to .bfabric_upload, and --help printed the
whole module docstring -- pages of design rationale that never once named the
file an operator has to create.

- New optional `marker_name` config key (default unchanged). Sites whose
  acquisition software already drops a done-flag can point the uploader at it
  instead of training operators to create ours, which is the difference
  between an operator doing nothing and an operator using a terminal.
  Validated at load: a name containing "/" would never match the per-folder
  existence check, so every run would be silently skipped forever.
- --help is now operator-facing: it names the marker file, shows the touch
  command, and points at the secret env var. The design rationale stays in
  the module docstring, for whoever reads the source.
@Caushi
Caushi force-pushed the feat/upload-exclude-names branch from e1eb7ab to d0efd47 Compare August 10, 2026 14:22
@Caushi Caushi changed the title feat(transfer): exclude_names + duplicate linking, lazy polars, instrument folder uploader feat(transfer): exclude_names + duplicate linking for uploads, plus instrument folder uploader Aug 10, 2026
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.

1 participant