fix(record): remove the empty dataset dir a failed recording leaves behind - #113
Open
ravediamond wants to merge 1 commit into
Open
ravediamond wants to merge 1 commit into
ravediamond wants to merge 1 commit into
Conversation
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.
Closes #112.
When a recording fails before any episode is saved, the dataset directory it already created gets left on disk and shows up in the dataset list as if something was recorded.
Added
_cleanup_failed_recording(), called from the recording worker's error handler: if the session wasn't resuming an existing dataset and saved 0 episodes, it removes the dataset directory. Best-effort — logs and swallows any cleanup failure rather than masking the original recording error.Also pulled the path-traversal guard out of
handle_delete_datasetinto_resolve_dataset_dirso the new cleanup path reuses it instead of a third copy, and switched it to read the cache root from the environment at call time (same asepisode_media.lerobot_cache_root()) instead of importing lerobot'sHF_LEROBOT_HOME, which is frozen at import time — the old version would have silently used the wrong root if the env var changed after the module first loaded. Writing a test for it is what surfaced this.Tested: pytest (277 passed, 9 new tests covering the traversal guard, delete-dataset, and the four cleanup branches), ruff check/format clean. No frontend changes.