Skip to content

Commit a5ff6f8

Browse files
mihowclaude
andcommitted
refactor: rename _get_progress to _commit_update in TaskStateManager
Clarify naming to distinguish mutating vs read-only methods: - _commit_update(): private, writes mutations to Redis, returns progress - get_progress(): public, read-only snapshot (added in #1129) - update_state(): public API, acquires lock, calls _commit_update() Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 89bf950 commit a5ff6f8

2 files changed

Lines changed: 29 additions & 25 deletions

File tree

ami/ml/orchestration/task_state.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ def update_state(
101101

102102
try:
103103
# Update progress tracking in Redis
104-
progress_info = self._get_progress(
104+
progress_info = self._commit_update(
105105
processed_image_ids, stage, detections_count, classifications_count, captures_count, failed_image_ids
106106
)
107107
return progress_info
@@ -134,7 +134,7 @@ def get_progress(self, stage: str) -> TaskProgress | None:
134134
failed=len(failed_set),
135135
)
136136

137-
def _get_progress(
137+
def _commit_update(
138138
self,
139139
processed_image_ids: set[str],
140140
stage: str,

ami/ml/tests.py

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,7 @@ def setUp(self):
874874
def _init_and_verify(self, image_ids):
875875
"""Helper to initialize job and verify initial state."""
876876
self.manager.initialize_job(image_ids)
877-
progress = self.manager._get_progress(set(), "process")
877+
progress = self.manager._commit_update(set(), "process")
878878
assert progress is not None
879879
self.assertEqual(progress.total, len(image_ids))
880880
self.assertEqual(progress.remaining, len(image_ids))
@@ -892,7 +892,7 @@ def test_initialize_job(self):
892892

893893
# Verify both stages are initialized
894894
for stage in self.manager.STAGES:
895-
progress = self.manager._get_progress(set(), stage)
895+
progress = self.manager._commit_update(set(), stage)
896896
assert progress is not None
897897
self.assertEqual(progress.total, len(self.image_ids))
898898
self.assertEqual(progress.detections, 0)
@@ -904,7 +904,7 @@ def test_progress_tracking(self):
904904
self._init_and_verify(self.image_ids)
905905

906906
# Process 2 images
907-
progress = self.manager._get_progress({"img1", "img2"}, "process")
907+
progress = self.manager._commit_update({"img1", "img2"}, "process")
908908
assert progress is not None
909909
self.assertEqual(progress.remaining, 3)
910910
self.assertEqual(progress.processed, 2)
@@ -913,14 +913,14 @@ def test_progress_tracking(self):
913913
self.assertEqual(progress.classifications, 0)
914914

915915
# Process 2 more images
916-
progress = self.manager._get_progress({"img3", "img4"}, "process")
916+
progress = self.manager._commit_update({"img3", "img4"}, "process")
917917
assert progress is not None
918918
self.assertEqual(progress.remaining, 1)
919919
self.assertEqual(progress.processed, 4)
920920
self.assertEqual(progress.percentage, 0.8)
921921

922922
# Process last image
923-
progress = self.manager._get_progress({"img5"}, "process")
923+
progress = self.manager._commit_update({"img5"}, "process")
924924
assert progress is not None
925925
self.assertEqual(progress.remaining, 0)
926926
self.assertEqual(progress.processed, 5)
@@ -958,20 +958,20 @@ def test_stages_independent(self):
958958
self._init_and_verify(self.image_ids)
959959

960960
# Update process stage
961-
self.manager._get_progress({"img1", "img2"}, "process")
962-
progress_process = self.manager._get_progress(set(), "process")
961+
self.manager._commit_update({"img1", "img2"}, "process")
962+
progress_process = self.manager._commit_update(set(), "process")
963963
assert progress_process is not None
964964
self.assertEqual(progress_process.remaining, 3)
965965

966966
# Results stage should still have all images pending
967-
progress_results = self.manager._get_progress(set(), "results")
967+
progress_results = self.manager._commit_update(set(), "results")
968968
assert progress_results is not None
969969
self.assertEqual(progress_results.remaining, 5)
970970

971971
def test_empty_job(self):
972972
"""Test handling of job with no images."""
973973
self.manager.initialize_job([])
974-
progress = self.manager._get_progress(set(), "process")
974+
progress = self.manager._commit_update(set(), "process")
975975
assert progress is not None
976976
self.assertEqual(progress.total, 0)
977977
self.assertEqual(progress.percentage, 1.0) # Empty job is 100% complete
@@ -983,36 +983,36 @@ def test_cleanup(self):
983983
self._init_and_verify(self.image_ids)
984984

985985
# Verify keys exist
986-
progress = self.manager._get_progress(set(), "process")
986+
progress = self.manager._commit_update(set(), "process")
987987
self.assertIsNotNone(progress)
988988

989989
# Cleanup
990990
self.manager.cleanup()
991991

992992
# Verify keys are gone
993-
progress = self.manager._get_progress(set(), "process")
993+
progress = self.manager._commit_update(set(), "process")
994994
self.assertIsNone(progress)
995995

996996
def test_cumulative_counting(self):
997997
"""Test that detection counts accumulate correctly across updates."""
998998
self._init_and_verify(self.image_ids)
999999

10001000
# Process first batch with some detections
1001-
progress = self.manager._get_progress({"img1", "img2"}, "process", detections_count=3)
1001+
progress = self.manager._commit_update({"img1", "img2"}, "process", detections_count=3)
10021002
assert progress is not None
10031003
self.assertEqual(progress.detections, 3)
10041004
self.assertEqual(progress.classifications, 0)
10051005
self.assertEqual(progress.captures, 0)
10061006

10071007
# Process second batch with more detections and a classification
1008-
progress = self.manager._get_progress({"img3"}, "process", detections_count=2, classifications_count=1)
1008+
progress = self.manager._commit_update({"img3"}, "process", detections_count=2, classifications_count=1)
10091009
assert progress is not None
10101010
self.assertEqual(progress.detections, 5) # Should be cumulative
10111011
self.assertEqual(progress.classifications, 1)
10121012
self.assertEqual(progress.captures, 0)
10131013

10141014
# Process with detections, classifications, and captures
1015-
progress = self.manager._get_progress(
1015+
progress = self.manager._commit_update(
10161016
{"img4"}, "results", detections_count=1, classifications_count=4, captures_count=1
10171017
)
10181018
assert progress is not None
@@ -1025,18 +1025,20 @@ def test_counts_persist_across_stages(self):
10251025
self._init_and_verify(self.image_ids)
10261026

10271027
# Add counts during process stage
1028-
progress_process = self.manager._get_progress({"img1"}, "process", detections_count=3)
1028+
progress_process = self.manager._commit_update({"img1"}, "process", detections_count=3)
10291029
assert progress_process is not None
10301030
self.assertEqual(progress_process.detections, 3)
10311031

10321032
# Verify counts are available in results stage
1033-
progress_results = self.manager._get_progress(set(), "results")
1033+
progress_results = self.manager._commit_update(set(), "results")
10341034
assert progress_results is not None
10351035
self.assertEqual(progress_results.detections, 3) # Should persist
10361036
self.assertEqual(progress_results.classifications, 0)
10371037

10381038
# Add more counts in results stage
1039-
progress_results = self.manager._get_progress({"img2"}, "results", detections_count=1, classifications_count=5)
1039+
progress_results = self.manager._commit_update(
1040+
{"img2"}, "results", detections_count=1, classifications_count=5
1041+
)
10401042
assert progress_results is not None
10411043
self.assertEqual(progress_results.detections, 4) # Should accumulate
10421044
self.assertEqual(progress_results.classifications, 5)
@@ -1048,7 +1050,9 @@ def test_cleanup_removes_count_keys(self):
10481050
self._init_and_verify(self.image_ids)
10491051

10501052
# Add some counts
1051-
self.manager._get_progress({"img1"}, "process", detections_count=5, classifications_count=10, captures_count=2)
1053+
self.manager._commit_update(
1054+
{"img1"}, "process", detections_count=5, classifications_count=10, captures_count=2
1055+
)
10521056

10531057
# Verify count keys exist
10541058
detections = cache.get(self.manager._detections_key)
@@ -1074,17 +1078,17 @@ def test_failed_image_tracking(self):
10741078
self._init_and_verify(self.image_ids)
10751079

10761080
# Mark 2 images as failed in process stage
1077-
progress = self.manager._get_progress({"img1", "img2"}, "process", failed_image_ids={"img1", "img2"})
1081+
progress = self.manager._commit_update({"img1", "img2"}, "process", failed_image_ids={"img1", "img2"})
10781082
assert progress is not None
10791083
self.assertEqual(progress.failed, 2)
10801084

10811085
# Retry same 2 images (fail again) - should not double-count
1082-
progress = self.manager._get_progress(set(), "process", failed_image_ids={"img1", "img2"})
1086+
progress = self.manager._commit_update(set(), "process", failed_image_ids={"img1", "img2"})
10831087
assert progress is not None
10841088
self.assertEqual(progress.failed, 2)
10851089

10861090
# Fail a different image
1087-
progress = self.manager._get_progress(set(), "process", failed_image_ids={"img3"})
1091+
progress = self.manager._commit_update(set(), "process", failed_image_ids={"img3"})
10881092
assert progress is not None
10891093
self.assertEqual(progress.failed, 3)
10901094

@@ -1093,7 +1097,7 @@ def test_failed_and_processed_mixed(self):
10931097
self._init_and_verify(self.image_ids)
10941098

10951099
# Process 2 successfully, 2 fail, 1 remains pending
1096-
progress = self.manager._get_progress(
1100+
progress = self.manager._commit_update(
10971101
{"img1", "img2", "img3", "img4"}, "process", failed_image_ids={"img3", "img4"}
10981102
)
10991103
assert progress is not None
@@ -1109,7 +1113,7 @@ def test_cleanup_removes_failed_set(self):
11091113
self._init_and_verify(self.image_ids)
11101114

11111115
# Add failed images
1112-
self.manager._get_progress({"img1", "img2"}, "process", failed_image_ids={"img1", "img2"})
1116+
self.manager._commit_update({"img1", "img2"}, "process", failed_image_ids={"img1", "img2"})
11131117

11141118
# Verify failed set exists
11151119
failed_set = cache.get(self.manager._failed_key)

0 commit comments

Comments
 (0)