Skip to content

Commit c402c93

Browse files
Copilotnolan1999
andcommitted
Implement comprehensive cleanup for local worker upon successful job run and upload
Co-authored-by: nolan1999 <54246789+nolan1999@users.noreply.github.com>
1 parent b61383c commit c402c93

2 files changed

Lines changed: 122 additions & 4 deletions

File tree

cli/main.py

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,11 @@ def main() -> None:
4242
worker_info = info.sys.collect()
4343

4444
while True:
45+
container = None
46+
job_id = None
47+
path_job = None
48+
4549
try:
46-
container = None
4750
jobs = api_worker.fetch_jobs(
4851
limit=1,
4952
cpu_cores=worker_info.sys.cores,
@@ -176,9 +179,36 @@ def main() -> None:
176179
container.kill()
177180
else:
178181
raise e
179-
180-
logger.info(f"Job {job_id} finished")
181-
shutil.rmtree(path_job)
182+
183+
finally:
184+
# Clean up resources after successful job run AND upload
185+
if job_id:
186+
logger.info(f"Cleaning up job {job_id}")
187+
188+
# Clean up Docker container
189+
if container:
190+
try:
191+
if container.status == "running":
192+
logger.info(f"Stopping running container for job {job_id}")
193+
container.stop()
194+
except Exception as e:
195+
logger.warning(f"Failed to stop container for job {job_id}: {e}")
196+
197+
try:
198+
logger.info(f"Removing container for job {job_id}")
199+
container.remove()
200+
except Exception as e:
201+
logger.warning(f"Failed to remove container for job {job_id}: {e}")
202+
203+
# Clean up job directory
204+
if path_job and path_job.exists():
205+
try:
206+
logger.info(f"Removing job directory {path_job}")
207+
shutil.rmtree(path_job)
208+
except Exception as e:
209+
logger.warning(f"Failed to clean up job directory {path_job}: {e}")
210+
211+
logger.info(f"Job {job_id} cleanup completed")
182212

183213

184214
if __name__ == "__main__":

tests/unit/test_cleanup.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
"""Tests for cleanup functionality in the main job processing loop."""
2+
3+
import shutil
4+
import tempfile
5+
from pathlib import Path
6+
from unittest.mock import Mock, MagicMock
7+
8+
import pytest
9+
10+
11+
class TestJobCleanup:
12+
"""Test cleanup behavior for local worker jobs."""
13+
14+
def test_container_cleanup_logic(self):
15+
"""Test the container cleanup logic works correctly."""
16+
# Mock a container
17+
mock_container = Mock()
18+
mock_container.status = "running"
19+
20+
job_id = "test_job_123"
21+
22+
# Test cleanup logic (simulating the finally block)
23+
if job_id:
24+
if mock_container:
25+
try:
26+
if mock_container.status == "running":
27+
mock_container.stop()
28+
mock_container.remove()
29+
except Exception as e:
30+
# Should handle exceptions gracefully
31+
pass
32+
33+
# Verify methods were called
34+
mock_container.stop.assert_called_once()
35+
mock_container.remove.assert_called_once()
36+
37+
def test_container_cleanup_handles_exceptions(self):
38+
"""Test that container cleanup handles exceptions gracefully."""
39+
# Mock a container that raises exceptions
40+
mock_container = Mock()
41+
mock_container.status = "running"
42+
mock_container.stop.side_effect = Exception("Stop failed")
43+
mock_container.remove.side_effect = Exception("Remove failed")
44+
45+
job_id = "test_job_456"
46+
47+
# Test cleanup logic should handle each step independently
48+
if job_id:
49+
if mock_container:
50+
try:
51+
if mock_container.status == "running":
52+
mock_container.stop()
53+
except Exception:
54+
pass
55+
try:
56+
mock_container.remove()
57+
except Exception:
58+
pass
59+
60+
# Verify methods were called despite exceptions
61+
mock_container.stop.assert_called_once()
62+
mock_container.remove.assert_called_once()
63+
64+
def test_cleanup_defensive_programming(self):
65+
"""Test that cleanup handles missing variables gracefully."""
66+
# This test verifies that the cleanup code can handle cases where
67+
# container, job_id, or path_job might be None or undefined
68+
69+
# Create a temporary directory for testing
70+
with tempfile.TemporaryDirectory() as temp_dir:
71+
path_job = Path(temp_dir) / "test_job"
72+
path_job.mkdir()
73+
74+
# Test cleanup with missing container (should not crash)
75+
container = None
76+
job_id = "test_job"
77+
78+
# Simulate the cleanup code
79+
if job_id:
80+
if container:
81+
# This should not execute
82+
assert False, "Should not try to clean up None container"
83+
84+
if path_job and path_job.exists():
85+
shutil.rmtree(path_job)
86+
87+
# Verify directory was cleaned up
88+
assert not path_job.exists()

0 commit comments

Comments
 (0)