Merge pull request #11 from Australian-Imaging-Service/develop

Minor bugfixes

xnat_ingest/cli/stage.py

@@ -127,6 +127,18 @@
         "mail and log level are delimited by ',' and separate destinations by ';'"
     ),
 )
+@click.option(
+    "--add-logger",
+    type=str,
+    multiple=True,
+    default=(),
+    envvar="XNAT_INGEST_UPLOAD_LOGGERS",
+    help=(
+        "The loggers to use for logging. By default just the 'xnat-ingest' logger is used. "
+        "But additional loggers can be included (e.g. 'xnat') can be "
+        "specified here"
+    ),
+)
 @click.option(
     "--mail-server",
     type=MailServer.cli_type,
@@ -173,6 +185,7 @@ def stage(
     log_level: str,
     log_files: ty.List[LogFile],
     log_emails: ty.List[LogEmail],
+    add_logger: ty.List[str],
     mail_server: MailServer,
     raise_errors: bool,
     deidentify: bool,
@@ -183,6 +196,7 @@ def stage(
         log_emails=log_emails,
         log_files=log_files,
         mail_server=mail_server,
+        add_logger=add_logger,
     )
 
     if xnat_login:

xnat_ingest/cli/upload.py

@@ -15,6 +15,7 @@
 from fileformats.generic import File
 from arcana.core.data.set import Dataset
 from arcana.xnat import Xnat
+from xnat.exceptions import XNATResponseError
 from xnat_ingest.cli.base import cli
 from xnat_ingest.session import ImagingSession
 from xnat_ingest.utils import (
@@ -48,12 +49,6 @@
 @click.argument("server", type=str, envvar="XNAT_INGEST_UPLOAD_HOST")
 @click.argument("user", type=str, envvar="XNAT_INGEST_UPLOAD_USER")
 @click.option("--password", default=None, type=str, envvar="XNAT_INGEST_UPLOAD_PASS")
-@click.option(
-    "--delete/--dont-delete",
-    default=True,
-    envvar="XNAT_INGEST_UPLOAD_DELETE",
-    help="Whether to delete the session directories after they have been uploaded or not",
-)
 @click.option(
     "--log-level",
     default="info",
@@ -88,6 +83,18 @@
         "mail and log level are delimited by ',' and separate destinations by ';'"
     ),
 )
+@click.option(
+    "--add-logger",
+    type=str,
+    multiple=True,
+    default=(),
+    envvar="XNAT_INGEST_UPLOAD_LOGGERS",
+    help=(
+        "The loggers to use for logging. By default just the 'xnat-ingest' logger is used. "
+        "But additional loggers can be included (e.g. 'xnat') can be "
+        "specified here"
+    ),
+)
 @click.option(
     "--mail-server",
     type=MailServer.cli_type,
@@ -157,7 +164,7 @@
 @click.option(
     "--verify-ssl/--dont-verify-ssl",
     type=bool,
-    default=False,
+    default=True,
     envvar="XNAT_INGEST_UPLOAD_VERIFY_SSL",
     help="Whether to verify the SSL certificate of the XNAT server",
 )
@@ -166,12 +173,12 @@ def upload(
     server: str,
     user: str,
     password: str,
-    delete: bool,
     log_level: str,
     log_files: ty.List[LogFile],
     log_emails: ty.List[LogEmail],
     mail_server: MailServer,
     always_include: ty.Sequence[str],
+    add_logger: ty.List[str],
     raise_errors: bool,
     store_credentials: ty.Tuple[str, str],
     temp_dir: ty.Optional[Path],
@@ -185,6 +192,7 @@ def upload(
         log_emails=log_emails,
         log_files=log_files,
         mail_server=mail_server,
+        add_logger=add_logger,
     )
     if temp_dir:
         tempfile.tempdir = str(temp_dir)
@@ -449,25 +457,29 @@ def iter_staged_sessions():
                 logger.info(f"Successfully uploaded all files in '{session.name}'")
                 # Extract DICOM metadata
                 logger.info("Extracting metadata from DICOMs on XNAT..")
-                xnat_repo.connection.put(
-                    f"/data/experiments/{xsession.id}?pullDataFromHeaders=true"
-                )
-                xnat_repo.connection.put(
-                    f"/data/experiments/{xsession.id}?fixScanTypes=true"
-                )
-                xnat_repo.connection.put(
-                    f"/data/experiments/{xsession.id}?triggerPipelines=true"
-                )
-                msg = f"Succesfully uploaded all files in '{session.name}'"
-                if delete:
-                    msg += ", deleting originals..."
-                logger.info(msg)
-                if delete:
-                    shutil.rmtree(session_staging_dir)
-                    logger.info(
-                        f"Deleted staging dir '{str(session_staging_dir)}' session data "
-                        "after successful upload"
+                try:
+                    xnat_repo.connection.put(
+                        f"/data/experiments/{xsession.id}?pullDataFromHeaders=true"
+                    )
+                except XNATResponseError as e:
+                    logger.warning(
+                        f"Failed to extract metadata from DICOMs in '{session.name}': {e}"
+                    )
+                try:
+                    xnat_repo.connection.put(
+                        f"/data/experiments/{xsession.id}?fixScanTypes=true"
+                    )
+                except XNATResponseError as e:
+                    logger.warning(f"Failed to fix scan types in '{session.name}': {e}")
+                try:
+                    xnat_repo.connection.put(
+                        f"/data/experiments/{xsession.id}?triggerPipelines=true"
+                    )
+                except XNATResponseError as e:
+                    logger.warning(
+                        f"Failed to trigger pipelines in '{session.name}': {e}"
                     )
+                logger.info(f"Succesfully uploaded all files in '{session.name}'")
             except Exception as e:
                 if not raise_errors:
                     logger.error(

xnat_ingest/tests/test_cli.py

@@ -196,9 +196,11 @@ def test_stage_and_upload(
             str(associated_files_dir)
             + "/{PatientName.given_name}_{PatientName.family_name}*.ptd",
             r".*/[^\.]+.[^\.]+.[^\.]+.(?P<id>\d+)\.[A-Z]+_(?P<resource>[^\.]+).*",
-            # "--log-file",
-            # str(log_file),
-            # "info",
+            "--log-file",
+            str(log_file),
+            "info",
+            "--add-logger",
+            "xnat",
             "--raise-errors",
             "--delete",
             "--xnat-login",
@@ -214,9 +216,11 @@ def test_stage_and_upload(
         upload,
         [
             str(staging_dir),
-            # "--log-file",
-            # str(log_file),
-            # "info",
+            "--log-file",
+            str(log_file),
+            "info",
+            "--add-logger",
+            "xnat",
             "--always-include",
             "medimage/dicom-series",
             "--raise-errors",

xnat_ingest/utils.py

@@ -134,14 +134,22 @@ def set_logger_handling(
     log_emails: ty.List[LogEmail] | None,
     log_files: ty.List[LogFile] | None,
     mail_server: MailServer,
+    add_logger: ty.Sequence[str] = (),
 ):
+    loggers = [logger]
+    for log in add_logger:
+        loggers.append(logging.getLogger(log))
 
     levels = [log_level]
-    levels.extend(le.loglevel for le in log_emails)
-    levels.extend(lf.loglevel for lf in log_files)
+    if log_emails:
+        levels.extend(le.loglevel for le in log_emails)
+    if log_files:
+        levels.extend(lf.loglevel for lf in log_files)
 
     min_log_level = min(getattr(logging, ll.upper()) for ll in levels)
-    logger.setLevel(min_log_level)
+
+    for logr in loggers:
+        logr.setLevel(min_log_level)
 
     # Configure the email logger
     if log_emails:
@@ -161,7 +169,8 @@ def set_logger_handling(
                 secure=None,
             )
             smtp_hdle.setLevel(getattr(logging, log_email.loglevel.upper()))
-            logger.addHandler(smtp_hdle)
+            for logr in loggers:
+                logr.addHandler(smtp_hdle)
 
     # Configure the file logger
     for log_file in log_files:
@@ -172,14 +181,16 @@ def set_logger_handling(
         log_file_hdle.setFormatter(
             logging.Formatter("%(asctime)s - %(levelname)s - %(message)s")
         )
-        logger.addHandler(log_file_hdle)
+        for logr in loggers:
+            logr.addHandler(log_file_hdle)
 
     console_hdle = logging.StreamHandler(sys.stdout)
     console_hdle.setLevel(getattr(logging, log_level.upper()))
     console_hdle.setFormatter(
         logging.Formatter("%(asctime)s - %(levelname)s - %(message)s")
     )
-    logger.addHandler(console_hdle)
+    for logr in loggers:
+        logr.addHandler(console_hdle)
 
 
 def get_checksums(xresource) -> ty.Dict[str, str]: