From 134687936b81114f82a12e2d7d691e4f65a32ca0 Mon Sep 17 00:00:00 2001 From: Amanpreet Singh Saimbhi Date: Wed, 20 Nov 2024 23:13:34 +0000 Subject: [PATCH 1/7] add initial setup for force study completion --- common/constants.py | 2 ++ common/types.py | 2 ++ routing/generate_taskfile.py | 1 + routing/route_studies.py | 21 +++++++++++++++++++++ webinterface/rules.py | 1 + webinterface/templates/rules_edit.html | 24 ++++++++++++++++++++++++ 6 files changed, 51 insertions(+) diff --git a/common/constants.py b/common/constants.py index 2da5274c..6e1329c1 100755 --- a/common/constants.py +++ b/common/constants.py @@ -67,6 +67,7 @@ class mercure_rule: ACTION = "action" ACTION_TRIGGER = "action_trigger" STUDY_TRIGGER_CONDITION = "study_trigger_condition" + STUDY_FORCE_COMPLETION_ACTION = "study_force_completion_action" STUDY_TRIGGER_CONDITION_TIMEOUT = "timeout" STUDY_TRIGGER_CONDITION_RECEIVED_SERIES = "received_series" STUDY_TRIGGER = "study_trigger" @@ -91,6 +92,7 @@ class mercure_study: COMPLETE_TRIGGER = "complete_trigger" COMPLETE_REQUIRED_SERIES = "complete_required_series" COMPLETE_FORCE = "complete_force" + COMPLETE_FORCE_ACTION = "complete_force_action" class mercure_info: diff --git a/common/types.py b/common/types.py index af0636a4..51cc5abb 100755 --- a/common/types.py +++ b/common/types.py @@ -190,6 +190,7 @@ class Rule(BaseModel, Compat): action: Literal["route", "both", "process", "discard", "notification"] = "route" action_trigger: Literal["series", "study"] = "series" study_trigger_condition: Literal["timeout", "received_series"] = "timeout" + study_force_completion_action: Literal["discard", "proceed", "ignore"] = "discard" study_trigger_series: str = "" priority: Literal["normal", "urgent", "offpeak"] = "normal" processing_module: Union[str,List[str]] = "" @@ -338,6 +339,7 @@ class TaskStudy(BaseModel, Compat): received_series: Optional[List[str]] received_series_uid: Optional[List[str]] complete_force: bool = False + complete_force_action: Optional[str] = "discard" class TaskProcessing(BaseModel, Compat): diff --git a/routing/generate_taskfile.py b/routing/generate_taskfile.py index 1908a0a4..f94d8da6 100755 --- a/routing/generate_taskfile.py +++ b/routing/generate_taskfile.py @@ -144,6 +144,7 @@ def add_study( received_series=[tags_list.get("SeriesDescription", mercure_options.INVALID)], received_series_uid=[tags_list.get("SeriesInstanceUID", mercure_options.INVALID)], complete_force=False, + complete_force_action=config.mercure.rules[applied_rule].study_force_completion_action, ) return study_info diff --git a/routing/route_studies.py b/routing/route_studies.py index a8137b35..d18afb3a 100755 --- a/routing/route_studies.py +++ b/routing/route_studies.py @@ -52,6 +52,8 @@ def route_studies(pending_series: Dict[str, float]) -> None: if entry.is_dir() and not is_study_locked(entry.path) and is_study_complete(entry.path, pending_series): modificationTime = entry.stat().st_mtime studies_ready[entry.name] = modificationTime + if entry.is_dir() and not is_study_locked(entry.path) and check_force_study_timeout(entry.path, pending_series): + logger.info(f"Force completion timeout met for study {entry.name}") logger.debug(f"Studies ready for processing: {studies_ready}") # Process all complete studies for dir_entry in sorted(studies_ready): @@ -172,6 +174,25 @@ def check_study_timeout(task: TaskHasStudy, pending_series: Dict[str, float]) -> logger.debug("Timeout not met.") return False +def check_force_study_timeout(task: TaskHasStudy, pending_series: Dict[str, float]) -> bool: + """ + Checks if the duration since the creation of the study exceeds the force study completion timeout + """ + logger.debug("Checking force study timeout") + study = task.study + creation_string = study.creation_time + logger.debug(f"Creation time: {creation_string}, now is: {datetime.now()}") + if not creation_string: + return False + + creation_time = datetime.strptime(creation_string, "%Y-%m-%d %H:%M:%S") + if datetime.now() > creation_time + timedelta(seconds=config.mercure.force_study_complete_trigger): + logger.debug("Force timeout met.") + return True + + logger.debug("Force timeout not met.") + return False + def check_study_series(task: TaskHasStudy, required_series: str) -> bool: """ diff --git a/webinterface/rules.py b/webinterface/rules.py index 512916c9..e7304d0d 100755 --- a/webinterface/rules.py +++ b/webinterface/rules.py @@ -177,6 +177,7 @@ async def rules_edit_post(request) -> Response: action_trigger=form.get("action_trigger", "series"), study_trigger_condition=form.get("study_trigger_condition", "timeout"), study_trigger_series=form.get("study_trigger_series", ""), + study_force_completion_action=form.get("study_force_completion_action", "discard"), priority=form.get("priority", "normal"), processing_module=processing_module, processing_settings=new_processing_settings, diff --git a/webinterface/templates/rules_edit.html b/webinterface/templates/rules_edit.html index 1d3e8c4c..7bcdd71e 100755 --- a/webinterface/templates/rules_edit.html +++ b/webinterface/templates/rules_edit.html @@ -195,6 +195,27 @@

Edit Rule - {{rule}}

+
+ +
+
+ +
+
+
@@ -692,6 +713,7 @@

Edit Rule - {{rule}}

if (action_trigger == 'series') { $('#study_trigger_field').hide(); + $("#force_study_completion_field").hide(); $('#study_trigger_series').prop('required', false); } else { $('#study_trigger_field').show(); @@ -704,9 +726,11 @@

Edit Rule - {{rule}}

if (study_trigger == 'timeout') { $('#study_trigger_series_section').hide(); $('#study_trigger_series').prop('required', false); + $("#force_study_completion_field").hide(); } else { $('#study_trigger_series_section').show(); $('#study_trigger_series').prop('required', true); + $("#force_study_completion_field").show(); } }); From eb3cba14eb148f7fb93db476fe80200e0dc74df9 Mon Sep 17 00:00:00 2001 From: Amanpreet Singh Saimbhi Date: Wed, 20 Nov 2024 23:26:19 +0000 Subject: [PATCH 2/7] minor UI fix --- webinterface/templates/rules_edit.html | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/webinterface/templates/rules_edit.html b/webinterface/templates/rules_edit.html index 7bcdd71e..d9d1b251 100755 --- a/webinterface/templates/rules_edit.html +++ b/webinterface/templates/rules_edit.html @@ -710,6 +710,7 @@

Edit Rule - {{rule}}

