Assume every job in database does have field last_slurm_update set.

Update tests.
mila-iqia · Jul 2, 2024 · e11c57b · e11c57b
1 parent 7553884
commit e11c57b
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 15 deletions.
diff --git a/scripts/cleanup_jobs.py b/scripts/cleanup_jobs.py
@@ -84,7 +84,6 @@ def keep_n_most_recent_jobs(n: int):
     # Find jobs to delete.
     # Sort jobs by slurm_last_update ascending
     # and keep only first jobs, excluding n last jobs.
-    # NB: Jobs that don't have `last_slurm_update` will appear first in sorting.
     jobs_to_delete = list(
         db_jobs.find({}).sort([("cw.last_slurm_update", 1)]).limit(nb_total_jobs - n)
     )
@@ -124,7 +123,6 @@ def keep_n_most_recent_jobs_per_user(n: int):
         # Find user jobs to delete.
         # Sort jobs by slurm_last_update ascending
         # and keep only first jobs, excluding n last jobs.
-        # NB: Jobs that don't have `last_slurm_update` will appear first in sorting.
         user_jobs_to_delete = list(
             db_jobs.find(user_filter)
             .sort([("cw.last_slurm_update", 1)])
@@ -158,7 +156,6 @@ def keep_jobs_from_date(date: datetime):
     nb_total_jobs = db_jobs.count_documents({})
 
     # Find jobs to delete
-    # NB: Jobs that don't have `last_slurm_update` won't be found by this filter.
 
     jobs_to_delete = list(
         db_jobs.find({"cw.last_slurm_update": {"$lt": date.timestamp()}})

diff --git a/scripts_test/test_cleanup_jobs.py b/scripts_test/test_cleanup_jobs.py
@@ -60,9 +60,6 @@ def __init__(self):
             }
             for i, fake_job in enumerate(fake_jobs)
         ]
-        # Unset `last_slurm_update` for 2 first jobs.
-        del fake_jobs[0]["cw"]["last_slurm_update"]
-        fake_jobs[1]["cw"]["last_slurm_update"] = None
 
         db_users = mc["users"]
         db_jobs = mc["jobs"]
@@ -263,20 +260,17 @@ def _fmt_date(d: datetime):
         assert jobs == ctx.check_user_props()
 
         cleanup_jobs(["-d", _fmt_date(inbound_date_1)])
-        # In database, 2 jobs don't have last_slurm_update,
-        # so these jobs should never be deleted under `-d` argument.
         remaining_jobs = ctx.check_user_props()
-        assert len(remaining_jobs) == 100 - (15 - 2)
-        assert jobs[:2] + jobs[15:] == remaining_jobs
+        assert len(remaining_jobs) == 100 - 15
+        assert jobs[15:] == remaining_jobs
 
         cleanup_jobs(["-d", _fmt_date(inbound_date_2)])
         remaining_jobs = ctx.check_user_props()
-        assert len(remaining_jobs) == 100 - (60 - 2)
-        assert jobs[:2] + jobs[60:] == remaining_jobs
+        assert len(remaining_jobs) == 100 - 60
+        assert jobs[60:] == remaining_jobs
 
         cleanup_jobs(["-d", _fmt_date(new_date)])
         # With a date more recent than latest job,
-        # all jobs should be deleted, excluding jobs that don't have last_slurm_update.
+        # all jobs should be deleted.
         remaining_jobs = ctx.check_user_props()
-        assert len(remaining_jobs) == 2
-        assert jobs[:2] == remaining_jobs
+        assert len(remaining_jobs) == 0