Add a check mode and a relocate command for legacy data trees - #36
Merged
Merged
Conversation
fwl-io could serve what is present and refuse when it is not, or download what is missing. Neither answers the question a diagnostic asks: is this tree complete and intact? `proteus doctor` wants the whole picture in one pass, and it must not repair the thing it is inspecting. `check_for(model)` returns a report of every dataset the model requires, one entry per file, in one of four states. A file is ok when it is present and matches the registry, missing when it is not there, and mismatch when it is there and its contents differ. Those last two are kept apart because the remedies differ: an absent file may never have been fetched, while a corrupt one was fetched and then damaged. The fourth state is present, meaning the file is on disk and nothing was available to check it against. An archive dataset's registry pins the checksum of the archive, not of what came out of it, so once the archive is dropped there is nothing to hash its members against. A report that called those verified would claim more than it checked, so it says presence only and `hashed` is false for that dataset. A manifest that fails to load is carried in the report rather than dropped, and it alone is enough to make the report not ok. Its datasets were never inspected, so treating it as harmless would let a tree with an unreadable provider read exactly like a healthy one. `fwl-io check <model>` prints the report and exits 1 on any fault. Nothing is downloaded and no dataset directory or file is written; resolving the data root creates that root when absent, as it does for every entry point, and that is the only mark a check leaves. Closes #20.
Adds the subcommand to the CLI reference with its four file states and what each one means, and says on the design page why a read-only mode exists beside the offline and online ones: neither of those can report the state of a whole tree, because one stops at the first fault and the other repairs it.
A diagnostic that overstates its own coverage is worse than none, and the first cut of the check mode did so in several places. Two kinds of failure were filed together and printed under one label, so a dataset whose registry has simply never been generated was reported as an unreadable manifest, telling the user to fix the wrong thing. They are now separate maps with separate labels, and the message for an unresolvable dataset names `fwl-io sync` as the remedy. An empty report claimed to be ok. Asking about a model nothing declares, and being told nothing is wrong, is indistinguishable from a clean tree to anyone reading a boolean. A report with no datasets in it is now never ok, which is what the command already did and the library did not. A file that could not be read raised out of the whole report, so one permission problem hid every other dataset. Unreadable is now a state of its own and counts as a fault: its contents are unknown, which is not the same as correct. The stamp reading that decides which members an extracted tree should have existed twice, in the fetcher and again in the checker, and the two had already drifted apart: the checker accepted a stamp left by a fetch of a different record, and joined member names onto the tree without checking they stayed inside it. Both now go through one method on the fetcher, which requires the stamp to describe this record and drops any name resolving outside the dataset directory. A stamp is an ordinary file and can be edited, so a name in it deserves the same suspicion as a name inside an archive. Also corrects the design page, which said a presence-only dataset counts against the verdict. It does not and should not: presence is all the archive-only checksum policy makes checkable, so the report says so plainly rather than failing every archive dataset forever.
A sound tree closed with "all data present and verified" even when a dataset line directly above it read "presence only". Nothing about those contents had been established, so the verdict now names them: "all data present, N dataset(s) by presence only". `CheckReport.verified` asks the same question programmatically. It is stricter than `ok`, which a presence-only dataset still satisfies, because presence is all the archive-only checksum policy makes checkable and failing every archive dataset forever is not the answer. Whether a dataset can be verified is now a property of the dataset rather than of what happened to survive on disk. Read off the file states, an archive whose members had all gone missing reported itself verifiable, since it had no present files left to say otherwise. An archive member the checker cannot reach is reported unreadable rather than aborting the dataset. The plain-file path already worked that way; the archive path let the error out, so one directory denying traversal turned every other member of that dataset into no information at all. The shared-cache stamp is read by the same method as the dataset's own, so both are held to one standard. The cache reader had grown its own qualifying rules and no containment check, which let an edited cache stamp name members outside the cached tree and still be trusted. Reading both through one method also means a stamp has to name this deposit, not only a record id parsed out of it. Alongside: the check reads and hashes every file the manifest declares, which the command's help text and the reference now say plainly, since a full pass over a multi-gigabyte tree is not what "without downloading" suggests; `fwl_io.check` gets a reference page like every other public module; and `file_matches` documents the KeyError it raises.
A stamp holding valid JSON that is not an object aborted an archive fetch with an AttributeError from deep inside the fetcher: the reader parsed the file and then asked a list for a key. Truncating or hand-editing `.fwl-io.json` is how that happens, and the file being editable is the whole reason the member names in it are treated with suspicion, so the reader itself has to survive one. The two readers now share `_read_stamp`, which decides absent, unreadable, not JSON and not an object all in one place; a stamp like that is now rewritten on the next fetch and reported as no tree by a check, which is what both already claimed to do. The check path was already safe, so this only ever bit the fetch path. `DatasetCheck.verifiable` loses its default. Its wrong value is the one that lets a presence-only dataset read as verified, and a default that is right for one of the two dataset kinds is a quiet way to reach that, so every construction site states it. Scoping two claims that were wider than the truth: the cost note now says a full read pass is what plain datasets cost, since an archive dataset is only tested for presence and reads nothing; and `file_matches` raises OSError for an absent file as much as an unreadable one, which its Raises clause now says. `CheckReport.faults` says in its docstring that it covers datasets and nothing else, so an empty tuple is not an answer to whether anything is wrong. Four gaps in what the tests pin: a stamp that parses to a list, null, a number or a string; a member entry that is not a name at all; the soundness half of `verified`, which a hashed tree with a missing file has to fail; and the command exiting 0, which nothing covered in either the verified or the presence-only wording.
`_STAMP_SCHEMA` went into every stamp and nothing ever read it back, which leaves the guarded parser guarding the wrong thing. A stamp written by a future version is well-formed JSON in an object, so it passes every check there was, while its fields need not mean what they mean here; the reader would then trust them precisely when it should not. An unrecognised schema now reads as no stamp, so the tree is refetched and restamped rather than misread. That is also what makes the shape checks worth having: they cover a file that was damaged, and this covers one that was written on purpose to different rules. The states that fail a dataset and the words the report prints for them are now one mapping rather than two lists. Adding a state to the first would have failed a dataset while the line a person reads never said why. Three corrections to the tests. The offline half of the malformed-stamp case pointed at an empty data root, so it raised for want of any data at all and would have passed whatever the stamp reader did; it now puts the bad stamp back over the tree it has just populated, where the stamp is the only thing left that can decide the dataset is unservable. The command-line fixture wrote a stamp without the schema field that every real one carries. And the shared cache had no malformed-stamp case, although a group-writable cache on a cluster is where a half-written one is most likely to turn up.
A tree fetched before the current layout existed still holds directories like `stellar_evolution_tracks/Baraffe`. Unmigrated code reads them, so they are left alone and age out as their consumers migrate, which is right for the ecosystem and no help at all to someone who wants their own tree tidy today. `fwl-io relocate` is for them: it finds the datasets the installed manifests declare, works out where each one used to live, and moves the files across. Nothing moves on trust. Every file is hashed against the registry before anything is touched, and a dataset with a file missing or a file whose contents differ is reported and left exactly where it is. Moving first and discovering afterwards would turn a stale copy into a stale copy at the location the fetcher then believes, which is worse than leaving it somewhere a reader can still tell it is old. A move that fails part way puts back what it moved, since a dataset split across two layouts is the one state neither the reader nor the fetcher can interpret. Where each dataset used to live is the one thing a manifest cannot say, so it ships as a table in the package. The manifest supplies the record id and the checksums; the table supplies the old path. Keying it by dataset key means a directory that held several datasets comes apart correctly, since each moves only the files its own registry names, and a dataset created after the migration needs no entry at all. The only directories it removes are ones it has just emptied itself, and the walk upward stops at the data root. A copy still sitting at the old location beside a current one is named rather than deleted, and counted in the closing line: on a tree where everything has already been refetched that count is the only thing the run has to say, and it is the disk the user can reclaim. An unread manifest keeps the report from claiming to be complete. It may be the one declaring the dataset whose tree is still sitting there, and a run that looked at nothing otherwise reads exactly like a tree with nothing left to move.
Fetching one file takes a per-target lock so a burst of processes that all miss it does not hit the mirrors at once. Rebuilding an archive dataset took no lock at all, although it is the more dangerous of the two: it replaces the whole version directory, so two processes doing it together move a tree out from under each other while a third is reading it. These are the shared filesystems this package is built for, and the concurrency note in the module header only ever described the per-file path. The lock is keyed on the archive, so unrelated datasets still fetch in parallel, and an intact tree is still served on the fast path without waiting for anything: the common case pays nothing. A process that did wait re-checks the tree before rebuilding, so it serves what the winner built instead of doing the same work again, which is the difference between suppressing a herd and staggering one. Where the lock cannot be taken at all, an archive fetch degrades to an unguarded one exactly as a file fetch does. It never carried correctness, only politeness towards the mirrors, and a mount with no lock manager has to keep working.
Four faults in a command whose whole job is moving files, so each one mattered more than its size suggests. A legacy location naming an absolute path, or climbing out with `..`, was joined onto the root and followed, so files were moved into the tree from outside it. The shipped table was trusted further than a provenance stamp is, which is backwards: the table is a file too, and it becomes the path files are moved out of. Entries that are not inside the root are now dropped when the table is read, the planner refuses a directory that resolves outside, and the move refuses again on its own account, so that guarantee belongs to the code doing the moving rather than to whoever called it. A symlinked legacy directory escapes the same way and only shows it when the path is resolved, which is why the check is on the resolved path. A registry name may nest. Moving `sub/nested.dat` emptied `sub/` and left it standing, which kept the whole legacy directory alive, and a later run then reported that husk as an old copy still holding data when it held nothing. Emptied subdirectories now go before the walk upward. A rollback that could not put a file back logged and carried on, leaving the dataset in both places at once: the exact state the rollback exists to prevent, reported as an ordinary failure. That is now a state of its own, because a rerun fixes a failure and does not fix this, and the run stops there instead of moving more data past a tree somebody has to look at. The command exited 0 when a manifest could not be read, although the summary on the same run said the pass may be partial and `ok` was already false. Anything reading the exit code was told a run that looked at nothing was clean. Also renames the entry point to `relocate_all`, since a module and a function of the same name shadow each other on the package, and every other module here already avoids that: `fetch_for`, `sync_manifest`, `mirror_to_dataverse`.
The claim that the walk upward stops at the data root was resting on a test that could not reach it: after a move the root holds the newly placed tree, so it is never empty, never a candidate for removal, and the guard never runs. The assertion that the root survived passed because the root had contents, not because anything refused to remove it. Pruning an emptied chain into an otherwise empty root reaches the guard, and a sibling dataset under a shared parent pins the other half. A directory that held two datasets now has a test as well. Each moves only the files its own registry names and the shared parent goes once both are done, which the layout table has been claiming since it was written. The design page said the package carries the mapping in `legacy_layout.toml` and the command reads it, which reads as though every row is live. Three of them are. The rest are the historical mapping and their trees are left alone until a model declares the dataset, so the page now says which is which rather than implying a tree will move when it will not.
The containment added last round guarded the two directories and nothing else, which left the case it was written for wide open. A registry name may nest, and `rename` follows a symlink in the middle of a path, so a member named `sub/nested.dat` whose `sub` points somewhere else resolves outside the tree: the directory holding it passes every check, the file that moves in comes from outside, and the original is gone. Every source and destination path is now checked, in the planner and again in the move. Pruning stopped at the first entry it could not remove and reported the move a success, so one stray symlink in an old tree left every emptied directory standing while the command said it had tidied up. A symlink answers `is_dir` for whatever it points at and `rmdir` refuses it, which is the combination that triggered it. Those are skipped now, a refusal moves to the next entry instead of ending the walk, and the walk order is fixed by name as well as depth so it does not vary by filesystem. The check meant to reject a table value that is not a path ran after the value had already been turned into one, so a number or a list raised `TypeError` out of the command rather than being dropped. It runs first now. Also corrects a test that could not fail: it pointed its symlink at a directory with a file in it, so the emptiness test skipped the symlink before the removal that was the whole point was ever attempted.
Three inputs reached the tree comparison and were answered from a check that could not mean what it said. A dataset with an empty registry passed every check vacuously, since each one asks whether the tree holds what the registry lists. An untouched legacy directory was reported as already moved, or moved with zero files, and the run exited 0 with the data still sitting where it was. An archive dataset was compared against a registry that pins the packed archive rather than the files a legacy tree actually holds, so an intact tree was reported incomplete for a file it never had. Both are now refused by name, with the reason, and nothing is touched. A layout table that will not parse raised out of the command instead of being reported. Every other unreadable input here is carried in the report, so this one is too.
Two test files carry a 30 s timeout marker, but the package was never a dependency, so the marker did nothing in a fresh developer or CI environment and a hang would have run unbounded.
…exist Two faults in how the previous commit refused what it cannot verify. An unreadable layout table produced no locations and an empty report, which reads exactly like a tidy tree with nothing left to move: ok was true and the command exited 0. The reason is now carried back with the locations and named in the report, the same way an unreadable manifest already is. A broken install raises ImportError from the resource lookup, which was not among the exceptions being caught, so that is handled too. The refusals also fired before anything asked whether a legacy tree exists, so an archive dataset on a machine that never had the old layout was a permanent fault rather than simply absent. They now apply only when there is a directory to refuse.
The relocate section described neither of the two datasets it will not move, and its exit-code sentence left out the case where the table of old locations cannot be read. The archive refusal also now says what to do about it, since there is nothing the command itself can do for such a tree.
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
1 participant
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.
What this does
Adds two commands to fwl-io.
fwl-io checkvalidates that the data a model needs is present and matches the checksums its manifest ships, without downloading anything and without writing to the tree, so it is safe to run against a tree another process is reading.fwl-io relocatemoves a dataset left by the previous FWL_DATA layout into the place the current layout puts it, after hashing every file against the registry, and reports rather than moves anything it cannot verify.Closes #20. Closes #13.
Why
proteus doctorneeds a way to answer "is the data there and is it intact" that is neither the offline path (which only asks whether files exist) nor the online path (which downloads what is missing). That is the check mode.Relocate is for the other half. Migration to the current layout is per-model, so legacy directories such as
stellar_evolution_tracks/Baraffelinger until their last consumer moves. Leaving them alone is the right default, and it is the wrong one for anyone who wants a tidy tree today.Changes
fwl_io.check: a validate-only report over every dataset a model requires. Nothing raises for the state of the data; a manifest that will not load, a registry that is missing and a file that cannot be read are all carried in the report, so a caller gets the whole picture including the parts that could not be established. An archive dataset is reported from the members recorded in its provenance stamp and marked as not verifiable, since its registry pins the packed archive rather than what came out of it.fwl_io.relocate: plan and execute the move, with a dry-run mode. A dataset is moved only when every file the registry names is present in the legacy directory and hashes correctly. Anything else is reported and left exactly as it is. A move that fails part way rolls back, and a rollback that cannot restore is reported as its own state rather than as a plain failure, because the remedy is a person looking at two directories.src/fwl_io/data/legacy_layout.toml: where each dataset used to live, keyed by manifest key. The current location is not repeated, since it is derived from the key and the Zenodo pin.fwl_io.fetch: an archive rebuild now takes the same per-target lock a file download already took, so concurrent processes on a shared tree serialise instead of racing. Provenance stamps are read through one guarded parser, so a stamp that parses to something other than a mapping is treated as unusable rather than raising from wherever it happened to be read.Testing
345 tests pass locally, up from 289 on main. No test touches the network: download paths run against a threaded
http.serverbound to the loopback, and the relocation path never downloads at all.The path-containment and rollback behaviour is tested against the cases that motivated it: a registry name that nests, a symlinked component that resolves outside the data root on either the legacy or the target side, a rollback that cannot restore, two datasets sharing one legacy directory, and a prune that must stop at a parent still holding something.
Every guard added here was confirmed by planting the defect back and checking the test fails, rather than by the test passing against the fixed code.
zensical build --cleanreports no issues.