Skip to content

Commit acaa942

Browse files
committed
add suggestions
Signed-off-by: Sarah Yurick <sarahyurick@gmail.com>
1 parent e6ca76c commit acaa942

4 files changed

Lines changed: 12 additions & 34 deletions

File tree

nemo_curator/backends/base.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,12 @@ def process_batch(self, tasks: list[Task]) -> list[Task]:
122122
raise ValueError(msg)
123123

124124
# Record failed tasks for later inspection or retry bookkeeping.
125-
record_failed_tasks(failed_tasks)
125+
if failed_tasks:
126+
record_failed_tasks()
126127

127128
# Filter tasks based on the Slurm array configuration.
128129
slurm_array = resolve_slurm_array_config(is_source_stage=is_source_stage)
129-
if slurm_array is not None:
130+
if slurm_array is not None and is_source_stage:
130131
results = filter_slurm_array_source_tasks(results, slurm_array, self.stage.name)
131132

132133
# Opt-in resumability: fire per-source deltas (no-op when no actor registered).

nemo_curator/backends/failed_task_markers.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import uuid
1717
from pathlib import Path
1818

19-
from nemo_curator.tasks.sentinels import FailedTask
2019
from nemo_curator.utils.retry_manifest import METADATA_DIRNAME
2120

2221
FAILED_TASKS_DIR_ENV_VAR = "NEMO_CURATOR_FAILED_TASKS_DIR"
@@ -61,10 +60,8 @@ def configure_slurm_array_failed_task_manifest_dir(checkpoint_path: str | Path,
6160
return _configure_failed_task_manifest_dir(manifest_dir)
6261

6362

64-
def record_failed_tasks(failed_tasks: list[FailedTask]) -> None:
63+
def record_failed_tasks() -> None:
6564
"""Write one attempt-scoped manifest after any FailedTask is detected."""
66-
if not failed_tasks:
67-
return
6865
manifest_dir = os.environ.get(FAILED_TASKS_DIR_ENV_VAR)
6966
if not manifest_dir:
7067
return

tests/backends/test_base_stage_adapter.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ def filter_tasks(
104104
calls["filter_tasks"] += 1
105105
return tasks
106106

107-
def record_failed_tasks(_failed_tasks: list[FailedTask]) -> None:
107+
def record_failed_tasks() -> None:
108108
calls["record_failed_tasks"] += 1
109109

110110
monkeypatch.setattr(base_module, "resolve_slurm_array_config", resolve_config)

tests/backends/test_failed_task_markers.py

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,6 @@
2626
failed_task_manifest_exists,
2727
record_failed_tasks,
2828
)
29-
from nemo_curator.tasks.sentinels import FailedTask
30-
31-
32-
def _failed_task(task_id: str = "0_7_0") -> FailedTask:
33-
task = FailedTask()
34-
task.task_id = task_id
35-
return task
3629

3730

3831
class TestFailedTaskManifest:
@@ -82,7 +75,7 @@ def test_record_failed_tasks_writes_single_manifest(
8275
manifest_dir = tmp_path / "failed-tasks"
8376
monkeypatch.setenv(FAILED_TASKS_DIR_ENV_VAR, str(manifest_dir))
8477

85-
record_failed_tasks([_failed_task("0_7_0"), _failed_task("0_8_0")])
78+
record_failed_tasks()
8679

8780
manifest_files = list(manifest_dir.glob("*.json"))
8881
assert manifest_files == [manifest_dir / FAILED_TASK_MANIFEST_FILENAME]
@@ -93,55 +86,42 @@ def test_additional_failed_tasks_leave_existing_manifest_unchanged(
9386
) -> None:
9487
manifest_dir = tmp_path / "failed-tasks"
9588
monkeypatch.setenv(FAILED_TASKS_DIR_ENV_VAR, str(manifest_dir))
96-
record_failed_tasks([_failed_task("0_7_0")])
89+
record_failed_tasks()
9790
manifest_file = manifest_dir / FAILED_TASK_MANIFEST_FILENAME
98-
original_manifest = manifest_file.read_text()
9991

100-
record_failed_tasks([_failed_task("0_8_0")])
92+
record_failed_tasks()
10193

10294
assert list(manifest_dir.glob("*.json")) == [manifest_file]
103-
assert manifest_file.read_text() == original_manifest
10495

10596
def test_record_failed_tasks_without_configured_attempt_is_noop(
10697
self, tmp_path: Path, monkeypatch: MonkeyPatch
10798
) -> None:
10899
monkeypatch.delenv(FAILED_TASKS_DIR_ENV_VAR, raising=False)
109100
monkeypatch.chdir(tmp_path)
110101

111-
record_failed_tasks([_failed_task()])
102+
record_failed_tasks()
112103

113104
assert not (tmp_path / ".nemo_curator_metadata").exists()
114105

115-
def test_record_failed_tasks_does_not_write_manifest_for_empty_list(
116-
self, tmp_path: Path, monkeypatch: MonkeyPatch
117-
) -> None:
118-
manifest_dir = tmp_path / "failed-tasks"
119-
monkeypatch.setenv(FAILED_TASKS_DIR_ENV_VAR, str(manifest_dir))
120-
121-
record_failed_tasks([])
122-
123-
assert not manifest_dir.exists()
124-
assert not failed_task_manifest_exists()
125-
126106
def test_record_failed_tasks_propagates_manifest_write_failure(
127107
self, tmp_path: Path, monkeypatch: MonkeyPatch
128108
) -> None:
129109
manifest_dir = tmp_path / "failed-tasks"
130110
monkeypatch.setenv(FAILED_TASKS_DIR_ENV_VAR, str(manifest_dir))
131111

132-
def fail_touch(self: Path, mode: int = 0o666, exist_ok: bool = True) -> None:
112+
def fail_touch(_self: Path, _mode: int = 0o666, _exist_ok: bool = True) -> None:
133113
msg = "storage unavailable"
134114
raise OSError(msg)
135115

136116
monkeypatch.setattr(Path, "touch", fail_touch)
137117

138118
with pytest.raises(OSError, match="storage unavailable"):
139-
record_failed_tasks([_failed_task()])
119+
record_failed_tasks()
140120

141121
def test_failed_task_manifest_exists_accepts_explicit_directory(self, tmp_path: Path) -> None:
142122
manifest_dir = tmp_path / "failed-tasks"
143123
manifest_dir.mkdir()
144-
(manifest_dir / FAILED_TASK_MANIFEST_FILENAME).write_text('{"status":"failed_tasks"}\n')
124+
(manifest_dir / FAILED_TASK_MANIFEST_FILENAME).touch()
145125

146126
assert failed_task_manifest_exists(manifest_dir)
147127
assert not failed_task_manifest_exists(tmp_path / "missing")

0 commit comments

Comments
 (0)