fix(table): include orphan file sizes in cleanup results#1412
fix(table): include orphan file sizes in cleanup results#1412fallintoplace wants to merge 1 commit into
Conversation
tanmayrauth
left a comment
tanmayrauth
left a comment
There was a problem hiding this comment.
LGTM, clean change and the table-level test covers the real path. Just a couple of small things around the CLI size lookup and the retained OrphanFileLocations field. Details inline.
|
|
||
| type OrphanCleanupResult struct { | ||
| OrphanFileLocations []string | ||
| OrphanFiles []OrphanFile |
There was a problem hiding this comment.
OrphanFiles []OrphanFile now carries the path and size for every orphan, while OrphanFileLocations []string above it carries just the paths — so the paths are duplicated across the two fields. Keeping OrphanFileLocations for backward compat is the right call, but could you add a one-line doc comment noting it's retained for compatibility and that OrphanFiles is the richer form? Otherwise a future reader has to diff the two to figure out why both
exist.
| entries = append(entries, OrphanFileEntry{Path: f}) | ||
| entries = append(entries, OrphanFileEntry{ | ||
| Path: f, | ||
| SizeBytes: orphanSizes[f], |
There was a problem hiding this comment.
SizeBytes: orphanSizes[f] — in the non-dry-run branch files comes from result.DeletedFiles, but this size map is built from result.OrphanFiles. Any deleted path not present in OrphanFiles would silently render as 0 B rather than being caught. Today DeletedFiles ⊆ OrphanFiles so it's safe, but that's an implicit invariant — worth a short comment, or build the deleted entries by filtering OrphanFiles against the deleted set so the size always
stays attached to its path.
| type OrphanFileEntry struct { | ||
| Path string `json:"path"` | ||
| SizeBytes int64 `json:"size_bytes,omitempty"` | ||
| SizeBytes int64 `json:"size_bytes"` |
There was a problem hiding this comment.
Dropping omitempty means a genuinely 0-byte orphan now always serializes "size_bytes":0 instead of omitting the field. I think that's the right move for a stable output schema (and it matches PartitionStatsEntry.SizeBytes at line 146, which also has no omitempty) — just flagging it as an observable JSON change for anyone parsing the output.
2 participants
Summary
Testing