Merge pull request #59 from radexperts/multiprocess-worker

Run processor inside worker in separate process

TODO.md

@@ -4,9 +4,9 @@
 
 - Before new release
   -- test job_utils
-  -- exclude autoreload when tests are saved (Custom Filter in server command watched files)
   -- Test canceled task/job in test_workers.py
 
+- Replace 7z with .zip format. Really save in patient folder?
 - Figure out if favicon works in all browsers
 - Rename C_STORE to C-STORE and so on in dimse connector
 - Replace AssertionError with assert
@@ -16,14 +16,11 @@
   -- Check aborted attribute every 10 seconds or so and the may be kill the process
   -- Set task to FAILURE with message "Task manually / forcefully aborted"
 - Make single task cancelable, retriable, resumeable, ...
-- Upgrade REDIS server on RADIS
 - Unfix pyright and its VS code extension
-- Replace sherlock on RADIS
 - Use DicomLogEntry during C-STORE
 - Allow to restart or cancel specific dicom task
 - Fix dicom explorer search over Accession Number
 - Make warning when only one image fails
-- Upgrade psycopg on RADIS
 - Use django-stubs instead of django-types (also on RADIS)
 - Exclude SR and PR when in pseudonymization mode
 - Cancel processing tasks actively
@@ -112,6 +109,7 @@
 
 ## Maybe
 
+- exclude test folders from autorelad in ServerCommand (maybe a custom filter is needed)
 - Switch from Daphne to Uvicorn (maybe it has faster restart times during development)
 - Switch from Celery to Huey
 - Upgrade postgres server to v15, but we have to migrate the data then as the database files are incompatible a newer version
@@ -189,3 +187,9 @@
 - Get rid of jQuery in ADIT and RADIS
 - Get rid of Jumbotron
 - Get rid of those not used accounts views and login form
+- Move over to SVG sprites
+- Get rid of RabbitMQ
+- Port over ServerCommand
+- Upgrade REDIS server on RADIS
+- Remove sherlock on RADIS as no need for distributed lock there and we use Redis here for that
+- Upgrade psycopg on RADIS

adit/batch_query/forms.py

@@ -1,3 +1,5 @@
+from typing import cast
+
 from crispy_forms.helper import FormHelper
 from crispy_forms.layout import Layout, Submit
 from django import forms
@@ -6,9 +8,11 @@
 from django.core.files import File
 from django.db import transaction
 from django.utils.safestring import mark_safe
+from django.utils.translation import gettext_lazy as _
 
 from adit.core.errors import BatchFileContentError, BatchFileFormatError, BatchFileSizeError
 from adit.core.fields import DicomNodeChoiceField, RestrictedFileField
+from adit.core.models import DicomNode
 
 from .models import BatchQueryJob, BatchQueryTask
 from .parsers import BatchQueryFileParser
@@ -47,19 +51,19 @@ def __init__(self, *args, **kwargs):
         self.tasks = []
         self.save_tasks = None
 
-        user = kwargs.pop("user")
+        self.user = kwargs.pop("user")
 
         super().__init__(*args, **kwargs)
 
-        self.fields["source"] = DicomNodeChoiceField("source", user)
+        self.fields["source"] = DicomNodeChoiceField("source", self.user)
         self.fields["source"].widget.attrs["@change"] = "onSourceChange($event)"
 
         self.fields["urgent"].widget.attrs["@change"] = "onUrgentChange($event)"
 
-        if not user.has_perm("batch_query.can_process_urgently"):
+        if not self.user.has_perm("batch_query.can_process_urgently"):
             del self.fields["urgent"]
 
