feat(transfer): exclude_names + duplicate linking for uploads, plus instrument folder uploader - #581
Draft
Caushi wants to merge 5 commits into
Draft
feat(transfer): exclude_names + duplicate linking for uploads, plus instrument folder uploader#581Caushi wants to merge 5 commits into
Caushi wants to merge 5 commits into
Conversation
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
force-pushed
the
feat/upload-exclude-names
branch
from
August 10, 2026 12:02
1eba9fc to
4ef5e88
Compare
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.
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
force-pushed
the
feat/upload-exclude-names
branch
from
August 10, 2026 14:22
e1eb7ab to
d0efd47
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three related upload changes, plus an example that exercises them.
1.
exclude_namesonupload_files/collect_file_infosDrops 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_Storeand friends).Filtering has to happen inside
collect_file_infosrather than in the caller's path list.compute_file_infoonly appliesbase_diron the directory branch, so pre-filtering into a flat file list silently changes resource names:It also breaks folders with same-named files in different subdirectories, which
_pair_resources_to_filesrejects as duplicate resource names.A directory whose contents are entirely excluded raises
ValueErrorthrough 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 anexistingResourceIdhas that id passed back aslinkFromResourceId; the server creates anAVAILABLEresource 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
skipfor every duplicate category, so the trigger is "a verdict carrying anexistingResourceId", not an explicitlinkaction. Without the flag, behaviour is unchanged.UploadSummaryreportslinked(a count, alongsideuploaded/skipped/failed) with the detail inlinks, mirroringuploads/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_dirinsidewatch_dir(resolved, so symlinks and..cannot smuggle it back in).Atomic state write.
write_texttruncates 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=Trueon 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.--helpis operator-facing rather than a dump of the module docstring: it names the marker file, shows thetouchcommand, 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:
AVAILABLEResources 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:
Also checked directly against the REST layer: mixed link+plain batches flag
linkedper file, and a batch containing one bad id registers nothing (all-or-nothing).Full
tests/bfabricsuite passes (868); basedpyright clean onbfabricandbfabric_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 reportsrenamed_duplicaterather thanexact_duplicate(name matching misses on a subpath, so detection falls back to MD5) — both carryskip, so branch onaction, notcategory.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 instate_dirtoo.The lazy-
polarschange that was briefly on this branch now lives in #585, since it touches core entity modules unrelated to uploads.