Fail when explicitly deleted data file is missing - #3818
Conversation
49d7ef9 to
ae10cb9
Compare
kevinjqliu
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the fix.
I pushed up a commit based on review with codex. Its mostly simple refactor of your existing code.
The only other significant change is in _write_delete_manifest: moving self._deleted_entries() out into the parent will run the validation code before we start writing manifests
|
Hi @rambleraptor, nice fix! While reviewing this I noticed a related pre-existing gap: This means Filed as #3857 with a reproduction script. |
Rationale for this change
Fixes #3814
This looks like the problem I pointed out in #3811 but didn't explain properly enough. We should look for all explicitly deleted files and make sure they exist. This matches how Java handles the situation.
Are these changes tested?
Included unit tests based off the issue's verification script
Are there any user-facing changes?