Skip to content

Commit 2155c26

Browse files
TanishGudiseclaude
andcommitted
fix: remove the temporary dataset file when moving it into place fails
If os.replace raised (for example a permission error, or a directory already at the dataset path), the completed temporary file was left in the cache directory. Run the move inside the try and remove the temporary file in finally, so it is cleaned up after any failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 1c8f5e9 commit 2155c26

2 files changed

Lines changed: 19 additions & 3 deletions

File tree

‎src/bespokelabs/curator/request_processor/base_request_processor.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ def _write_atomically(path: str):
4141
tmp_path = f"{path}.{os.getpid()}.tmp"
4242
try:
4343
yield tmp_path
44-
except BaseException:
44+
os.replace(tmp_path, path)
45+
finally:
46+
# Already moved after a successful replace; otherwise the write or the move failed.
4547
if os.path.exists(tmp_path):
4648
os.remove(tmp_path)
47-
raise
48-
os.replace(tmp_path, path)
4949

5050

5151
class BaseRequestProcessor(ABC):

‎tests/unittests/test_create_dataset_files.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,19 @@ def test_dataset_is_rebuilt_from_cached_responses_after_interrupted_build(tmp_pa
104104
assert sorted(os.listdir(tmp_path)) == sorted(
105105
[f"{_PARSE_FUNC_HASH}.arrow", "failed_requests.jsonl", "metadata_0.json", "requests_0.jsonl", "responses_0.jsonl"]
106106
)
107+
108+
109+
def test_failed_move_into_place_leaves_no_temporary_file(tmp_path, monkeypatch):
110+
"""If the final rename fails (for example a permission error), the error surfaces and no dataset or temporary file is left behind."""
111+
_write_cached_run(tmp_path)
112+
113+
def failing_replace(src, dst):
114+
raise PermissionError(f"cannot move {src} to {dst}")
115+
116+
with monkeypatch.context() as patch:
117+
patch.setattr(os, "replace", failing_replace)
118+
with pytest.raises(PermissionError):
119+
_processor(tmp_path, _parse).create_dataset_files(_PARSE_FUNC_HASH)
120+
121+
assert sorted(os.listdir(tmp_path)) == sorted(["failed_requests.jsonl", "metadata_0.json", "requests_0.jsonl", "responses_0.jsonl"])
122+
assert _processor(tmp_path, _parse).attempt_loading_cached_dataset(_PARSE_FUNC_HASH) is None

0 commit comments

Comments
 (0)