$('#action_trigger').change(function () { var action_trigger = $(this).children("option:selected").val(); + var study_trigger = $('#study_trigger_condition').children("option:selected").val(); if (action_trigger == 'series') { $('#study_trigger_field').hide(); @@ -717,6 +718,9 @@

Edit Rule - {{rule}}

$('#study_trigger_series').prop('required', false); } else { $('#study_trigger_field').show(); + if (study_trigger == 'received_series') { + $("#force_study_completion_field").show(); + } } }); From 825b4fbf534441e53ff29cd86472e26825ab07b5 Mon Sep 17 00:00:00 2001 From: Amanpreet Singh Saimbhi Date: Fri, 22 Nov 2024 21:35:08 +0000 Subject: [PATCH 3/7] add support for force completion actions --- routing/route_studies.py | 69 +++++++++++++++++++++++++++++----------- webinterface/rules.py | 2 +- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/routing/route_studies.py b/routing/route_studies.py index d18afb3a..9ecb2176 100755 --- a/routing/route_studies.py +++ b/routing/route_studies.py @@ -49,11 +49,13 @@ def route_studies(pending_series: Dict[str, float]) -> None: studies_ready = {} with os.scandir(config.mercure.studies_folder) as it: for entry in it: - if entry.is_dir() and not is_study_locked(entry.path) and is_study_complete(entry.path, pending_series): - modificationTime = entry.stat().st_mtime - studies_ready[entry.name] = modificationTime - if entry.is_dir() and not is_study_locked(entry.path) and check_force_study_timeout(entry.path, pending_series): - logger.info(f"Force completion timeout met for study {entry.name}") + if entry.is_dir() and not is_study_locked(entry.path): + if is_study_complete(entry.path, pending_series): + modificationTime = entry.stat().st_mtime + studies_ready[entry.name] = modificationTime + else: + if not check_force_study_timeout(Path(entry.path), pending_series): + logger.error(f"Error during checking force study timeout for study {entry.path}") logger.debug(f"Studies ready for processing: {studies_ready}") # Process all complete studies for dir_entry in sorted(studies_ready): @@ -174,24 +176,53 @@ def check_study_timeout(task: TaskHasStudy, pending_series: Dict[str, float]) -> logger.debug("Timeout not met.") return False -def check_force_study_timeout(task: TaskHasStudy, pending_series: Dict[str, float]) -> bool: + +def check_force_study_timeout(folder: Path, pending_series: Dict[str, float]) -> bool: """ Checks if the duration since the creation of the study exceeds the force study completion timeout """ - logger.debug("Checking force study timeout") - study = task.study - creation_string = study.creation_time - logger.debug(f"Creation time: {creation_string}, now is: {datetime.now()}") - if not creation_string: - return False + try: + logger.debug("Checking force study timeout") + + with open(folder / mercure_names.TASKFILE, "r") as json_file: + task: TaskHasStudy = TaskHasStudy(**json.load(json_file)) - creation_time = datetime.strptime(creation_string, "%Y-%m-%d %H:%M:%S") - if datetime.now() > creation_time + timedelta(seconds=config.mercure.force_study_complete_trigger): - logger.debug("Force timeout met.") + study = task.study + creation_string = study.creation_time + if not creation_string: + logger.error(f"Missing creation time in task file in study folder {folder}", task.id) # handle_error + return False + logger.debug(f"Creation time: {creation_string}, now is: {datetime.now()}") + + creation_time = datetime.strptime(creation_string, "%Y-%m-%d %H:%M:%S") + if datetime.now() > creation_time + timedelta(seconds=config.mercure.study_forcecomplete_trigger): + logger.info(f"Force timeout met for study {folder}") + if not study.complete_force_action or study.complete_force_action == "ignore": + return True + elif study.complete_force_action == "proceed": + logger.info(f"Forcing study completion for study {folder}") + (folder / mercure_names.FORCE_COMPLETE).touch() + elif study.complete_force_action == "discard": + logger.info(f"Moving folder to discard: {folder.name}") + lock_file = Path(folder / mercure_names.LOCK) + try: + lock = helper.FileLock(lock_file) + except: + logger.error(f"Unable to lock study for removal {lock_file}") # handle_error + return False + if not move_study_folder(task.id, folder.name, "DISCARD"): + logger.error(f"Error during moving study to discard folder {study}", task.id) # handle_error + return False + if not remove_study_folder(None, folder.name, lock): + logger.error(f"Unable to delete study folder {lock_file}") # handle_error + return False + else: + logger.debug("Force timeout not met.") return True - logger.debug("Force timeout not met.") - return False + except Exception: + logger.error(f"Could not check force study timeout for study {folder}") # handle_error + return False def check_study_series(task: TaskHasStudy, required_series: str) -> bool: @@ -338,7 +369,7 @@ def move_study_folder(task_id: Union[str, None], study: str, destination: str) - """ logger.debug(f"Move_study_folder {study} to {destination}") source_folder = config.mercure.studies_folder + "/" + study - destination_folder = config.mercure.discard_folder + destination_folder = None if destination == "PROCESSING": destination_folder = config.mercure.processing_folder elif destination == "SUCCESS": @@ -347,6 +378,8 @@ def move_study_folder(task_id: Union[str, None], study: str, destination: str) - destination_folder = config.mercure.error_folder elif destination == "OUTGOING": destination_folder = config.mercure.outgoing_folder + elif destination == "DISCARD": + destination_folder = config.mercure.discard_folder else: logger.error(f"Unknown destination {destination} requested for {study}", task_id) # handle_error return False diff --git a/webinterface/rules.py b/webinterface/rules.py index e7304d0d..96def5e5 100755 --- a/webinterface/rules.py +++ b/webinterface/rules.py @@ -177,7 +177,7 @@ async def rules_edit_post(request) -> Response: action_trigger=form.get("action_trigger", "series"), study_trigger_condition=form.get("study_trigger_condition", "timeout"), study_trigger_series=form.get("study_trigger_series", ""), - study_force_completion_action=form.get("study_force_completion_action", "discard"), + study_force_completion_action=form.get("study_force_completion_action", ""), priority=form.get("priority", "normal"), processing_module=processing_module, processing_settings=new_processing_settings, From 354e1b78c007a56ca4ce28ba041201d435af6d96 Mon Sep 17 00:00:00 2001 From: Amanpreet Singh Saimbhi Date: Tue, 26 Nov 2024 20:44:41 +0000 Subject: [PATCH 4/7] add test for force complete actions --- routing/route_studies.py | 4 +-- tests/test_studies.py | 59 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/routing/route_studies.py b/routing/route_studies.py index 9ecb2176..90468239 100755 --- a/routing/route_studies.py +++ b/routing/route_studies.py @@ -54,7 +54,7 @@ def route_studies(pending_series: Dict[str, float]) -> None: modificationTime = entry.stat().st_mtime studies_ready[entry.name] = modificationTime else: - if not check_force_study_timeout(Path(entry.path), pending_series): + if not check_force_study_timeout(Path(entry.path)): logger.error(f"Error during checking force study timeout for study {entry.path}") logger.debug(f"Studies ready for processing: {studies_ready}") # Process all complete studies @@ -177,7 +177,7 @@ def check_study_timeout(task: TaskHasStudy, pending_series: Dict[str, float]) -> return False -def check_force_study_timeout(folder: Path, pending_series: Dict[str, float]) -> bool: +def check_force_study_timeout(folder: Path) -> bool: """ Checks if the duration since the creation of the study exceeds the force study completion timeout """ diff --git a/tests/test_studies.py b/tests/test_studies.py index 63a34d7b..99f7386e 100644 --- a/tests/test_studies.py +++ b/tests/test_studies.py @@ -310,4 +310,61 @@ def test_route_study_series_trigger(fs: FakeFilesystem, mercure_config, mocked): # print(k) # assert ( outgoing / study_uid).exists() -# assert list((outgoing / study_uid).glob("**/*.dcm")) != [] \ No newline at end of file +# assert list((outgoing / study_uid).glob("**/*.dcm")) != [] + + +@pytest.mark.parametrize("force_complete_action", ["ignore", "proceed", "discard"]) +def test_route_study_force_complete(fs: FakeFilesystem, mercure_config, mocked, force_complete_action): + """ + Test that a study exceeding the force completion timeout is handled according to the action specified. + """ + config = mercure_config( + { + "series_complete_trigger": 10, + "study_complete_trigger": 30, + "study_forcecomplete_trigger": 60, + "rules": { + "route_study": Rule( + rule="True", + action="route", + study_trigger_condition="received_series", + study_trigger_series=" 'test_series_complete' and 'test_series_missing' ", + target="test_target_2", + action_trigger="study", + study_force_completion_action=force_complete_action, + ).dict(), + }, + } + ) + study_uid = str(uuid.uuid4()) + series_uid = str(uuid.uuid4()) + series_description = "test_series_complete" + out_path = Path(config.outgoing_folder) + discard_path = Path(config.discard_folder) + + with freeze_time("2020-01-01 00:00:00") as frozen_time: + # Create the initial series. + create_series(mocked, fs, config, study_uid, series_uid, series_description) + frozen_time.tick(delta=timedelta(seconds=11)) + # Run the router as the first series completes to generate a study task. + router.run_router() + frozen_time.tick(delta=timedelta(seconds=61)) + # Run router after force complete trigger. + try: + router.run_router() + except Exception as e: + # workaround for moving the study folder while using iterator. + assert str(e) == "dictionary changed size during iteration" + + if force_complete_action == "ignore": + # ensure that the study is not routed + assert list(out_path.glob("**/*")) == [] + assert list(discard_path.glob("**/*")) == [] + elif force_complete_action == "proceed": + # run router again to proceed with the force complete file. + router.run_router() + assert list(out_path.glob("**/*")) != [] + assert list(discard_path.glob("**/*")) == [] + elif force_complete_action == "discard": + assert list(discard_path.glob("**/*")) != [] + assert list(out_path.glob("**/*")) == [] \ No newline at end of file From bb7fbc230608133fed16aece9643e665ef3459e1 Mon Sep 17 00:00:00 2001 From: Roy Wiggins Date: Wed, 27 Nov 2024 23:12:57 +0000 Subject: [PATCH 5/7] pyfakefs fix --- routing/route_studies.py | 4 ++++ tests/test_studies.py | 6 +----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/routing/route_studies.py b/routing/route_studies.py index 90468239..0dcaa014 100755 --- a/routing/route_studies.py +++ b/routing/route_studies.py @@ -14,6 +14,8 @@ import daiquiri from datetime import datetime, timedelta +import pyfakefs + # App-specific includes import common.config as config import common.rule_evaluation as rule_evaluation @@ -48,6 +50,8 @@ def route_studies(pending_series: Dict[str, float]) -> None: # TODO: Handle studies that exceed the "force completion" timeout in the "CONDITION_RECEIVED_SERIES" mode studies_ready = {} with os.scandir(config.mercure.studies_folder) as it: + if isinstance(it,pyfakefs.fake_scandir.ScanDirIter): + it = list(it) # prevent pyfakefs issue for entry in it: if entry.is_dir() and not is_study_locked(entry.path): if is_study_complete(entry.path, pending_series): diff --git a/tests/test_studies.py b/tests/test_studies.py index 99f7386e..bc37c94c 100644 --- a/tests/test_studies.py +++ b/tests/test_studies.py @@ -350,11 +350,7 @@ def test_route_study_force_complete(fs: FakeFilesystem, mercure_config, mocked, router.run_router() frozen_time.tick(delta=timedelta(seconds=61)) # Run router after force complete trigger. - try: - router.run_router() - except Exception as e: - # workaround for moving the study folder while using iterator. - assert str(e) == "dictionary changed size during iteration" + router.run_router() if force_complete_action == "ignore": # ensure that the study is not routed From 92dda27a4cb65c566495df4d351b3f53cf16bfba Mon Sep 17 00:00:00 2001 From: Amanpreet Singh Saimbhi Date: Thu, 12 Dec 2024 17:37:04 +0000 Subject: [PATCH 6/7] satisfy mypy --- app/routing/route_studies.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/routing/route_studies.py b/app/routing/route_studies.py index 831d45db..92b7958e 100644 --- a/app/routing/route_studies.py +++ b/app/routing/route_studies.py @@ -50,7 +50,8 @@ def route_studies(pending_series: Dict[str, float]) -> None: studies_ready = {} with os.scandir(config.mercure.studies_folder) as it: if isinstance(it,pyfakefs.fake_scandir.ScanDirIter): - it = list(it) # prevent pyfakefs issue + # prevent pyfakefs issue + it = list(it) # type: ignore for entry in it: if entry.is_dir() and not is_study_locked(entry.path): if is_study_complete(entry.path, pending_series): From 046481e321e720a2a9f202c3f262f45082d3115f Mon Sep 17 00:00:00 2001 From: Amanpreet Singh Saimbhi Date: Thu, 12 Dec 2024 18:10:09 +0000 Subject: [PATCH 7/7] minor change for pytest --- app/routing/route_studies.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/routing/route_studies.py b/app/routing/route_studies.py index 92b7958e..b72ae120 100644 --- a/app/routing/route_studies.py +++ b/app/routing/route_studies.py @@ -49,9 +49,7 @@ def route_studies(pending_series: Dict[str, float]) -> None: # TODO: Handle studies that exceed the "force completion" timeout in the "CONDITION_RECEIVED_SERIES" mode studies_ready = {} with os.scandir(config.mercure.studies_folder) as it: - if isinstance(it,pyfakefs.fake_scandir.ScanDirIter): - # prevent pyfakefs issue - it = list(it) # type: ignore + it = list(it) # type: ignore for entry in it: if entry.is_dir() and not is_study_locked(entry.path): if is_study_complete(entry.path, pending_series):