-        self.max_batch_size = settings.MAX_BATCH_QUERY_SIZE if not user.is_staff else None
+        self.max_batch_size = settings.MAX_BATCH_QUERY_SIZE if not self.user.is_staff else None
 
         if self.max_batch_size is not None:
             self.fields[
@@ -76,6 +80,12 @@ def __init__(self, *args, **kwargs):
         self.helper.attrs["x-data"] = "batchQueryJobForm()"
         self.helper.add_input(Submit("save", "Create Job"))
 
+    def clean_source(self):
+        source = cast(DicomNode, self.cleaned_data["source"])
+        if not source.is_accessible_by_user(self.user, "source"):
+            raise ValidationError(_("You do not have access to this source."))
+        return source
+
     def clean_batch_file(self):
         batch_file: File = self.cleaned_data["batch_file"]
         parser = BatchQueryFileParser()

adit/batch_query/utils/query_utils.py

@@ -23,12 +23,9 @@ class QueryExecutor:
     def __init__(self, query_task: BatchQueryTask) -> None:
         self.query_task = query_task
 
-        # Make sure source is accessible by the user
-        source_node = DicomNode.objects.accessible_by_user(self.query_task.job.owner, "source").get(
-            pk=self.query_task.source.pk
-        )
-        assert source_node.node_type == DicomNode.NodeType.SERVER
-        self.operator = DicomOperator(source_node.dicomserver)
+        source = self.query_task.source
+        assert source.node_type == DicomNode.NodeType.SERVER
+        self.operator = DicomOperator(source.dicomserver)
 
     def start(self) -> tuple[BatchQueryTask.Status, str, list[DicomLogEntry]]:
         patient = self._fetch_patient()
@@ -184,7 +181,7 @@ def _query_studies(self, patient_id: str) -> list[BatchQueryResult]:
                 study_date=study.StudyDate,
                 study_time=study.StudyTime,
                 study_description=study.StudyDescription,
-                modalities=study.ModalitiesInStudy,  # All modalities in this study
+                modalities=study.ModalitiesInStudy,  # type: ignore TODO: pyright issue #6456
                 image_count=study.NumberOfStudyRelatedInstances,
                 pseudonym=self.query_task.pseudonym,
                 series_uid="",
@@ -213,7 +210,7 @@ def _query_series(self, patient_id: str) -> list[BatchQueryResult]:
                     study_date=study.StudyDate,
                     study_time=study.StudyTime,
                     study_description=study.StudyDescription,
-                    modalities=[series.Modality],  # The modality of this series
+                    modalities=[series.Modality],
                     image_count=study.NumberOfStudyRelatedInstances,
                     pseudonym=self.query_task.pseudonym,
                     series_uid=series.SeriesInstanceUID,

adit/batch_transfer/forms.py

@@ -1,14 +1,18 @@
+from typing import cast
+
 from crispy_forms.helper import FormHelper
 from crispy_forms.layout import Layout, Submit
 from django import forms
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.db import transaction
 from django.utils.safestring import mark_safe
+from django.utils.translation import gettext_lazy as _
 
 from adit.accounts.models import User
 from adit.core.errors import BatchFileContentError, BatchFileFormatError, BatchFileSizeError
 from adit.core.fields import DicomNodeChoiceField, RestrictedFileField
+from adit.core.models import DicomNode
 
 from .models import BatchTransferJob, BatchTransferTask
 from .parsers import BatchTransferFileParser
@@ -103,6 +107,18 @@ def __init__(self, *args, **kwargs):
         self.helper.attrs["x-data"] = "batchTransferJobForm()"
         self.helper.add_input(Submit("save", "Create Job"))
 
+    def clean_source(self):
+        source = cast(DicomNode, self.cleaned_data["source"])
+        if not source.is_accessible_by_user(self.user, "source"):
+            raise ValidationError(_("You do not have access to this source."))
+        return source
+
+    def clean_destination(self):
+        destination = cast(DicomNode, self.cleaned_data["destination"])
+        if not destination.is_accessible_by_user(self.user, "destination"):
+            raise ValidationError(_("You do not have access to this destination."))
+        return destination
+
     def clean_batch_file(self):
         batch_file = self.cleaned_data["batch_file"]
         can_transfer_unpseudonymized = self.user.has_perm(

adit/core/models.py

@@ -110,6 +110,11 @@ def __init__(self, *args, **kwargs):
                 raise AssertionError(f"Invalid node type: {self.NODE_TYPE}")
             self.node_type = self.NODE_TYPE
 
+    def is_accessible_by_user(
+        self, user: User, access_type: Literal["source", "destination"]
+    ) -> bool:
+        return DicomNode.objects.accessible_by_user(user, access_type).filter(id=self.id).exists()
+
 
 class DicomNodeInstituteAccess(models.Model):
     id: int