Repository navigation
Test catalog rebuild survives a missing tape (roadmap #11) - #35
Merged
Merged
Conversation
The review asked for CI proving a blank DB rebuilds from tape "including partially corrupt and missing tapes". The blank-DB rebuild (test_lost_db_recovery_full_scenario) and corrupt/incomplete-metadata cases (missing/invalid manifest, missing shard) were already covered and CI-run; the one uncovered clause was a physically MISSING tape. Add test_rebuild_with_a_missing_tape_recovers_available_subset: seed two tapes, wipe the DB, then rebuild from only ONE — asserts the available tape's files are recovered, the missing tape's are gracefully left unrecovered (no fabrication, no crash), and the run still completes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012K5Zjc2bggcyJtFbz2D4Pw
There was a problem hiding this comment.
Pull request overview
Adds a unit-level regression test to ensure the catalog rebuild worker can successfully rebuild the catalog from only the tapes that are actually available, while leaving data from an absent tape unrecovered (no fabrication, no crash). This supports OpenBlade’s simulator-first “lost DB recovery” workflow coverage under roadmap item #11.
Changes:
- Add
test_rebuild_with_a_missing_tape_recovers_available_subsetcovering “rebuild from subset of tapes” behavior after a DB wipe. - Assert the rebuild completes successfully and only restores records for the provided/available barcode.
Comment on lines
+465
to
+484
| barcodes = rebuild_env["context"].library.get_all_barcodes()[:2] | ||
| seeded_one = _seed_tape(rebuild_env, barcodes[0]) | ||
| seeded_two = _seed_tape(rebuild_env, barcodes[1]) | ||
| _populate_catalog_from_seed(rebuild_env, seeded_one) | ||
| _populate_catalog_from_seed(rebuild_env, seeded_two) | ||
|
|
||
| session = rebuild_env["context"].catalog.session | ||
| session.query(PathMappingRow).delete() | ||
| session.query(NasFileRecordRow).delete() | ||
| session.query(NasDatasetRow).delete() | ||
| session.commit() | ||
|
|
||
| # Only tape 0 is available for the rebuild; tape 1 is "missing". | ||
| result = rebuild_env["worker"].auto_plan_and_execute([barcodes[0]], triggered_by="operator") | ||
|
|
||
| assert result.status is RebuildRunStatus.COMPLETED | ||
| assert result.files_recovered == 2 # tape 0's files only | ||
| assert rebuild_env["context"].catalog.get_nas_file_record(f"{barcodes[0]}-file-0") is not None | ||
| # The missing tape's records remain unrecovered — no silent fabrication, no crash. | ||
| assert rebuild_env["context"].catalog.get_nas_file_record(f"{barcodes[1]}-file-1") is None |
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.
Completes the review's #11 ("blank DB rebuilds from tape, including corrupt and missing tapes"). Investigating showed #11 was already substantially built and CI-tested:
test_lost_db_recovery_full_scenariodoes the full wipe→rebuild→assert cycle, and missing/invalid-manifest + missing-shard cover the corrupt-metadata cases. The one uncovered clause was a physically missing tape.Adds
test_rebuild_with_a_missing_tape_recovers_available_subset: seed two tapes, wipe the DB, rebuild from only one — asserts the available tape's files recover, the missing tape's are gracefully left unrecovered (no fabrication, no crash), run completes.(Meta-note: several review gaps turned out more complete than claimed — #11 rebuild, cookie-session support, full coordinate dicts. The genuinely-unbuilt items are Phase 2 persistent leases and Phase 6 real SMB/NFS.)
https://claude.ai/code/session_012K5Zjc2bggcyJtFbz2D4Pw