From 176bfa6ccbac22cf29ef24cf87a825676caa5dfc Mon Sep 17 00:00:00 2001 From: "Ivan A. Melnikov" Date: Thu, 17 Oct 2013 16:35:32 +0400 Subject: [PATCH] Do not erase task progress details Before this change we used to loose any progress details set by task when it was updating its progress when task state was changed from RUNNING to SUCCESS (and thus progress forced to 1.0). This commit fixes this, so that when progress is updated from the engine latest progress details are preserved. Breaking change: to allow details to be associated with progress value they were set for we save progress and then return progress value with details. Change-Id: Ic90e61ee3dcf62731696a0f10134bc448e7d6384 --- taskflow/engines/action_engine/task_action.py | 2 +- taskflow/storage.py | 18 +++++++------ taskflow/tests/unit/test_progress.py | 25 +++++++++++++++++-- taskflow/tests/unit/test_storage.py | 23 ++++++++++++++--- 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/taskflow/engines/action_engine/task_action.py b/taskflow/engines/action_engine/task_action.py index 5ab7eeec..984e2b55 100644 --- a/taskflow/engines/action_engine/task_action.py +++ b/taskflow/engines/action_engine/task_action.py @@ -73,7 +73,7 @@ class TaskAction(base.Action): """Update task progress value that stored in engine.""" try: engine = event_data['engine'] - engine.storage.set_task_progress(self.uuid, progress, **kwargs) + engine.storage.set_task_progress(self.uuid, progress, kwargs) except Exception: # Update progress callbacks should never fail, so capture and log # the emitted exception instead of raising it. diff --git a/taskflow/storage.py b/taskflow/storage.py index bee11f52..5e659558 100644 --- a/taskflow/storage.py +++ b/taskflow/storage.py @@ -144,22 +144,26 @@ class Storage(object): """Get state of task with given uuid""" return self._taskdetail_by_uuid(uuid).state - def set_task_progress(self, uuid, progress, **kwargs): + def set_task_progress(self, uuid, progress, details=None): """Set task progress. :param uuid: task uuid :param progress: task progress - :param kwargs: task specific progress information + :param details: task specific progress information """ td = self._taskdetail_by_uuid(uuid) if not td.meta: td.meta = {} td.meta['progress'] = progress - if kwargs: - td.meta['progress_details'] = kwargs - else: - if 'progress_details' in td.meta: - td.meta.pop('progress_details') + if details is not None: + # NOTE(imelnikov): as we can update progress without + # updating details (e.g. automatically from engine) + # we save progress value with details, too + if details: + td.meta['progress_details'] = dict(at_progress=progress, + details=details) + else: + td.meta['progress_details'] = None self._with_connection(self._save_task_detail, task_detail=td) def get_task_progress(self, uuid): diff --git a/taskflow/tests/unit/test_progress.py b/taskflow/tests/unit/test_progress.py index 2b97f4e0..b268cbc7 100644 --- a/taskflow/tests/unit/test_progress.py +++ b/taskflow/tests/unit/test_progress.py @@ -40,6 +40,11 @@ class ProgressTask(task.Task): self.update_progress(progress) +class ProgressTaskWithDetails(task.Task): + def execute(self): + self.update_progress(0.5, test='test data', foo='bar') + + class TestProgress(test.TestCase): def _make_engine(self, flow, flow_detail=None, backend=None): e = taskflow.engines.load(flow, @@ -99,7 +104,22 @@ class TestProgress(test.TestCase): end_progress = e.storage.get_task_progress(t_uuid) self.assertEquals(1.0, end_progress) td = fd.find(t_uuid) - self.assertEquals({'progress': 1.0}, td.meta) + self.assertEquals(1.0, td.meta['progress']) + self.assertFalse(td.meta['progress_details']) + + def test_storage_progress_detail(self): + flo = ProgressTaskWithDetails("test") + e = self._make_engine(flo) + e.run() + t_uuid = e.storage.get_uuid_by_name("test") + end_progress = e.storage.get_task_progress(t_uuid) + self.assertEquals(1.0, end_progress) + end_details = e.storage.get_task_progress_details(t_uuid) + self.assertEquals(end_details.get('at_progress'), 0.5) + self.assertEquals(end_details.get('details'), { + 'test': 'test data', + 'foo': 'bar' + }) def test_dual_storage_progress(self): fired_events = [] @@ -120,5 +140,6 @@ class TestProgress(test.TestCase): end_progress = e.storage.get_task_progress(t_uuid) self.assertEquals(1.0, end_progress) td = fd.find(t_uuid) - self.assertEquals({'progress': 1.0}, td.meta) + self.assertEquals(1.0, td.meta['progress']) + self.assertFalse(td.meta['progress_details']) self.assertEquals(6, len(fired_events)) diff --git a/taskflow/tests/unit/test_storage.py b/taskflow/tests/unit/test_storage.py index ec629158..f1fcefb1 100644 --- a/taskflow/tests/unit/test_storage.py +++ b/taskflow/tests/unit/test_storage.py @@ -139,10 +139,27 @@ class StorageTest(test.TestCase): def test_task_progress(self): s = self._get_storage() s.add_task('42', 'my task') - s.set_task_progress('42', 0.5, test_data=11) + + s.set_task_progress('42', 0.5, {'test_data': 11}) self.assertEquals(s.get_task_progress('42'), 0.5) - self.assertEquals(s.get_task_progress_details('42'), - {'test_data': 11}) + self.assertEquals(s.get_task_progress_details('42'), { + 'at_progress': 0.5, + 'details': {'test_data': 11} + }) + + s.set_task_progress('42', 0.7, {'test_data': 17}) + self.assertEquals(s.get_task_progress('42'), 0.7) + self.assertEquals(s.get_task_progress_details('42'), { + 'at_progress': 0.7, + 'details': {'test_data': 17} + }) + + s.set_task_progress('42', 0.99) + self.assertEquals(s.get_task_progress('42'), 0.99) + self.assertEquals(s.get_task_progress_details('42'), { + 'at_progress': 0.7, + 'details': {'test_data': 17} + }) def test_fetch_result_not_ready(self): s = self._get_storage()