From c7fb3d1d7a89e45c2a3510260a01aabcea95e3ff Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 11:35:27 +0000
Subject: [PATCH 01/60] Update `pixl_ehr` README, not actually using cogstack
 Docker service

---
 pixl_ehr/README.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pixl_ehr/README.md b/pixl_ehr/README.md
index 73fff42c7..4aea406e3 100644
--- a/pixl_ehr/README.md
+++ b/pixl_ehr/README.md
@@ -43,7 +43,8 @@ and the processing tests with
 ```
 
 To test the availability of a CogStack instance, we mock up a *FastAPI* server which simply takes in
-some input text and returns the same text. The configuration of this mock instance is defined in [`test/dummy-services`](/test/dummy-services/).
+some input text and returns the same text. The mocking is handled by a *pytest* fixture in
+`test_processing.py()` (`_mock_requests`).
 
 ## Usage
 

From 8faea3c0a9a8047054d197a82219ebc6b4fd7798 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 11:35:50 +0000
Subject: [PATCH 02/60] Move cogstack dummy service to subdirectory

---
 test/docker-compose.yml                           | 4 +---
 test/dummy-services/{ => cogstack}/Dockerfile     | 0
 test/dummy-services/{ => cogstack}/cogstack.py    | 0
 test/dummy-services/{ => cogstack}/pyproject.toml | 0
 4 files changed, 1 insertion(+), 3 deletions(-)
 rename test/dummy-services/{ => cogstack}/Dockerfile (100%)
 rename test/dummy-services/{ => cogstack}/cogstack.py (100%)
 rename test/dummy-services/{ => cogstack}/pyproject.toml (100%)

diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index 50cc47f24..faf3a3380 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -67,7 +67,7 @@ services:
   cogstack-api:
     container_name: system-test-cogstack
     build:
-      context: ./dummy-services/
+      context: ./dummy-services/cogstack/
     healthcheck:
       test: [ "CMD", "curl", "-f", "http://localhost:8000/heart-beat" ]
       interval: 10s
@@ -75,5 +75,3 @@ services:
       retries: 5
     networks:
       pixl-net:
-
-
diff --git a/test/dummy-services/Dockerfile b/test/dummy-services/cogstack/Dockerfile
similarity index 100%
rename from test/dummy-services/Dockerfile
rename to test/dummy-services/cogstack/Dockerfile
diff --git a/test/dummy-services/cogstack.py b/test/dummy-services/cogstack/cogstack.py
similarity index 100%
rename from test/dummy-services/cogstack.py
rename to test/dummy-services/cogstack/cogstack.py
diff --git a/test/dummy-services/pyproject.toml b/test/dummy-services/cogstack/pyproject.toml
similarity index 100%
rename from test/dummy-services/pyproject.toml
rename to test/dummy-services/cogstack/pyproject.toml

From c936aacec4e47a0ea53a29f7437991feb90d1f74 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 11:54:49 +0000
Subject: [PATCH 03/60] Move ftp-server to `test/dummy-services`

---
 pixl_core/tests/conftest.py                               | 7 +++++--
 pixl_core/tests/docker-compose.yml                        | 8 ++++----
 .../tests => test/dummy-services}/ftp-server/Dockerfile   | 0
 .../dummy-services}/ftp-server/mounts/data/.gitkeep       | 0
 .../dummy-services}/ftp-server/mounts/ssl/localhost.crt   | 0
 .../dummy-services}/ftp-server/mounts/ssl/localhost.key   | 0
 6 files changed, 9 insertions(+), 6 deletions(-)
 rename {pixl_core/tests => test/dummy-services}/ftp-server/Dockerfile (100%)
 rename {pixl_core/tests => test/dummy-services}/ftp-server/mounts/data/.gitkeep (100%)
 rename {pixl_core/tests => test/dummy-services}/ftp-server/mounts/ssl/localhost.crt (100%)
 rename {pixl_core/tests => test/dummy-services}/ftp-server/mounts/ssl/localhost.key (100%)

diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py
index 76465c3c7..4741b6233 100644
--- a/pixl_core/tests/conftest.py
+++ b/pixl_core/tests/conftest.py
@@ -35,6 +35,7 @@
 os.environ["FTP_PORT"] = "20021"
 
 TEST_DIR = Path(__file__).parent
+DUMMY_SERVICES_DIR = Path(__file__).parents[2] / "test" / "dummy-services"
 STUDY_DATE = datetime.date.fromisoformat("2023-01-01")
 
 
@@ -71,9 +72,11 @@ def mounted_data() -> Path:
     The mounted data directory for the ftp server.
     This will contain the data after successful upload.
     """
-    yield TEST_DIR / "ftp-server" / "mounts" / "data"
+    yield DUMMY_SERVICES_DIR / "ftp-server" / "mounts" / "data"
     sub_dirs = [
-        f.path for f in os.scandir(TEST_DIR / "ftp-server" / "mounts" / "data") if f.is_dir()
+        f.path
+        for f in os.scandir(DUMMY_SERVICES_DIR / "ftp-server" / "mounts" / "data")
+        if f.is_dir()
     ]
     # Tear down the directory after tests
     for sub_dir in sub_dirs:
diff --git a/pixl_core/tests/docker-compose.yml b/pixl_core/tests/docker-compose.yml
index 0e577a5bd..096992cfd 100644
--- a/pixl_core/tests/docker-compose.yml
+++ b/pixl_core/tests/docker-compose.yml
@@ -32,16 +32,16 @@ services:
       retries: 5
   ftp-server:
     container_name: test-ftp-server
-    build: 
-      context: ftp-server
+    build:
+      context: ../../test/dummy-services/ftp-server
     ports:
       - "20021:21"
       - "21000-21010:21000-21010"
     volumes:
       # Mount for uploaded data
-      - "./ftp-server/mounts/data/:/home/pixl/"
+      - "../../test/dummy-services/ftp-server/mounts/data/:/home/pixl/"
       # Mount SSL keys for TLS
-      - "./ftp-server/mounts/ssl/:/etc/ssl/private/"
+      - "../../test/dummy-services/ftp-server/mounts/ssl/:/etc/ssl/private/"
     environment:
       ADDRESS: "localhost"
       USERS: pixl|longpassword|/home/pixl
diff --git a/pixl_core/tests/ftp-server/Dockerfile b/test/dummy-services/ftp-server/Dockerfile
similarity index 100%
rename from pixl_core/tests/ftp-server/Dockerfile
rename to test/dummy-services/ftp-server/Dockerfile
diff --git a/pixl_core/tests/ftp-server/mounts/data/.gitkeep b/test/dummy-services/ftp-server/mounts/data/.gitkeep
similarity index 100%
rename from pixl_core/tests/ftp-server/mounts/data/.gitkeep
rename to test/dummy-services/ftp-server/mounts/data/.gitkeep
diff --git a/pixl_core/tests/ftp-server/mounts/ssl/localhost.crt b/test/dummy-services/ftp-server/mounts/ssl/localhost.crt
similarity index 100%
rename from pixl_core/tests/ftp-server/mounts/ssl/localhost.crt
rename to test/dummy-services/ftp-server/mounts/ssl/localhost.crt
diff --git a/pixl_core/tests/ftp-server/mounts/ssl/localhost.key b/test/dummy-services/ftp-server/mounts/ssl/localhost.key
similarity index 100%
rename from pixl_core/tests/ftp-server/mounts/ssl/localhost.key
rename to test/dummy-services/ftp-server/mounts/ssl/localhost.key

From afa866db98a8875efcbf7525d1aabda88e1a11b3 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 11:56:38 +0000
Subject: [PATCH 04/60] Add ftp-server to system-test docker compose

---
 test/docker-compose.yml | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index faf3a3380..aeed12ee9 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -75,3 +75,26 @@ services:
       retries: 5
     networks:
       pixl-net:
+
+  ftp-server:
+    container_name: test-ftp-server
+    build:
+      context: ./dummy-services/ftp-server
+    ports:
+      - "20021:21"
+      - "21000-21010:21000-21010"
+    volumes:
+      # Mount for uploaded data
+      - "./dummy-services/ftp-server/mounts/data/:/home/pixl/"
+      # Mount SSL keys for TLS
+      - "./dummy-services/ftp-server/mounts/ssl/:/etc/ssl/private/"
+    environment:
+      ADDRESS: "localhost"
+      USERS: pixl|longpassword|/home/pixl
+      TLS_KEY: /etc/ssl/private/localhost.key
+      TLS_CERT: /etc/ssl/private/localhost.crt
+    healthcheck:
+      test: ping localhost:21 -w 2
+      interval: 10s
+      timeout: 5s
+      retries: 5

From ca639b42740ce0e52210a106004ff3627f6d07e7 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 12:14:11 +0000
Subject: [PATCH 05/60] Add `SendViaFTPS` to `OnChange` callback for
 `orthanc-anon`

---
 .env.sample                         |  6 +++---
 docker-compose.yml                  |  2 +-
 orthanc/orthanc-anon/plugin/pixl.py | 18 +++++++++++-------
 test/.env.test                      |  2 +-
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/.env.sample b/.env.sample
index 19d922e4d..325ce1fe1 100644
--- a/.env.sample
+++ b/.env.sample
@@ -43,16 +43,16 @@ ORTHANC_RAW_MAXIMUM_STORAGE_SIZE= // MB
 
 # PIXL Orthanc anon instance
 ORTHANC_ANON_USERNAME=
-ORTHANC_ANON_PASSWORD=     
+ORTHANC_ANON_PASSWORD=
 ORTHANC_ANON_AE_TITLE=
 ORTHANC_ANON_HTTP_TIMEOUT=60
-ORTHANC_AUTOROUTE_ANON_TO_AZURE=false
+ORTHANC_AUTOROUTE_ANON_TO_FTPS=true
 ENABLE_DICOM_WEB=true
 STUDY_TIME_OFFSET=
 SALT_VALUE=PIXL
 
 
-# UCVNAQR DICOM node information  
+# UCVNAQR DICOM node information
 VNAQR_AE_TITLE=
 VNAQR_DICOM_PORT=
 VNAQR_IP_ADDR=
diff --git a/docker-compose.yml b/docker-compose.yml
index b860e6e55..dbe1715bf 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -115,7 +115,7 @@ services:
       ORTHANC_USERNAME: ${ORTHANC_ANON_USERNAME}
       ORTHANC_PASSWORD: ${ORTHANC_ANON_PASSWORD}
       ORTHANC_ANON_AE_TITLE: ${ORTHANC_ANON_AE_TITLE}
-      ORTHANC_AUTOROUTE_ANON_TO_AZURE: ${ORTHANC_AUTOROUTE_ANON_TO_AZURE}
+      ORTHANC_AUTOROUTE_ANON_TO_FTPS: ${ORTHANC_AUTOROUTE_ANON_TO_FTPS}
       ORTHANC_RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE}
       ORTHANC_RAW_DICOM_PORT: "4242"
       ORTHANC_RAW_HOSTNAME: "orthanc-raw"
diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 4244bea22..becc5024f 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -28,6 +28,7 @@
 from io import BytesIO
 from pathlib import Path
 from time import sleep
+from typing import TYPE_CHECKING
 
 import requests
 import yaml
@@ -38,6 +39,9 @@
 import orthanc
 import pixl_dcmd
 
+if TYPE_CHECKING:
+    from typing import Any
+
 ORTHANC_USERNAME = config("ORTHANC_USERNAME")
 ORTHANC_PASSWORD = config("ORTHANC_PASSWORD")
 ORTHANC_URL = "http://localhost:8042"
@@ -180,17 +184,17 @@ def SendViaFTPS(resourceId: str) -> None:
 
 def ShouldAutoRoute():
     """
-    Checks whether ORTHANC_AUTOROUTE_ANON_TO_AZURE environment variable is
+    Checks whether ORTHANC_AUTOROUTE_ANON_TO_FTPS environment variable is
     set to true or false
     """
-    return os.environ.get("ORTHANC_AUTOROUTE_ANON_TO_AZURE", "false").lower() == "true"
+    return os.environ.get("ORTHANC_AUTOROUTE_ANON_TO_FTPS", "false").lower() == "true"
 
 
-def OnChange(changeType, level, resource) -> None:  # noqa: ARG001
+def OnChange(changeType: str, _level: None, resource: str) -> Any:
     """
     Three ChangeTypes included in this function:
     - If a study if stable and if ShouldAutoRoute returns true
-    then SendViaStow is called
+    then SendViaFTPS is called
     - If orthanc has started then message added to Orthanc LogWarning
     and AzureDICOMTokenRefresh called
     - If orthanc has stopped and TIMER is not none then message added
@@ -202,7 +206,7 @@ def OnChange(changeType, level, resource) -> None:  # noqa: ARG001
 
     if changeType == orthanc.ChangeType.STABLE_STUDY and ShouldAutoRoute():
         print("Stable study: %s" % resource)  # noqa: T201
-        SendViaStow(resource)
+        SendViaFTPS(resource)
 
     if changeType == orthanc.ChangeType.ORTHANC_STARTED:
         orthanc.LogWarning("Starting the scheduler")
@@ -213,13 +217,13 @@ def OnChange(changeType, level, resource) -> None:  # noqa: ARG001
             TIMER.cancel()
 
 
-def OnHeartBeat(output, uri, **request):  # noqa: ARG001
+def OnHeartBeat(output, uri, **request) -> Any:  # noqa: ARG001
     """Extends the REST API by registering a new route in the REST API"""
     orthanc.LogWarning("OK")
     output.AnswerBuffer("OK\n", "text/plain")
 
 
-def ReceivedInstanceCallback(receivedDicom, origin):
+def ReceivedInstanceCallback(receivedDicom: BytesIO, origin: str) -> Any:
     """Modifies a DICOM instance received by Orthanc and applies anonymisation."""
     if origin == orthanc.InstanceOrigin.REST_API:
         orthanc.LogWarning("DICOM instance received from the REST API")
diff --git a/test/.env.test b/test/.env.test
index 4d4181bfe..17b7aa7c3 100644
--- a/test/.env.test
+++ b/test/.env.test
@@ -43,7 +43,7 @@ ORTHANC_ANON_PASSWORD=orthanc_anon_password
 ORTHANC_ANON_AE_TITLE=ORTHANCANON
 ORTHANC_ANON_HTTP_TIMEOUT=60
 ENABLE_DICOM_WEB=true
-ORTHANC_AUTOROUTE_ANON_TO_AZURE=false
+ORTHANC_AUTOROUTE_ANON_TO_FTPS=true
 PIXL_DICOM_TRANSFER_TIMEOUT=240
 
 # UCVNAQR DICOM node information

From 69eeee1e2ee8805ebd48eda912603bdaf0b481a7 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 12:36:01 +0000
Subject: [PATCH 06/60] Check FTPS upload in system test

---
 test/run-system-test.sh           |  2 +-
 test/scripts/check_ftps_upload.py | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100755 test/scripts/check_ftps_upload.py

diff --git a/test/run-system-test.sh b/test/run-system-test.sh
index 2817acd54..39faf12ff 100755
--- a/test/run-system-test.sh
+++ b/test/run-system-test.sh
@@ -34,6 +34,7 @@ sleep 65  # need to wait until the DICOM image is "stable" = 60s
 ./scripts/check_entry_in_pixl_anon.sh
 ./scripts/check_entry_in_orthanc_anon.sh
 ./scripts/check_max_storage_in_orthanc_raw.sh
+./scripts/check_ftps_upload.py
 
 pixl extract-radiology-reports "${PACKAGE_DIR}/test/resources/omop"
 
@@ -45,4 +46,3 @@ docker exec system-test-ehr-api-1 rm -r /run/exports/test-extract-uclh-omop-cdm/
 
 cd "${PACKAGE_DIR}"
 docker compose -f docker-compose.yml -f test/docker-compose.yml -p system-test down --volumes
-
diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
new file mode 100755
index 000000000..80ab7acb6
--- /dev/null
+++ b/test/scripts/check_ftps_upload.py
@@ -0,0 +1,31 @@
+#!/usr/bin/env python
+#  Copyright (c) University College London Hospitals NHS Foundation Trust
+#
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+import logging
+from pathlib import Path
+from core import config_from_log_file
+
+logger = logging.getLogger(__name__)
+
+PARQUET_PATH = Path(__file__).parents[1] / "resources" / "omop"
+MOUNTED_DATA_DIR = Path(__file__).parents[1] / "dummy-services" / "ftp-server" / "mounts" / "data"
+
+project_name, omop_es_timestamp = config_from_log_file(PARQUET_PATH)
+expected_output_dir = MOUNTED_DATA_DIR / project_name
+
+# Test whether DICOM images have been uploaded
+glob_list = [expected_output_dir.glob("*.zip")]
+assert len(glob_list) == 1
+
+# TODO: check parquet files upload

From cafff1ab1fc8bb1e2362e8b4ea13f2933cb5f172 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 12:57:13 +0000
Subject: [PATCH 07/60] Fix `check_ftps_upload` and log expected export files

---
 test/scripts/check_ftps_upload.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 80ab7acb6..5c570f53c 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -14,7 +14,7 @@
 #  limitations under the License.
 import logging
 from pathlib import Path
-from core import config_from_log_file
+from pixl_cli._io import config_from_log_file
 
 logger = logging.getLogger(__name__)
 
@@ -26,6 +26,8 @@
 
 # Test whether DICOM images have been uploaded
 glob_list = [expected_output_dir.glob("*.zip")]
+logger.info(f"glob_list: {glob_list}")
+
 assert len(glob_list) == 1
 
 # TODO: check parquet files upload

From 6da329617bcf0ba99a5989d12e39e3bf616be67c Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 15:32:32 +0000
Subject: [PATCH 08/60] Trigger system test on CI


From 843c04b5a0534fa797a701b9f0f6a311a6e07f4d Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 17:56:33 +0000
Subject: [PATCH 09/60] Typo fix

Co-authored-by: Peter Tsrunchev <peter.tsrunchev@gmail.com>
---
 orthanc/orthanc-anon/plugin/pixl.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index becc5024f..7d607f049 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -193,7 +193,7 @@ def ShouldAutoRoute():
 def OnChange(changeType: str, _level: None, resource: str) -> Any:
     """
     Three ChangeTypes included in this function:
-    - If a study if stable and if ShouldAutoRoute returns true
+    - If a study is stable and if ShouldAutoRoute returns true
     then SendViaFTPS is called
     - If orthanc has started then message added to Orthanc LogWarning
     and AzureDICOMTokenRefresh called

From a9be50b78a53c39027f2520a864a8527e9566f25 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 17:50:42 +0000
Subject: [PATCH 10/60] Rename endpoint uploading env variable and set default
 to `false`

---
 .env.sample                         | 2 +-
 docker-compose.yml                  | 2 +-
 orthanc/orthanc-anon/plugin/pixl.py | 4 ++--
 test/.env.test                      | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/.env.sample b/.env.sample
index 325ce1fe1..8bf322311 100644
--- a/.env.sample
+++ b/.env.sample
@@ -46,7 +46,7 @@ ORTHANC_ANON_USERNAME=
 ORTHANC_ANON_PASSWORD=
 ORTHANC_ANON_AE_TITLE=
 ORTHANC_ANON_HTTP_TIMEOUT=60
-ORTHANC_AUTOROUTE_ANON_TO_FTPS=true
+ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT=false
 ENABLE_DICOM_WEB=true
 STUDY_TIME_OFFSET=
 SALT_VALUE=PIXL
diff --git a/docker-compose.yml b/docker-compose.yml
index dbe1715bf..6c332321f 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -115,7 +115,7 @@ services:
       ORTHANC_USERNAME: ${ORTHANC_ANON_USERNAME}
       ORTHANC_PASSWORD: ${ORTHANC_ANON_PASSWORD}
       ORTHANC_ANON_AE_TITLE: ${ORTHANC_ANON_AE_TITLE}
-      ORTHANC_AUTOROUTE_ANON_TO_FTPS: ${ORTHANC_AUTOROUTE_ANON_TO_FTPS}
+      ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT: ${ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT}
       ORTHANC_RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE}
       ORTHANC_RAW_DICOM_PORT: "4242"
       ORTHANC_RAW_HOSTNAME: "orthanc-raw"
diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 869dde345..1efdacad3 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -184,10 +184,10 @@ def SendViaFTPS(resourceId: str) -> None:
 
 def ShouldAutoRoute():
     """
-    Checks whether ORTHANC_AUTOROUTE_ANON_TO_FTPS environment variable is
+    Checks whether ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT environment variable is
     set to true or false
     """
-    return os.environ.get("ORTHANC_AUTOROUTE_ANON_TO_FTPS", "false").lower() == "true"
+    return os.environ.get("ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT", "false").lower() == "true"
 
 
 def OnChange(changeType: str, _level: None, resource: str) -> Any:
diff --git a/test/.env.test b/test/.env.test
index 17b7aa7c3..ab30ff527 100644
--- a/test/.env.test
+++ b/test/.env.test
@@ -43,7 +43,7 @@ ORTHANC_ANON_PASSWORD=orthanc_anon_password
 ORTHANC_ANON_AE_TITLE=ORTHANCANON
 ORTHANC_ANON_HTTP_TIMEOUT=60
 ENABLE_DICOM_WEB=true
-ORTHANC_AUTOROUTE_ANON_TO_FTPS=true
+ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT=true
 PIXL_DICOM_TRANSFER_TIMEOUT=240
 
 # UCVNAQR DICOM node information

From 0a94fdb8de714d278cfb8c144a20dfa3e5e38daa Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 17:55:10 +0000
Subject: [PATCH 11/60] Remove type annotations for `OnChange` again

---
 orthanc/orthanc-anon/plugin/pixl.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 1efdacad3..0cc520f97 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -190,7 +190,7 @@ def ShouldAutoRoute():
     return os.environ.get("ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT", "false").lower() == "true"
 
 
-def OnChange(changeType: str, _level: None, resource: str) -> Any:
+def OnChange(changeType, level, resource):  # noqa: ARG001
     """
     Three ChangeTypes included in this function:
     - If a study is stable and if ShouldAutoRoute returns true

From 139c0b93c6ced22832bd385feb96d3e67aa91428 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 18:06:37 +0000
Subject: [PATCH 12/60] Fix `glob_list` creation

Co-authored-by: Jeremy Stein <j.stein@ucl.ac.uk>
---
 test/scripts/check_ftps_upload.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 5c570f53c..7084a506d 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -25,7 +25,7 @@
 expected_output_dir = MOUNTED_DATA_DIR / project_name
 
 # Test whether DICOM images have been uploaded
-glob_list = [expected_output_dir.glob("*.zip")]
+glob_list = list(expected_output_dir.glob("*.zip"))
 logger.info(f"glob_list: {glob_list}")
 
 assert len(glob_list) == 1

From eaea84a2c8bb9b4359a6207efa87b158b68bf3fd Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 18:05:51 +0000
Subject: [PATCH 13/60] Make container naming consistent

---
 test/docker-compose.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index aeed12ee9..44adf9b95 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -77,7 +77,7 @@ services:
       pixl-net:
 
   ftp-server:
-    container_name: test-ftp-server
+    container_name: system-test-ftp-server
     build:
       context: ./dummy-services/ftp-server
     ports:

From 769c0ac7088bb5485b82bb486ec981fae72c25a9 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Tue, 30 Jan 2024 18:24:47 +0000
Subject: [PATCH 14/60] Rename mount data env variable and expand it

---
 pixl_core/tests/conftest.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py
index 4741b6233..0f554030f 100644
--- a/pixl_core/tests/conftest.py
+++ b/pixl_core/tests/conftest.py
@@ -35,7 +35,9 @@
 os.environ["FTP_PORT"] = "20021"
 
 TEST_DIR = Path(__file__).parent
-DUMMY_SERVICES_DIR = Path(__file__).parents[2] / "test" / "dummy-services"
+MOUNTED_DATA_DIR = (
+    Path(__file__).parents[2] / "test" / "dummy-services" / "ftp-server" / "mounts" / "data"
+)
 STUDY_DATE = datetime.date.fromisoformat("2023-01-01")
 
 
@@ -72,12 +74,8 @@ def mounted_data() -> Path:
     The mounted data directory for the ftp server.
     This will contain the data after successful upload.
     """
-    yield DUMMY_SERVICES_DIR / "ftp-server" / "mounts" / "data"
-    sub_dirs = [
-        f.path
-        for f in os.scandir(DUMMY_SERVICES_DIR / "ftp-server" / "mounts" / "data")
-        if f.is_dir()
-    ]
+    yield MOUNTED_DATA_DIR
+    sub_dirs = [f.path for f in os.scandir(MOUNTED_DATA_DIR) if f.is_dir()]
     # Tear down the directory after tests
     for sub_dir in sub_dirs:
         shutil.rmtree(sub_dir, ignore_errors=True)

From 99a209f5acb8a978893479748056fc38c012d974 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Wed, 31 Jan 2024 12:38:24 +0000
Subject: [PATCH 15/60] Print uploaded files instead of logging

---
 test/scripts/check_ftps_upload.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 7084a506d..d1ee9555b 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -12,12 +12,9 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-import logging
 from pathlib import Path
 from pixl_cli._io import config_from_log_file
 
-logger = logging.getLogger(__name__)
-
 PARQUET_PATH = Path(__file__).parents[1] / "resources" / "omop"
 MOUNTED_DATA_DIR = Path(__file__).parents[1] / "dummy-services" / "ftp-server" / "mounts" / "data"
 
@@ -26,7 +23,7 @@
 
 # Test whether DICOM images have been uploaded
 glob_list = list(expected_output_dir.glob("*.zip"))
-logger.info(f"glob_list: {glob_list}")
+print(f"glob_list: {glob_list}")
 
 assert len(glob_list) == 1
 

From 9c2942166149668483e005a4c735c33533652dcc Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Wed, 31 Jan 2024 19:18:38 +0000
Subject: [PATCH 16/60] Hardcode project name slug

---
 test/scripts/check_ftps_upload.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index d1ee9555b..506b62b28 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -13,13 +13,16 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 from pathlib import Path
-from pixl_cli._io import config_from_log_file
 
 PARQUET_PATH = Path(__file__).parents[1] / "resources" / "omop"
+print(f"parquet path: {PARQUET_PATH}")
 MOUNTED_DATA_DIR = Path(__file__).parents[1] / "dummy-services" / "ftp-server" / "mounts" / "data"
+print(f"mounted data dir: {MOUNTED_DATA_DIR}")
 
-project_name, omop_es_timestamp = config_from_log_file(PARQUET_PATH)
+project_name = "test-extract-uclh-omop-cdm"
+print(f"project name: {project_name}")
 expected_output_dir = MOUNTED_DATA_DIR / project_name
+print(f"expected output dir: {expected_output_dir}")
 
 # Test whether DICOM images have been uploaded
 glob_list = list(expected_output_dir.glob("*.zip"))

From 7744f0d62718cf3507fea6b2b0451c00dfd47e29 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Wed, 31 Jan 2024 19:37:06 +0000
Subject: [PATCH 17/60] Disable Azure related code in `orthanc-anon` plugin if
 no Azure available

---
 orthanc/orthanc-anon/plugin/pixl.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 0cc520f97..cd13754bd 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -49,7 +49,6 @@
 
 def AzureAccessToken():
     """
-
     Send payload to oath2/token url and
     return the response
     """
@@ -73,7 +72,6 @@ def AzureAccessToken():
 
 def AzureDICOMTokenRefresh():
     """
-
     Refresh Azure DICOM token
     If this fails then wait 30s and try again
     If successful then access_token can be used in
@@ -190,6 +188,10 @@ def ShouldAutoRoute():
     return os.environ.get("ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT", "false").lower() == "true"
 
 
+def _AzureAvailable() -> bool:
+    return os.environ("AZ_DICOM_ENDPOINT_CLIENT_ID") is not None
+
+
 def OnChange(changeType, level, resource):  # noqa: ARG001
     """
     Three ChangeTypes included in this function:
@@ -208,7 +210,7 @@ def OnChange(changeType, level, resource):  # noqa: ARG001
         print("Stable study: %s" % resource)  # noqa: T201
         SendViaFTPS(resource)
 
-    if changeType == orthanc.ChangeType.ORTHANC_STARTED:
+    if changeType == orthanc.ChangeType.ORTHANC_STARTED and _AzureAvailable():
         orthanc.LogWarning("Starting the scheduler")
         AzureDICOMTokenRefresh()
     elif changeType == orthanc.ChangeType.ORTHANC_STOPPED:

From c906fcb0d5b0426948f2592b32a9ca989080d5a8 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Wed, 31 Jan 2024 20:25:48 +0000
Subject: [PATCH 18/60] fix: get the correct pseudo-anon ID before sending via
 FTPS

---
 orthanc/orthanc-anon/plugin/pixl.py | 34 +++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index cd13754bd..779c9858b 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -162,22 +162,38 @@ def SendViaFTPS(resourceId: str) -> None:
     Makes a POST API call to upload the resource to an FTPS server
     using orthanc credentials as authorisation
     """
-    # Query orthanc-anon for the study
+    # Download zip archive of the DICOM resource
     query = f"{ORTHANC_URL}/studies/{resourceId}/archive"
+    fail_msg = "Could not download archive of resource '%s'"
+    response_study = _make_query(resourceId, query, fail_msg)
+
+    # get the zip content
+    zip_content = response_study.content
+    logging.info("Downloaded data for resource %s", resourceId)
+
+    upload.upload_dicom_image(zip_content, GetPatientID(resourceId))
+
+
+def GetPatientID(resourceId: str) -> str:
+    """
+    Queries the Orthanc instance to get the PatientID for a given resource.
+    When anonymisation has been applied, the PatientID is the pseudo-anonymised ID.
+    """
+    query = f"{ORTHANC_URL}/studies/{resourceId}"
+    fail_msg = "Could not query study for resource '%s'"
+    response_study = _make_query(resourceId, query, fail_msg)
+    return json.loads(response_study.content.decode())["PatientMainDicomTags"]["PatientID"]
+
+
+def _make_query(resourceId: str, query: str, fail_msg: str) -> requests.Response:
     try:
         response_study = requests.get(query, auth=(ORTHANC_USERNAME, ORTHANC_PASSWORD), timeout=10)
         success_code = 200
         if response_study.status_code != success_code:
-            msg = "Could not download archive of resource '%s'"
-            raise RuntimeError(msg, resourceId)
+            raise RuntimeError(fail_msg, resourceId)
     except requests.exceptions.RequestException:
         orthanc.LogError(f"Failed to query'{resourceId}'")
-
-    # get the zip content
-    zip_content = response_study.content
-    logging.info("Downloaded data for resource %s", resourceId)
-
-    upload.upload_dicom_image(zip_content, resourceId)
+    return response_study
 
 
 def ShouldAutoRoute():

From 5d53ea0baab3ad2e4499623583138b843f47fc24 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Wed, 31 Jan 2024 21:00:35 +0000
Subject: [PATCH 19/60] Add missing env variables

---
 .env.sample    | 5 +++++
 test/.env.test | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/.env.sample b/.env.sample
index 8bf322311..e1936fb3f 100644
--- a/.env.sample
+++ b/.env.sample
@@ -83,3 +83,8 @@ RABBITMQ_PASSWORD=
 # PACS extraction API
 PIXL_DICOM_TRANSFER_TIMEOUT=240
 PIXL_QUERY_TIMEOUT=10
+
+# FTP server
+FTP_HOST=
+FTP_USERNAMES=
+FTP_USER_PASS=
diff --git a/test/.env.test b/test/.env.test
index f2c0fe232..b779d9723 100644
--- a/test/.env.test
+++ b/test/.env.test
@@ -59,3 +59,8 @@ PIXL_EHR_COGSTACK_REDACT_URL=http://cogstack-api:8000/redact
 # RabbitMQ
 RABBITMQ_USERNAME=rabbitmq_username
 RABBITMQ_PASSWORD=rabbitmq_password
+
+# FTP server
+FTP_HOST=localhost
+FTP_USERNAMES=ftp_username
+FTP_USER_PASS=longpassword

From edc1789e6fda3473d96c7c58fc46645f15070d8d Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Wed, 31 Jan 2024 21:09:08 +0000
Subject: [PATCH 20/60] fix: check for env var correctly

---
 orthanc/orthanc-anon/plugin/pixl.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 779c9858b..0623831ea 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -205,7 +205,7 @@ def ShouldAutoRoute():
 
 
 def _AzureAvailable() -> bool:
-    return os.environ("AZ_DICOM_ENDPOINT_CLIENT_ID") is not None
+    return os.environ.get("AZ_DICOM_ENDPOINT_CLIENT_ID") is not None
 
 
 def OnChange(changeType, level, resource):  # noqa: ARG001

From f217a0d531991bde8458bdda2458198ba062e8ba Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Wed, 31 Jan 2024 21:39:13 +0000
Subject: [PATCH 21/60] More bug fixes

---
 .env.sample                         | 2 +-
 docker-compose.yml                  | 9 +++++++--
 orthanc/orthanc-anon/plugin/pixl.py | 4 +++-
 test/.env.test                      | 2 +-
 test/docker-compose.yml             | 4 ++--
 5 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/.env.sample b/.env.sample
index e1936fb3f..242407bbf 100644
--- a/.env.sample
+++ b/.env.sample
@@ -86,5 +86,5 @@ PIXL_QUERY_TIMEOUT=10
 
 # FTP server
 FTP_HOST=
-FTP_USERNAMES=
+FTP_USER_NAME=
 FTP_USER_PASS=
diff --git a/docker-compose.yml b/docker-compose.yml
index 71738143d..249a6563e 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -55,6 +55,11 @@ x-pixl-db: &pixl-db
   PIXL_DB_PASSWORD: ${PIXL_DB_PASSWORD}
   PIXL_DB_NAME: ${PIXL_DB_NAME}
 
+x-ftp-host: &ftp-host
+  FTP_HOST: ${FTP_HOST}
+  FTP_USER_NAME: ${FTP_USER_NAME}
+  FTP_USER_PASS: ${FTP_USER_PASS}
+
 x-logs-volume: &logs-volume
   type: volume
   source: logs
@@ -110,7 +115,7 @@ services:
         <<: *build-args-common
     command: /run/secrets
     environment:
-      <<: [*proxy-common, *pixl-common-env]
+      <<: [*proxy-common, *pixl-common-env, *ftp-host]
       ORTHANC_NAME: "PIXL: Anon"
       ORTHANC_USERNAME: ${ORTHANC_ANON_USERNAME}
       ORTHANC_PASSWORD: ${ORTHANC_ANON_PASSWORD}
@@ -223,7 +228,7 @@ services:
       args:
         <<: *build-args-common
     environment:
-      <<: [*pixl-db, *emap-db, *proxy-common, *pixl-common-env, *pixl-rabbit-mq]
+      <<: [*pixl-db, *emap-db, *proxy-common, *pixl-common-env, *pixl-rabbit-mq, *ftp-host]
       AZURE_CLIENT_ID: ${PIXL_EHR_API_AZ_CLIENT_ID}
       AZURE_CLIENT_SECRET: ${PIXL_EHR_API_AZ_CLIENT_SECRET}
       AZURE_TENANT_ID: ${PIXL_EHR_API_AZ_TENANT_ID}
diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 0623831ea..5af16a344 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -172,6 +172,7 @@ def SendViaFTPS(resourceId: str) -> None:
     logging.info("Downloaded data for resource %s", resourceId)
 
     upload.upload_dicom_image(zip_content, GetPatientID(resourceId))
+    logging.info("Uploaded data to FTPS for resource %s", resourceId)
 
 
 def GetPatientID(resourceId: str) -> str:
@@ -205,7 +206,8 @@ def ShouldAutoRoute():
 
 
 def _AzureAvailable() -> bool:
-    return os.environ.get("AZ_DICOM_ENDPOINT_CLIENT_ID") is not None
+    # Check if AZ_DICOM_ENDPOINT_CLIENT_ID is set
+    return config("AZ_DICOM_ENDPOINT_CLIENT_ID", default="") != ""
 
 
 def OnChange(changeType, level, resource):  # noqa: ARG001
diff --git a/test/.env.test b/test/.env.test
index b779d9723..ac63ef5a7 100644
--- a/test/.env.test
+++ b/test/.env.test
@@ -62,5 +62,5 @@ RABBITMQ_PASSWORD=rabbitmq_password
 
 # FTP server
 FTP_HOST=localhost
-FTP_USERNAMES=ftp_username
+FTP_USER_NAME=ftp_username
 FTP_USER_PASS=longpassword
diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index 44adf9b95..bc184fd18 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -89,8 +89,8 @@ services:
       # Mount SSL keys for TLS
       - "./dummy-services/ftp-server/mounts/ssl/:/etc/ssl/private/"
     environment:
-      ADDRESS: "localhost"
-      USERS: pixl|longpassword|/home/pixl
+      ADDRESS: "${FTP_HOST}"
+      USERS: ${FTP_USER_NAME}|${FTP_USER_PASS}|/home/${FTP_USER_NAME}
       TLS_KEY: /etc/ssl/private/localhost.key
       TLS_CERT: /etc/ssl/private/localhost.crt
     healthcheck:

From 52e5ce1ad2a63031e459a72279c51a05495bf097 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 14:35:01 +0000
Subject: [PATCH 22/60] More consistent naming of env variable

---
 docker-compose.yml           |   2 +-
 pixl_core/src/core/upload.py |   2 +-
 pixl_core/tests/conftest.py  |   2 +-
 test/docker-compose.yml      | 153 +++++++++++++++++------------------
 4 files changed, 79 insertions(+), 80 deletions(-)

diff --git a/docker-compose.yml b/docker-compose.yml
index 249a6563e..41a142266 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -58,7 +58,7 @@ x-pixl-db: &pixl-db
 x-ftp-host: &ftp-host
   FTP_HOST: ${FTP_HOST}
   FTP_USER_NAME: ${FTP_USER_NAME}
-  FTP_USER_PASS: ${FTP_USER_PASS}
+  FTP_USER_PASSWORD: ${FTP_USER_PASSWORD}
 
 x-logs-volume: &logs-volume
   type: volume
diff --git a/pixl_core/src/core/upload.py b/pixl_core/src/core/upload.py
index 411f7dbfd..98d126db2 100644
--- a/pixl_core/src/core/upload.py
+++ b/pixl_core/src/core/upload.py
@@ -82,7 +82,7 @@ def _connect_to_ftp() -> FTP_TLS:
     ftp_host = os.environ["FTP_HOST"]
     ftp_port = os.environ["FTP_PORT"]  # FTPS usually uses port 21
     ftp_user = os.environ["FTP_USER_NAME"]
-    ftp_password = os.environ["FTP_USER_PASS"]
+    ftp_password = os.environ["FTP_USER_PASSWORD"]
 
     # Connect to the server and login
     ftp = ImplicitFtpTls()
diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py
index 0f554030f..5d96492c4 100644
--- a/pixl_core/tests/conftest.py
+++ b/pixl_core/tests/conftest.py
@@ -31,7 +31,7 @@
 os.environ["RABBITMQ_PORT"] = "25672"
 os.environ["FTP_HOST"] = "localhost"
 os.environ["FTP_USER_NAME"] = "pixl"
-os.environ["FTP_USER_PASS"] = "longpassword"  # noqa: S105 Hardcoding password
+os.environ["FTP_USER_PASSWORD"] = "longpassword"  # noqa: S105 Hardcoding password
 os.environ["FTP_PORT"] = "20021"
 
 TEST_DIR = Path(__file__).parent
diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index bc184fd18..383528195 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -14,87 +14,86 @@
 version: "3.8"
 
 volumes:
-  vna-qr-data:
+    vna-qr-data:
 
 networks:
-  pixl-net:
+    pixl-net:
 
 services:
+    vna-qr:
+        image: osimis/orthanc:22.9.0-full-stable
+        environment:
+            ORTHANC_NAME: ${VNAQR_AE_TITLE}
+            ORTHANC_USERNAME: "orthanc"
+            ORTHANC_PASSWORD: "orthanc"
+            ORTHANC_AE_TITLE: "VNAQR"
+            RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE}
+            RAW_DICOM_PORT: "4242"
+            RAW_IP_ADDR: "orthanc-raw" # aka. hostname
+        ports:
+            - "4243:4242"
+            - "8043:8042"
+        volumes:
+            - ${PWD}/vna_config/:/run/secrets:ro
+        healthcheck:
+            test: ["CMD", "curl", "-f", "-u", "orthanc:orthanc", "http://vna-qr:8042/patients"]
+            interval: 10s
+            timeout: 30s
+            retries: 5
+        networks:
+            pixl-net:
 
-  vna-qr:
-    image: osimis/orthanc:22.9.0-full-stable
-    environment:
-      ORTHANC_NAME: ${VNAQR_AE_TITLE}
-      ORTHANC_USERNAME: "orthanc"
-      ORTHANC_PASSWORD: "orthanc"
-      ORTHANC_AE_TITLE: "VNAQR"
-      RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE}
-      RAW_DICOM_PORT: "4242"
-      RAW_IP_ADDR: "orthanc-raw"  # aka. hostname
-    ports:
-      - "4243:4242"
-      - "8043:8042"
-    volumes:
-      - ${PWD}/vna_config/:/run/secrets:ro
-    healthcheck:
-      test: [ "CMD", "curl", "-f", "-u" , "orthanc:orthanc", "http://vna-qr:8042/patients" ]
-      interval: 10s
-      timeout: 30s
-      retries: 5
-    networks:
-      pixl-net:
+    star:
+        container_name: system-test-fake-star-db
+        build:
+            context: ..
+            dockerfile: test/Dockerfile.fake_emap_star
+            args:
+                N_TABLE_ROWS: 2
+        environment:
+            POSTGRES_USER: ${EMAP_UDS_USER}
+            POSTGRES_PASSWORD: ${EMAP_UDS_PASSWORD}
+        healthcheck:
+            test: ["CMD", "pg_isready", "-U", "postgres"]
+            interval: 10s
+            timeout: 30s
+            retries: 5
+        ports:
+            - "5432:5432"
+        networks:
+            pixl-net:
 
-  star:
-    container_name: system-test-fake-star-db
-    build:
-      context: ..
-      dockerfile: test/Dockerfile.fake_emap_star
-      args:
-        N_TABLE_ROWS: 2
-    environment:
-      POSTGRES_USER: ${EMAP_UDS_USER}
-      POSTGRES_PASSWORD: ${EMAP_UDS_PASSWORD}
-    healthcheck:
-      test: ["CMD", "pg_isready", "-U", "postgres"]
-      interval: 10s
-      timeout: 30s
-      retries: 5
-    ports:
-      - "5432:5432"
-    networks:
-      pixl-net:
+    cogstack-api:
+        container_name: system-test-cogstack
+        build:
+            context: ./dummy-services/cogstack/
+        healthcheck:
+            test: ["CMD", "curl", "-f", "http://localhost:8000/heart-beat"]
+            interval: 10s
+            timeout: 30s
+            retries: 5
+        networks:
+            pixl-net:
 
-  cogstack-api:
-    container_name: system-test-cogstack
-    build:
-      context: ./dummy-services/cogstack/
-    healthcheck:
-      test: [ "CMD", "curl", "-f", "http://localhost:8000/heart-beat" ]
-      interval: 10s
-      timeout: 30s
-      retries: 5
-    networks:
-      pixl-net:
-
-  ftp-server:
-    container_name: system-test-ftp-server
-    build:
-      context: ./dummy-services/ftp-server
-    ports:
-      - "20021:21"
-      - "21000-21010:21000-21010"
-    volumes:
-      # Mount for uploaded data
-      - "./dummy-services/ftp-server/mounts/data/:/home/pixl/"
-      # Mount SSL keys for TLS
-      - "./dummy-services/ftp-server/mounts/ssl/:/etc/ssl/private/"
-    environment:
-      ADDRESS: "${FTP_HOST}"
-      USERS: ${FTP_USER_NAME}|${FTP_USER_PASS}|/home/${FTP_USER_NAME}
-      TLS_KEY: /etc/ssl/private/localhost.key
-      TLS_CERT: /etc/ssl/private/localhost.crt
-    healthcheck:
-      test: ping localhost:21 -w 2
-      interval: 10s
-      timeout: 5s
-      retries: 5
+    ftp-server:
+        container_name: system-test-ftp-server
+        build:
+            context: ./dummy-services/ftp-server
+        ports:
+            - "20021:21"
+            - "21000-21010:21000-21010"
+        volumes:
+            # Mount for uploaded data
+            - "./dummy-services/ftp-server/mounts/data/:/home/pixl/"
+            # Mount SSL keys for TLS
+            - "./dummy-services/ftp-server/mounts/ssl/:/etc/ssl/private/"
+        environment:
+            ADDRESS: "${FTP_HOST}"
+            USERS: ${FTP_USER_NAME}|${FTP_USER_PASSWORD}|/home/${FTP_USER_NAME}
+            TLS_KEY: /etc/ssl/private/localhost.key
+            TLS_CERT: /etc/ssl/private/localhost.crt
+        healthcheck:
+            test: ping localhost:21 -w 2
+            interval: 10s
+            timeout: 5s
+            retries: 5

From 104d50c6b260420439606e4006793e1bad1ec527 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 14:39:56 +0000
Subject: [PATCH 23/60] Rename query helper function

---
 orthanc/orthanc-anon/plugin/pixl.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 5af16a344..6bb9c2bf8 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -165,7 +165,7 @@ def SendViaFTPS(resourceId: str) -> None:
     # Download zip archive of the DICOM resource
     query = f"{ORTHANC_URL}/studies/{resourceId}/archive"
     fail_msg = "Could not download archive of resource '%s'"
-    response_study = _make_query(resourceId, query, fail_msg)
+    response_study = _query(resourceId, query, fail_msg)
 
     # get the zip content
     zip_content = response_study.content
@@ -182,11 +182,11 @@ def GetPatientID(resourceId: str) -> str:
     """
     query = f"{ORTHANC_URL}/studies/{resourceId}"
     fail_msg = "Could not query study for resource '%s'"
-    response_study = _make_query(resourceId, query, fail_msg)
+    response_study = _query(resourceId, query, fail_msg)
     return json.loads(response_study.content.decode())["PatientMainDicomTags"]["PatientID"]
 
 
-def _make_query(resourceId: str, query: str, fail_msg: str) -> requests.Response:
+def _query(resourceId: str, query: str, fail_msg: str) -> requests.Response:
     try:
         response_study = requests.get(query, auth=(ORTHANC_USERNAME, ORTHANC_PASSWORD), timeout=10)
         success_code = 200

From 9136b4f608e43495f53aa57df47775a293ba7c79 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 14:41:49 +0000
Subject: [PATCH 24/60] Make helper functions snakecase

---
 orthanc/orthanc-anon/plugin/pixl.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 6bb9c2bf8..7d68cfc22 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -171,11 +171,11 @@ def SendViaFTPS(resourceId: str) -> None:
     zip_content = response_study.content
     logging.info("Downloaded data for resource %s", resourceId)
 
-    upload.upload_dicom_image(zip_content, GetPatientID(resourceId))
+    upload.upload_dicom_image(zip_content, _get_patient_id(resourceId))
     logging.info("Uploaded data to FTPS for resource %s", resourceId)
 
 
-def GetPatientID(resourceId: str) -> str:
+def _get_patient_id(resourceId: str) -> str:
     """
     Queries the Orthanc instance to get the PatientID for a given resource.
     When anonymisation has been applied, the PatientID is the pseudo-anonymised ID.
@@ -205,7 +205,7 @@ def ShouldAutoRoute():
     return os.environ.get("ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT", "false").lower() == "true"
 
 
-def _AzureAvailable() -> bool:
+def _azure_available() -> bool:
     # Check if AZ_DICOM_ENDPOINT_CLIENT_ID is set
     return config("AZ_DICOM_ENDPOINT_CLIENT_ID", default="") != ""
 
@@ -228,7 +228,7 @@ def OnChange(changeType, level, resource):  # noqa: ARG001
         print("Stable study: %s" % resource)  # noqa: T201
         SendViaFTPS(resource)
 
-    if changeType == orthanc.ChangeType.ORTHANC_STARTED and _AzureAvailable():
+    if changeType == orthanc.ChangeType.ORTHANC_STARTED and _azure_available():
         orthanc.LogWarning("Starting the scheduler")
         AzureDICOMTokenRefresh()
     elif changeType == orthanc.ChangeType.ORTHANC_STOPPED:

From dd1dd7a477d9c321ca911ff0ce8f0c0c62855acd Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 14:47:13 +0000
Subject: [PATCH 25/60] Also update the `.env` files

---
 .env.sample    | 2 +-
 test/.env.test | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/.env.sample b/.env.sample
index 242407bbf..6438e8c8a 100644
--- a/.env.sample
+++ b/.env.sample
@@ -87,4 +87,4 @@ PIXL_QUERY_TIMEOUT=10
 # FTP server
 FTP_HOST=
 FTP_USER_NAME=
-FTP_USER_PASS=
+FTP_USER_PASSWORD=
diff --git a/test/.env.test b/test/.env.test
index ac63ef5a7..3170a3f6a 100644
--- a/test/.env.test
+++ b/test/.env.test
@@ -63,4 +63,4 @@ RABBITMQ_PASSWORD=rabbitmq_password
 # FTP server
 FTP_HOST=localhost
 FTP_USER_NAME=ftp_username
-FTP_USER_PASS=longpassword
+FTP_USER_PASSWORD=longpassword

From fce8fab2f7c2c915afe15533fee420e2bd6e5dbd Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 15:37:04 +0000
Subject: [PATCH 26/60] Make orthanc plugin code more type safe and system exit
 when query fails

---
 orthanc/orthanc-anon/plugin/pixl.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 7d68cfc22..a1099fb58 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -188,16 +188,18 @@ def _get_patient_id(resourceId: str) -> str:
 
 def _query(resourceId: str, query: str, fail_msg: str) -> requests.Response:
     try:
-        response_study = requests.get(query, auth=(ORTHANC_USERNAME, ORTHANC_PASSWORD), timeout=10)
+        response = requests.get(query, auth=(ORTHANC_USERNAME, ORTHANC_PASSWORD), timeout=10)
         success_code = 200
-        if response_study.status_code != success_code:
+        if response.status_code != success_code:
             raise RuntimeError(fail_msg, resourceId)
-    except requests.exceptions.RequestException:
+    except requests.exceptions.RequestException as request_exception:
         orthanc.LogError(f"Failed to query'{resourceId}'")
-    return response_study
+        raise SystemExit from request_exception
+    else:
+        return response
 
 
-def ShouldAutoRoute():
+def ShouldAutoRoute() -> bool:
     """
     Checks whether ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT environment variable is
     set to true or false

From 5484e0602cd36be8e7e045bc087d22e9a29478ec Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 15:48:04 +0000
Subject: [PATCH 27/60] More type safety

---
 orthanc/orthanc-anon/plugin/pixl.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index a1099fb58..ae24b33bf 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -182,8 +182,10 @@ def _get_patient_id(resourceId: str) -> str:
     """
     query = f"{ORTHANC_URL}/studies/{resourceId}"
     fail_msg = "Could not query study for resource '%s'"
+
     response_study = _query(resourceId, query, fail_msg)
-    return json.loads(response_study.content.decode())["PatientMainDicomTags"]["PatientID"]
+    json_response = json.loads(response_study.content.decode())
+    return str(json_response["PatientMainDicomTags"]["PatientID"])
 
 
 def _query(resourceId: str, query: str, fail_msg: str) -> requests.Response:

From a7f292eccec2e36fdb0bdc955b3a3fb5b6d0e483 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 16:00:44 +0000
Subject: [PATCH 28/60] Define the `FTP_PORT` environment variable

---
 .env.sample             | 1 +
 docker-compose.yml      | 1 +
 test/.env.test          | 1 +
 test/docker-compose.yml | 2 +-
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/.env.sample b/.env.sample
index 6438e8c8a..23ce281d8 100644
--- a/.env.sample
+++ b/.env.sample
@@ -88,3 +88,4 @@ PIXL_QUERY_TIMEOUT=10
 FTP_HOST=
 FTP_USER_NAME=
 FTP_USER_PASSWORD=
+FTP_PORT=
diff --git a/docker-compose.yml b/docker-compose.yml
index 41a142266..a73f686d0 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -59,6 +59,7 @@ x-ftp-host: &ftp-host
   FTP_HOST: ${FTP_HOST}
   FTP_USER_NAME: ${FTP_USER_NAME}
   FTP_USER_PASSWORD: ${FTP_USER_PASSWORD}
+  FTP_PORT: ${FTP_PORT}
 
 x-logs-volume: &logs-volume
   type: volume
diff --git a/test/.env.test b/test/.env.test
index 3170a3f6a..9c8074b05 100644
--- a/test/.env.test
+++ b/test/.env.test
@@ -64,3 +64,4 @@ RABBITMQ_PASSWORD=rabbitmq_password
 FTP_HOST=localhost
 FTP_USER_NAME=ftp_username
 FTP_USER_PASSWORD=longpassword
+FTP_PORT=20021
diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index 383528195..aec9d99d8 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -80,7 +80,7 @@ services:
         build:
             context: ./dummy-services/ftp-server
         ports:
-            - "20021:21"
+            - "${FTP_PORT}:21"
             - "21000-21010:21000-21010"
         volumes:
             # Mount for uploaded data

From 7f314cdc365c4bc9722cff4a3f41405b964f723e Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 16:06:11 +0000
Subject: [PATCH 29/60] Document the FTPS server requirements

Should have done this way earlier!
---
 docs/services/ftp-server.md | 21 +++++++++++++++++++++
 pixl_core/README.md         | 14 ++++++++++++++
 2 files changed, 35 insertions(+)
 create mode 100644 docs/services/ftp-server.md

diff --git a/docs/services/ftp-server.md b/docs/services/ftp-server.md
new file mode 100644
index 000000000..2c33bcc66
--- /dev/null
+++ b/docs/services/ftp-server.md
@@ -0,0 +1,21 @@
+# FTPS server
+
+Currently, we can only upload files to the Data Safe Haven (DSH) through an
+[FTPS](https://en.wikipedia.org/wiki/FTPS) connection.
+
+The [`core.upload`](../../pixl_core/src/core/upload.py) module implements functionality to upload
+DICOM tags and parquet files to an **FTPS server**. This requires the following environment
+variables to be set:
+
+-   `FTP_HOST`: URL to the FTPS server
+-   `FTP_PORT`: port on which the FTPS server is listening
+-   `FTP_USER_NAME`: name of user with access to the FTPS server
+-   `FTP_USER_PASSWORD`: password for the authorised user
+
+We provide mock values for these for the unit tests (see
+[`./tests/conftest.py`](./tests/conftest.py)). When running in production, these should be defined
+in the `.env` file (see [the example](../.env.sample)).
+
+For the `pixl_core` unit tests and the system test, we spin up an FTPS server with a Docker
+container, defined in [`test/dummy-services/ftp-server`](../../test/dummy-services/ftp-server/) and
+set the necessary environment variables in [`.env.test`](../../test/.env.test).
diff --git a/pixl_core/README.md b/pixl_core/README.md
index b8b513535..447317ed5 100644
--- a/pixl_core/README.md
+++ b/pixl_core/README.md
@@ -85,3 +85,17 @@ for convenience `latest` is a symlink to the most recent extract.
     │             └── public
     └── latest -> all_extracts/2023-12-13t16-22-40
 ```
+
+## Uploading to an FTPS server
+
+The `core.upload` module implements functionality to upload DICOM tags and parquet files to an
+**FTPS server**. This requires the following environment variables to be set:
+
+-   `FTP_HOST`: URL to the FTPS server
+-   `FTP_PORT`: port on which the FTPS server is listening
+-   `FTP_USER_NAME`: name of user with access to the FTPS server
+-   `FTP_USER_PASSWORD`: password for the authorised user
+
+We provide mock values for these for the unit tests (see
+[`./tests/conftest.py`](./tests/conftest.py)). When running in production, these should be defined
+in the `.env` file (see [the example](../.env.sample)).

From c25ac15336b46e45baa46ac1eace43845a90db0c Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 16:06:40 +0000
Subject: [PATCH 30/60] Markdown formatting

---
 pixl_core/README.md | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/pixl_core/README.md b/pixl_core/README.md
index 447317ed5..8e477b6d1 100644
--- a/pixl_core/README.md
+++ b/pixl_core/README.md
@@ -6,13 +6,14 @@ upstream services.
 
 Specifically, it defines:
 
-- The [Token buffer](#token-buffer) for rate limiting requests to the upstream services
-- The [RabbitMQ queue](#patient-queue) implementation shared by the EHR and Imaging APIs
-- The PIXL `postgres` internal database for storing exported images and extracts from the messages
-  processed by the CLI driver
-- The [`ParquetExport`](./src/core/exports.py) class for exporting OMOP and EMAP extracts to parquet files
-- Handling of [uploads over FTPS](./src/core/upload.py), used to transfer images and parquet files
-  to the DSH (Data Safe Haven)
+-   The [Token buffer](#token-buffer) for rate limiting requests to the upstream services
+-   The [RabbitMQ queue](#patient-queue) implementation shared by the EHR and Imaging APIs
+-   The PIXL `postgres` internal database for storing exported images and extracts from the messages
+    processed by the CLI driver
+-   The [`ParquetExport`](./src/core/exports.py) class for exporting OMOP and EMAP extracts to
+    parquet files
+-   Handling of [uploads over FTPS](./src/core/upload.py), used to transfer images and parquet files
+    to the DSH (Data Safe Haven)
 
 ## Installation
 
@@ -33,9 +34,9 @@ The token buffer is needed to limit the download rate for images from PAX/VNA. C
 suggests that a rate limit of five images per second should be sufficient, however that may have to
 be altered dynamically through command line interaction.
 
-The current implementation of the token buffer uses the [token bucket implementation from
-Falconry](https://github.com/falconry/token-bucket/). Furthermore, the token buffer is not set up as
-a service as it is only needed for the image download rate.
+The current implementation of the token buffer uses the
+[token bucket implementation from Falconry](https://github.com/falconry/token-bucket/). Furthermore,
+the token buffer is not set up as a service as it is only needed for the image download rate.
 
 ## Patient queue
 
@@ -45,7 +46,8 @@ different PIXL services. Currently, we define two queues:
 1. `pacs` for downloading and de-identifying images
 2. `ehr` for downloading and de-identifying the EHR data
 
-The image anonymisation will be triggered automatically once the image has been downloaded to the raw Orthanc server.
+The image anonymisation will be triggered automatically once the image has been downloaded to the
+raw Orthanc server.
 
 ### RabbitMQ
 
@@ -54,8 +56,8 @@ RabbitMQ is used for the queue implementation.
 The client of choice for RabbitMQ at this point in time is
 [pika](https://pika.readthedocs.io/en/stable/), which provides both a synchronous and asynchronous
 way of transferring messages. The former is geared towards high data throughput whereas the latter
-is geared towards stability.  The asynchronous mode of transferring messages is a lot more complex
-as it is based on the [asyncio event loop](https://docs.python.org/3/library/asyncio-eventloop.html).
+is geared towards stability. The asynchronous mode of transferring messages is a lot more complex as
+it is based on the [asyncio event loop](https://docs.python.org/3/library/asyncio-eventloop.html).
 
 ### OMOP ES files
 

From a5b1123c8f8ff7656473f721445a1345f4d260ab Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 18:03:36 +0000
Subject: [PATCH 31/60] Fix FTP config

---
 orthanc/orthanc-anon/plugin/pixl.py |  1 -
 pixl_core/src/core/upload.py        | 19 ++++++++++++++-----
 test/.env.test                      |  4 ++--
 test/docker-compose.yml             |  8 +++++---
 4 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index ae24b33bf..855321063 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -223,7 +223,6 @@ def OnChange(changeType, level, resource):  # noqa: ARG001
     and AzureDICOMTokenRefresh called
     - If orthanc has stopped and TIMER is not none then message added
     to Orthanc LogWarning and TIMER cancelled
-
     """
     if not ShouldAutoRoute():
         return
diff --git a/pixl_core/src/core/upload.py b/pixl_core/src/core/upload.py
index 98d126db2..ab1d6bf3b 100644
--- a/pixl_core/src/core/upload.py
+++ b/pixl_core/src/core/upload.py
@@ -68,7 +68,12 @@ def upload_dicom_image(zip_content: BinaryIO, pseudo_anon_id: str) -> None:
     logger.debug("Running %s", command)
 
     # Store the file using a binary handler
-    ftp.storbinary(command, zip_content)
+    try:
+        ftp.storbinary(command, zip_content)
+    except ftplib.all_errors as ftp_error:
+        ftp.quit()
+        error_msg = "Failed to run STOR command '%s': '%s'"
+        raise ConnectionError(error_msg, command, ftp_error) from ftp_error
 
     # Close the FTP connection
     ftp.quit()
@@ -85,10 +90,14 @@ def _connect_to_ftp() -> FTP_TLS:
     ftp_password = os.environ["FTP_USER_PASSWORD"]
 
     # Connect to the server and login
-    ftp = ImplicitFtpTls()
-    ftp.connect(ftp_host, int(ftp_port))
-    ftp.login(ftp_user, ftp_password)
-    ftp.prot_p()
+    try:
+        ftp = ImplicitFtpTls()
+        ftp.connect(ftp_host, int(ftp_port))
+        ftp.login(ftp_user, ftp_password)
+        ftp.prot_p()
+    except ftplib.all_errors as ftp_error:
+        error_msg = "Failed to connect to FTPS server: '%s'"
+        raise ConnectionError(error_msg, ftp_error) from ftp_error
     return ftp
 
 
diff --git a/test/.env.test b/test/.env.test
index 9c8074b05..6ac095d2c 100644
--- a/test/.env.test
+++ b/test/.env.test
@@ -29,6 +29,7 @@ PIXL_EHR_API_PORT=7006
 PIXL_PACS_API_PORT=7007
 RABBITMQ_PORT=7008
 RABBITMQ_ADMIN_PORT=7009
+FTP_PORT=21
 
 # PIXL Orthanc raw instance
 ORTHANC_RAW_USERNAME=orthanc_raw_username
@@ -61,7 +62,6 @@ RABBITMQ_USERNAME=rabbitmq_username
 RABBITMQ_PASSWORD=rabbitmq_password
 
 # FTP server
-FTP_HOST=localhost
+FTP_HOST=ftp-server
 FTP_USER_NAME=ftp_username
 FTP_USER_PASSWORD=longpassword
-FTP_PORT=20021
diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index aec9d99d8..be5dc54b3 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -80,15 +80,15 @@ services:
         build:
             context: ./dummy-services/ftp-server
         ports:
-            - "${FTP_PORT}:21"
+            - "127.0.0.1:20021:21"
             - "21000-21010:21000-21010"
         volumes:
             # Mount for uploaded data
-            - "./dummy-services/ftp-server/mounts/data/:/home/pixl/"
+            - "./dummy-services/ftp-server/mounts/data/:/home/${FTP_USER_NAME}/"
             # Mount SSL keys for TLS
             - "./dummy-services/ftp-server/mounts/ssl/:/etc/ssl/private/"
         environment:
-            ADDRESS: "${FTP_HOST}"
+            ADDRESS: "localhost"
             USERS: ${FTP_USER_NAME}|${FTP_USER_PASSWORD}|/home/${FTP_USER_NAME}
             TLS_KEY: /etc/ssl/private/localhost.key
             TLS_CERT: /etc/ssl/private/localhost.crt
@@ -97,3 +97,5 @@ services:
             interval: 10s
             timeout: 5s
             retries: 5
+        networks:
+            pixl-net:

From 65f7d0b2e5aece81287038c4418bb478c87c1346 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Thu, 1 Feb 2024 18:58:42 +0000
Subject: [PATCH 32/60] Add more FTP logging

---
 test/dummy-services/ftp-server/Dockerfile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/test/dummy-services/ftp-server/Dockerfile b/test/dummy-services/ftp-server/Dockerfile
index deea7891f..be4aeb079 100644
--- a/test/dummy-services/ftp-server/Dockerfile
+++ b/test/dummy-services/ftp-server/Dockerfile
@@ -19,4 +19,8 @@ ssl_enable=YES\n\
 ssl_sslv2=YES\n\
 ssl_sslv3=YES\n\
 implicit_ssl=YES\n\
-require_ssl_reuse=NO' >> /etc/vsftpd/vsftpd.conf
\ No newline at end of file
+require_ssl_reuse=NO\n\
+xferlog_std_format=YES\n\
+log_ftp_protocol=YES\n\
+vsftpd_log_file=/var/log/vsftpd.log\n\
+syslog_enable=NO' >> /etc/vsftpd/vsftpd.conf

From b487e4b70178c56103b3ce03f51dab1e99de8e8f Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 09:26:38 +0000
Subject: [PATCH 33/60] Allow python 11 for CLI

---
 cli/pyproject.toml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cli/pyproject.toml b/cli/pyproject.toml
index fbc33dd7b..3c7f01d03 100644
--- a/cli/pyproject.toml
+++ b/cli/pyproject.toml
@@ -6,7 +6,7 @@ authors = [
 ]
 description = "PIXL command line interface"
 readme = "README.md"
-requires-python = "<3.11"
+requires-python = "<3.12"
 classifiers = [
     "Programming Language :: Python :: 3"
 ]

From d1f0a6d6782a460a5377d93a97ed36f99cf1f8f7 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 09:27:50 +0000
Subject: [PATCH 34/60] Ignore new data mount location

---
 .gitignore | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index e8f43ec66..ed0409bdb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -148,4 +148,4 @@ dmypy.json
 # project specific files
 /exports/
 **/test_producer.csv
-pixl_core/tests/ftp-server/mounts/data/*
\ No newline at end of file
+/test/dummy-services/ftp-server/mounts/data/*

From 0ad83b1b7d874ce6527e20108d4a6b27c127fe19 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 09:28:11 +0000
Subject: [PATCH 35/60] Allow system tests to pass

---
 orthanc/orthanc-anon/plugin/pixl.py | 4 ++--
 test/docker-compose.yml             | 2 +-
 test/scripts/check_ftps_upload.py   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 855321063..7a4ef0304 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -171,7 +171,7 @@ def SendViaFTPS(resourceId: str) -> None:
     zip_content = response_study.content
     logging.info("Downloaded data for resource %s", resourceId)
 
-    upload.upload_dicom_image(zip_content, _get_patient_id(resourceId))
+    upload.upload_dicom_image(BytesIO(zip_content), _get_patient_id(resourceId))
     logging.info("Uploaded data to FTPS for resource %s", resourceId)
 
 
@@ -246,7 +246,7 @@ def OnHeartBeat(output, uri, **request) -> Any:  # noqa: ARG001
     output.AnswerBuffer("OK\n", "text/plain")
 
 
-def ReceivedInstanceCallback(receivedDicom: BytesIO, origin: str) -> Any:
+def ReceivedInstanceCallback(receivedDicom: bytes, origin: str) -> Any:
     """Modifies a DICOM instance received by Orthanc and applies anonymisation."""
     if origin == orthanc.InstanceOrigin.REST_API:
         orthanc.LogWarning("DICOM instance received from the REST API")
diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index be5dc54b3..c119a9661 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -88,7 +88,7 @@ services:
             # Mount SSL keys for TLS
             - "./dummy-services/ftp-server/mounts/ssl/:/etc/ssl/private/"
         environment:
-            ADDRESS: "localhost"
+            ADDRESS: ${FTP_HOST}
             USERS: ${FTP_USER_NAME}|${FTP_USER_PASSWORD}|/home/${FTP_USER_NAME}
             TLS_KEY: /etc/ssl/private/localhost.key
             TLS_CERT: /etc/ssl/private/localhost.crt
diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 506b62b28..45f971119 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -28,6 +28,6 @@
 glob_list = list(expected_output_dir.glob("*.zip"))
 print(f"glob_list: {glob_list}")
 
-assert len(glob_list) == 1
+assert len(glob_list) == 2
 
 # TODO: check parquet files upload

From c9d708362e8771fc2619e0fc455e856890053f03 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 10:13:33 +0000
Subject: [PATCH 36/60] DEBUG: check ftp logs

---
 .github/workflows/main.yml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index a5dd3b952..22019a637 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -245,3 +245,8 @@ jobs:
         working-directory: test
         run: |
           ./run-system-test.sh
+      - name: Check ftp logs
+        if: always()
+        working-directory: test
+        run: |
+          docker logs system-test-ftp-server

From de73920cf919a4b9a9fef74ad6536e3db73361d6 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 10:32:45 +0000
Subject: [PATCH 37/60] Poll for two minutes instead of hardcoded wait

---
 test/run-system-test.sh                       |  4 ++--
 ... check_entry_in_orthanc_anon_for_2_min.py} | 19 +++++++++++++++----
 2 files changed, 17 insertions(+), 6 deletions(-)
 rename test/scripts/{check_entry_in_orthanc_anon.py => check_entry_in_orthanc_anon_for_2_min.py} (56%)

diff --git a/test/run-system-test.sh b/test/run-system-test.sh
index 745275d15..7b73cb0c6 100755
--- a/test/run-system-test.sh
+++ b/test/run-system-test.sh
@@ -30,9 +30,9 @@ docker compose --env-file .env.test -p system-test up --wait -d --build --remove
 pip install -e "${PACKAGE_DIR}/pixl_core" && pip install -e "${PACKAGE_DIR}/cli"
 pixl populate "${PACKAGE_DIR}/test/resources/omop"
 pixl start
-sleep 65  # need to wait until the DICOM image is "stable" = 60s
+# need to wait until the DICOM image is "stable" so poll for 2 minutes to check
+./scripts/check_entry_in_orthanc_anon_for_2_min.py
 ./scripts/check_entry_in_pixl_anon.sh
-./scripts/check_entry_in_orthanc_anon.py
 ./scripts/check_max_storage_in_orthanc_raw.sh
 ./scripts/check_ftps_upload.py
 
diff --git a/test/scripts/check_entry_in_orthanc_anon.py b/test/scripts/check_entry_in_orthanc_anon_for_2_min.py
similarity index 56%
rename from test/scripts/check_entry_in_orthanc_anon.py
rename to test/scripts/check_entry_in_orthanc_anon_for_2_min.py
index 13d43243a..311657bd2 100755
--- a/test/scripts/check_entry_in_orthanc_anon.py
+++ b/test/scripts/check_entry_in_orthanc_anon_for_2_min.py
@@ -17,15 +17,26 @@
 """
 After pixl has run this script will query the orthanc-anon REST API to check
 that the correct number of instances have been received.
+
+Because we have to wait for a stable study, poll for 2 minutes
 """
 
 import json
 import shlex
 import subprocess
+from time import sleep
+
+SECONDS_WAIT = 5
 
+# Check for two minutes that the instances have been received in orthanc anon
+instances = []
+for seconds in range(0, 121, SECONDS_WAIT):
+    instances_cmd = shlex.split('docker exec system-test-orthanc-anon-1 curl -u "orthanc_anon_username:orthanc_anon_password" http://orthanc-anon:8042/instances')
+    instances_output = subprocess.run(instances_cmd, capture_output=True, check=True, text=True)
+    instances = json.loads(instances_output.stdout)
+    print("orthanc-anon instances:", instances)
+    if len(instances) == 2:
+        break
+    sleep(SECONDS_WAIT)
 
-instances_cmd = shlex.split('docker exec system-test-orthanc-anon-1 curl -u "orthanc_anon_username:orthanc_anon_password" http://orthanc-anon:8042/instances')
-instances_output = subprocess.run(instances_cmd, capture_output=True, check=True, text=True)
-instances = json.loads(instances_output.stdout)
-print("orthanc-anon instances:", instances)
 assert len(instances) == 2

From 4bff8b956b8d688187f949ed164de671e6a503f5 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 10:33:57 +0000
Subject: [PATCH 38/60] Poll for two minutes instead of hardcoded wait

---
 test/scripts/check_entry_in_orthanc_anon_for_2_min.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/scripts/check_entry_in_orthanc_anon_for_2_min.py b/test/scripts/check_entry_in_orthanc_anon_for_2_min.py
index 311657bd2..b753acd19 100755
--- a/test/scripts/check_entry_in_orthanc_anon_for_2_min.py
+++ b/test/scripts/check_entry_in_orthanc_anon_for_2_min.py
@@ -34,7 +34,7 @@
     instances_cmd = shlex.split('docker exec system-test-orthanc-anon-1 curl -u "orthanc_anon_username:orthanc_anon_password" http://orthanc-anon:8042/instances')
     instances_output = subprocess.run(instances_cmd, capture_output=True, check=True, text=True)
     instances = json.loads(instances_output.stdout)
-    print("orthanc-anon instances:", instances)
+    print(f"Waited for {seconds} seconds, orthanc-anon instances: {instances}")
     if len(instances) == 2:
         break
     sleep(SECONDS_WAIT)

From a84cde7b5471739675b7d0db90b77022a7d8d286 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 10:46:30 +0000
Subject: [PATCH 39/60] More debugging

---
 .github/workflows/main.yml        |  3 ++-
 test/scripts/check_ftps_upload.py | 12 +++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 22019a637..a6c82cf24 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -249,4 +249,5 @@ jobs:
         if: always()
         working-directory: test
         run: |
-          docker logs system-test-ftp-server
+          docker logs system-test-ftp-server &&\
+          docker logs system-test-orthanc-anon-1 --tail=1000
diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 45f971119..403952828 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -24,9 +24,15 @@
 expected_output_dir = MOUNTED_DATA_DIR / project_name
 print(f"expected output dir: {expected_output_dir}")
 
-# Test whether DICOM images have been uploaded
-glob_list = list(expected_output_dir.glob("*.zip"))
-print(f"glob_list: {glob_list}")
+SECONDS_WAIT = 5
+
+glob_list = []
+for seconds in range(0, 121, SECONDS_WAIT):
+    # Test whether DICOM images have been uploaded
+    glob_list = list(expected_output_dir.glob("*.zip"))
+    print(f"Waited for {seconds} seconds. glob_list: {glob_list}")
+    if glob_list:
+        break
 
 assert len(glob_list) == 2
 

From 324d17ca49ebefc6a6200b23c792df125ed6d6aa Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 10:48:59 +0000
Subject: [PATCH 40/60] Reorder system test scripts

---
 test/run-system-test.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/run-system-test.sh b/test/run-system-test.sh
index 7b73cb0c6..c0be0fd80 100755
--- a/test/run-system-test.sh
+++ b/test/run-system-test.sh
@@ -30,10 +30,11 @@ docker compose --env-file .env.test -p system-test up --wait -d --build --remove
 pip install -e "${PACKAGE_DIR}/pixl_core" && pip install -e "${PACKAGE_DIR}/cli"
 pixl populate "${PACKAGE_DIR}/test/resources/omop"
 pixl start
+
+./scripts/check_max_storage_in_orthanc_raw.sh
 # need to wait until the DICOM image is "stable" so poll for 2 minutes to check
 ./scripts/check_entry_in_orthanc_anon_for_2_min.py
 ./scripts/check_entry_in_pixl_anon.sh
-./scripts/check_max_storage_in_orthanc_raw.sh
 ./scripts/check_ftps_upload.py
 
 pixl extract-radiology-reports "${PACKAGE_DIR}/test/resources/omop"

From cd838e46e7c232259af10c8a8acd654ca28c42ba Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 10:58:00 +0000
Subject: [PATCH 41/60] For arguments sake, try polling for images for 4
 minutes

---
 test/scripts/check_ftps_upload.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 403952828..953fddc4d 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -27,7 +27,7 @@
 SECONDS_WAIT = 5
 
 glob_list = []
-for seconds in range(0, 121, SECONDS_WAIT):
+for seconds in range(0, 241, SECONDS_WAIT):
     # Test whether DICOM images have been uploaded
     glob_list = list(expected_output_dir.glob("*.zip"))
     print(f"Waited for {seconds} seconds. glob_list: {glob_list}")

From dcffd58b6978b3d58e84a15cdf639cd074aaef14 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 11:21:56 +0000
Subject: [PATCH 42/60] Increase timeout

---
 test/scripts/check_ftps_upload.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 953fddc4d..f40d943ca 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -27,7 +27,7 @@
 SECONDS_WAIT = 5
 
 glob_list = []
-for seconds in range(0, 241, SECONDS_WAIT):
+for seconds in range(0, 361, SECONDS_WAIT):
     # Test whether DICOM images have been uploaded
     glob_list = list(expected_output_dir.glob("*.zip"))
     print(f"Waited for {seconds} seconds. glob_list: {glob_list}")

From 7c4ab34149e8f8ee9eaf5ddb9f01b49ccf0a1a28 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 11:23:45 +0000
Subject: [PATCH 43/60] Use logger rather than logging directly

---
 orthanc/orthanc-anon/plugin/pixl.py | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py
index 7a4ef0304..cefb9f3b2 100644
--- a/orthanc/orthanc-anon/plugin/pixl.py
+++ b/orthanc/orthanc-anon/plugin/pixl.py
@@ -46,6 +46,8 @@
 ORTHANC_PASSWORD = config("ORTHANC_PASSWORD")
 ORTHANC_URL = "http://localhost:8042"
 
+logger = logging.getLogger(__name__)
+
 
 def AzureAccessToken():
     """
@@ -143,7 +145,7 @@ def SendViaStow(resourceId):
 
     payload = {"Resources": [resourceId], "Synchronous": False}
 
-    logging.info("Payload: %s", payload)
+    logger.info("Payload: %s", payload)
 
     try:
         requests.post(
@@ -162,6 +164,8 @@ def SendViaFTPS(resourceId: str) -> None:
     Makes a POST API call to upload the resource to an FTPS server
     using orthanc credentials as authorisation
     """
+    msg = f"Sending {resourceId} via FTPS"
+    logging.info(msg)
     # Download zip archive of the DICOM resource
     query = f"{ORTHANC_URL}/studies/{resourceId}/archive"
     fail_msg = "Could not download archive of resource '%s'"
@@ -169,10 +173,10 @@ def SendViaFTPS(resourceId: str) -> None:
 
     # get the zip content
     zip_content = response_study.content
-    logging.info("Downloaded data for resource %s", resourceId)
+    logger.info("Downloaded data for resource %s", resourceId)
 
     upload.upload_dicom_image(BytesIO(zip_content), _get_patient_id(resourceId))
-    logging.info("Uploaded data to FTPS for resource %s", resourceId)
+    logger.info("Uploaded data to FTPS for resource %s", resourceId)
 
 
 def _get_patient_id(resourceId: str) -> str:
@@ -206,6 +210,7 @@ def ShouldAutoRoute() -> bool:
     Checks whether ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT environment variable is
     set to true or false
     """
+    logger.debug("Checking value of autoroute")
     return os.environ.get("ORTHANC_AUTOROUTE_ANON_TO_ENDPOINT", "false").lower() == "true"
 
 
@@ -228,7 +233,8 @@ def OnChange(changeType, level, resource):  # noqa: ARG001
         return
 
     if changeType == orthanc.ChangeType.STABLE_STUDY and ShouldAutoRoute():
-        print("Stable study: %s" % resource)  # noqa: T201
+        msg = f"Stable study: {resource}"
+        logger.info(msg)
         SendViaFTPS(resource)
 
     if changeType == orthanc.ChangeType.ORTHANC_STARTED and _azure_available():

From 6ba192699c5c33f1ed5fc61cd68a3c90f02aa675 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 12:34:38 +0000
Subject: [PATCH 44/60] Increase timeout

---
 test/scripts/check_ftps_upload.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index f40d943ca..93fa191ec 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -27,7 +27,7 @@
 SECONDS_WAIT = 5
 
 glob_list = []
-for seconds in range(0, 361, SECONDS_WAIT):
+for seconds in range(0, 601, SECONDS_WAIT):
     # Test whether DICOM images have been uploaded
     glob_list = list(expected_output_dir.glob("*.zip"))
     print(f"Waited for {seconds} seconds. glob_list: {glob_list}")

From 89874f9d21e98de21ce0120a267bcf35d6a5d3d9 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Fri, 2 Feb 2024 13:17:05 +0000
Subject: [PATCH 45/60] Add comment about number of expected studies

---
 test/scripts/check_ftps_upload.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 93fa191ec..9cfb21ce0 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -34,6 +34,7 @@
     if glob_list:
         break
 
+# We expect 2 DICOM image studies to be uploaded
 assert len(glob_list) == 2
 
 # TODO: check parquet files upload

From 4103790270dca9bbb4e8ef568539e547cc2be258 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Fri, 2 Feb 2024 13:22:44 +0000
Subject: [PATCH 46/60] Clean up ftp output directory after check

---
 test/scripts/check_ftps_upload.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 9cfb21ce0..e46f4f54b 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -13,6 +13,7 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 from pathlib import Path
+from shutil import rmtree
 
 PARQUET_PATH = Path(__file__).parents[1] / "resources" / "omop"
 print(f"parquet path: {PARQUET_PATH}")
@@ -34,7 +35,11 @@
     if glob_list:
         break
 
-# We expect 2 DICOM image studies to be uploaded
-assert len(glob_list) == 2
+# Check for expected number of uploaded files and clean up, even if the assertion fails
+try:
+    # We expect 2 DICOM image studies to be uploaded
+    assert len(glob_list) == 2
+finally:
+    rmtree(expected_output_dir)
 
 # TODO: check parquet files upload

From e71b42e99e8722a35d5b567d3f7bb55278657e1f Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Fri, 2 Feb 2024 13:26:15 +0000
Subject: [PATCH 47/60] Actually wait for the desired number of seconds

---
 test/scripts/check_ftps_upload.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index e46f4f54b..b9f8685a1 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -14,6 +14,7 @@
 #  limitations under the License.
 from pathlib import Path
 from shutil import rmtree
+from time import sleep
 
 PARQUET_PATH = Path(__file__).parents[1] / "resources" / "omop"
 print(f"parquet path: {PARQUET_PATH}")
@@ -34,6 +35,7 @@
     print(f"Waited for {seconds} seconds. glob_list: {glob_list}")
     if glob_list:
         break
+    sleep(SECONDS_WAIT)
 
 # Check for expected number of uploaded files and clean up, even if the assertion fails
 try:

From 75cc1d9cbba584e87201db25eb60d85848ffe870 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 13:45:18 +0000
Subject: [PATCH 48/60] Empty-Commit to test building


From d8daa93b6480f128ba22ef9c6b592ad84ae5a29d Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 14:14:53 +0000
Subject: [PATCH 49/60] Fix CI for ftps upload

---
 test/scripts/check_ftps_upload.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index b9f8685a1..43e4d60ed 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -12,6 +12,7 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
+import subprocess
 from pathlib import Path
 from shutil import rmtree
 from time import sleep
@@ -29,7 +30,7 @@
 SECONDS_WAIT = 5
 
 glob_list = []
-for seconds in range(0, 601, SECONDS_WAIT):
+for seconds in range(0, 121, SECONDS_WAIT):
     # Test whether DICOM images have been uploaded
     glob_list = list(expected_output_dir.glob("*.zip"))
     print(f"Waited for {seconds} seconds. glob_list: {glob_list}")
@@ -42,6 +43,7 @@
     # We expect 2 DICOM image studies to be uploaded
     assert len(glob_list) == 2
 finally:
-    rmtree(expected_output_dir)
+    # To we want to always remove the files if its failed, may help debugging not to?
+    rmtree(expected_output_dir, ignore_errors=True)
 
 # TODO: check parquet files upload

From 06db377c19c946fabc160495a85f2cc4827d166c Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 14:23:43 +0000
Subject: [PATCH 50/60] Remove logs check

---
 .github/workflows/main.yml | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index a6c82cf24..a5dd3b952 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -245,9 +245,3 @@ jobs:
         working-directory: test
         run: |
           ./run-system-test.sh
-      - name: Check ftp logs
-        if: always()
-        working-directory: test
-        run: |
-          docker logs system-test-ftp-server &&\
-          docker logs system-test-orthanc-anon-1 --tail=1000

From 71db543adf53c4472ef3af4f8cafcdb27160aed8 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 14:25:59 +0000
Subject: [PATCH 51/60] Reorder system tests to make parquet upload export
 testing easier

---
 test/run-system-test.sh           | 5 +++--
 test/scripts/check_ftps_upload.py | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/test/run-system-test.sh b/test/run-system-test.sh
index c0be0fd80..bff6eb14c 100755
--- a/test/run-system-test.sh
+++ b/test/run-system-test.sh
@@ -35,12 +35,13 @@ pixl start
 # need to wait until the DICOM image is "stable" so poll for 2 minutes to check
 ./scripts/check_entry_in_orthanc_anon_for_2_min.py
 ./scripts/check_entry_in_pixl_anon.sh
-./scripts/check_ftps_upload.py
 
+# test export and upload
 pixl extract-radiology-reports "${PACKAGE_DIR}/test/resources/omop"
-
 ./scripts/check_radiology_parquet.py \
   ../exports/test-extract-uclh-omop-cdm/latest/radiology/radiology.parquet
+./scripts/check_ftps_upload.py
+
 
 ls -laR ../exports/
 docker exec system-test-ehr-api-1 rm -r /run/exports/test-extract-uclh-omop-cdm/
diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 43e4d60ed..8ff3e56b7 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -42,8 +42,8 @@
 try:
     # We expect 2 DICOM image studies to be uploaded
     assert len(glob_list) == 2
+    # TODO: check parquet files upload before deleting
 finally:
     # To we want to always remove the files if its failed, may help debugging not to?
     rmtree(expected_output_dir, ignore_errors=True)
 
-# TODO: check parquet files upload

From ffd53afbf8fff84fc3f66382e4095ed79eb8bd3c Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 14:32:07 +0000
Subject: [PATCH 52/60] Empty-Commit to test building


From 6e399d4a730c3a5662d30863606193f521857465 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 14:46:59 +0000
Subject: [PATCH 53/60] Wait until there are two exported DICOM messages

In case we hit CI when there is only one
---
 test/scripts/check_ftps_upload.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/scripts/check_ftps_upload.py b/test/scripts/check_ftps_upload.py
index 8ff3e56b7..e81a5182f 100755
--- a/test/scripts/check_ftps_upload.py
+++ b/test/scripts/check_ftps_upload.py
@@ -34,7 +34,7 @@
     # Test whether DICOM images have been uploaded
     glob_list = list(expected_output_dir.glob("*.zip"))
     print(f"Waited for {seconds} seconds. glob_list: {glob_list}")
-    if glob_list:
+    if len(glob_list) == 2:
         break
     sleep(SECONDS_WAIT)
 

From bce13f84ec41e5b58c407ac04825eddfd071ce5f Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Fri, 2 Feb 2024 14:51:33 +0000
Subject: [PATCH 54/60] Document FTPS server specifics

---
 docs/services/ftp-server.md |  5 ++++
 test/README.md              | 47 +++++++++++++++++++++++++++----------
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/docs/services/ftp-server.md b/docs/services/ftp-server.md
index 2c33bcc66..8b3a31216 100644
--- a/docs/services/ftp-server.md
+++ b/docs/services/ftp-server.md
@@ -19,3 +19,8 @@ in the `.env` file (see [the example](../.env.sample)).
 For the `pixl_core` unit tests and the system test, we spin up an FTPS server with a Docker
 container, defined in [`test/dummy-services/ftp-server`](../../test/dummy-services/ftp-server/) and
 set the necessary environment variables in [`.env.test`](../../test/.env.test).
+
+## FTPS test server
+
+We provide a Docker container to spin up a test FTPS server. The documentation for this can be found
+in [`test/README.md`](../../test/README.md).
diff --git a/test/README.md b/test/README.md
index 57a359b17..04c87a008 100644
--- a/test/README.md
+++ b/test/README.md
@@ -1,15 +1,13 @@
 # PIXL System Tests
 
-This directory contains a system/integration test that runs locally and aims to
-test the essential functionality of the full PIXL system.
+This directory contains a system/integration test that runs locally and aims to test the essential
+functionality of the full PIXL system.
 
-**Given** a DICOM image in an Orthanc instance (mocked vendor
-neutral archive, VNA) and a single patient with the same identifier in a
-postgres instance (mocked EMAP database, star schema).
-**When** a message containing the patient and study identifier is added to the
-queue and the consumers started.
-**Then** a row in the "anon" EMAP data instance of the PIXL postgres instance exists
-and the DICOM study exists in the "anon" PIXL Orthanc instance.
+**Given** a DICOM image in an Orthanc instance (mocked vendor neutral archive, VNA) and a single
+patient with the same identifier in a postgres instance (mocked EMAP database, star schema).
+**When** a message containing the patient and study identifier is added to the queue and the
+consumers started. **Then** a row in the "anon" EMAP data instance of the PIXL postgres instance
+exists and the DICOM study exists in the "anon" PIXL Orthanc instance.
 
 You can run the system test with:
 
@@ -36,11 +34,36 @@ A test `pixl_config.yml` file is provided to run the PIXL pipeline.
 `./dummy-services` contains a Python script and Dockerfile to mock the CogStack service, used for
 de-identification of the radiology reports in the EHR API.
 
+#### FTP server
+
+We spin up a test FTP server from the Docker container defined in `./dummy-services/ftp-server/`.
+The Docker container inherits from
+[`delfer/alpine-ftp-server`](https://github.com/delfer/docker-alpine-ftp-server) and uses `vsftpd`
+as the FTP client. The Docker container requires the following environment variables to be defined:
+
+-   `ADDRESS`: external address to which clients can connect for passive ports
+-   `USERS`: space and `|` separated list of usernames, passwords, home directories and groups:
+    -   format `name1|password1|[folder1][|uid1][|gid1] name2|password2|[folder2][|uid2][|gid2]`
+    -   the values in `[]` are optional
+-   `TLS_KEY`: keyfile for the TLS certificate
+-   `TLS_CERT`: TLS certificate
+
+> [!warning] The `ADDRESS` should match the `FTP_HOST` environment variable defined in `.env.test`,
+> otherwise FTP commands such as `STOR` or `dir` run from other Docker containers in the network
+> (such as `orthanc-anon`) will fail. _Note: connecting and logging into the FTP server might still
+> work, as the address name is only checked for protected operations such as listing and transfering
+> files._
+
+**Volumes**:
+
+-   To check succesful uploads, we mount a local data directory to `/home/${FTP_USERNAME}/`
+-   The `TLS_*` files should be mounted to the `/etc/ssl/private` directory.
+
 ### Resources
 
-- `./resources/` provides 2 mock DICOM images used to populate the mock VNA.
-- `./resources/omop` contains mock public and private Parquet files used to populate the message
-  queues and extract the radiology reports
+-   `./resources/` provides 2 mock DICOM images used to populate the mock VNA.
+-   `./resources/omop` contains mock public and private Parquet files used to populate the message
+    queues and extract the radiology reports
 
 ### VNA config
 

From 327275f893807ede6181594ab2370131ea8f0b8c Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 15:06:07 +0000
Subject: [PATCH 55/60] Try to make ftp server more robust in CI

---
 .github/workflows/main.yml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index a5dd3b952..3bf9f9a28 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -211,6 +211,11 @@ jobs:
     timeout-minutes: 30
     steps:
       - uses: actions/checkout@v3
+      - name: Prune docker volumes
+        # seems like we're getting an error from ftp-server exiting with zero status code
+        # this is a workaround that resolves it locally 🤞
+        run: |
+          docker volume prune --force
 
       - uses: docker/setup-buildx-action@v3
       # pre-build and cache the postgres container as installing python3 takes a while, doesn't push

From ffa298fa3f4cfa693ed43f026c7cfdbd6f060bce Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 15:56:47 +0000
Subject: [PATCH 56/60] Copy certs into dockerfile instead of mounting

Could this fix our volume error? Who knows
---
 test/docker-compose.yml                                       | 2 --
 test/dummy-services/ftp-server/Dockerfile                     | 2 ++
 test/dummy-services/ftp-server/{mounts => }/ssl/localhost.crt | 0
 test/dummy-services/ftp-server/{mounts => }/ssl/localhost.key | 0
 4 files changed, 2 insertions(+), 2 deletions(-)
 rename test/dummy-services/ftp-server/{mounts => }/ssl/localhost.crt (100%)
 rename test/dummy-services/ftp-server/{mounts => }/ssl/localhost.key (100%)

diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index c119a9661..b8ff98416 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -85,8 +85,6 @@ services:
         volumes:
             # Mount for uploaded data
             - "./dummy-services/ftp-server/mounts/data/:/home/${FTP_USER_NAME}/"
-            # Mount SSL keys for TLS
-            - "./dummy-services/ftp-server/mounts/ssl/:/etc/ssl/private/"
         environment:
             ADDRESS: ${FTP_HOST}
             USERS: ${FTP_USER_NAME}|${FTP_USER_PASSWORD}|/home/${FTP_USER_NAME}
diff --git a/test/dummy-services/ftp-server/Dockerfile b/test/dummy-services/ftp-server/Dockerfile
index be4aeb079..7291e8c95 100644
--- a/test/dummy-services/ftp-server/Dockerfile
+++ b/test/dummy-services/ftp-server/Dockerfile
@@ -13,6 +13,8 @@
 #  limitations under the License.
 FROM delfer/alpine-ftp-server
 
+COPY /dummy-services/ftp-server/ssl/ /etc/ssl/private/
+
 SHELL ["/bin/sh", "-c"]
 RUN echo $'# Attempting to enable FTPS.\n\
 ssl_enable=YES\n\
diff --git a/test/dummy-services/ftp-server/mounts/ssl/localhost.crt b/test/dummy-services/ftp-server/ssl/localhost.crt
similarity index 100%
rename from test/dummy-services/ftp-server/mounts/ssl/localhost.crt
rename to test/dummy-services/ftp-server/ssl/localhost.crt
diff --git a/test/dummy-services/ftp-server/mounts/ssl/localhost.key b/test/dummy-services/ftp-server/ssl/localhost.key
similarity index 100%
rename from test/dummy-services/ftp-server/mounts/ssl/localhost.key
rename to test/dummy-services/ftp-server/ssl/localhost.key

From cd3706a7e9b7e724fd1732185a7f5c723bf6e841 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 15:59:30 +0000
Subject: [PATCH 57/60] Copy certs into dockerfile instead of mounting

Could this fix our volume error? Who knows
---
 test/dummy-services/ftp-server/Dockerfile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/dummy-services/ftp-server/Dockerfile b/test/dummy-services/ftp-server/Dockerfile
index 7291e8c95..dcb3c0617 100644
--- a/test/dummy-services/ftp-server/Dockerfile
+++ b/test/dummy-services/ftp-server/Dockerfile
@@ -13,7 +13,7 @@
 #  limitations under the License.
 FROM delfer/alpine-ftp-server
 
-COPY /dummy-services/ftp-server/ssl/ /etc/ssl/private/
+COPY ssl/ /etc/ssl/private/
 
 SHELL ["/bin/sh", "-c"]
 RUN echo $'# Attempting to enable FTPS.\n\

From 7bdedefd107b516e90653faa788d55a90586b09b Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Fri, 2 Feb 2024 16:05:19 +0000
Subject: [PATCH 58/60] Update core test compose file too

---
 pixl_core/tests/docker-compose.yml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/pixl_core/tests/docker-compose.yml b/pixl_core/tests/docker-compose.yml
index 096992cfd..f14217904 100644
--- a/pixl_core/tests/docker-compose.yml
+++ b/pixl_core/tests/docker-compose.yml
@@ -40,8 +40,6 @@ services:
     volumes:
       # Mount for uploaded data
       - "../../test/dummy-services/ftp-server/mounts/data/:/home/pixl/"
-      # Mount SSL keys for TLS
-      - "../../test/dummy-services/ftp-server/mounts/ssl/:/etc/ssl/private/"
     environment:
       ADDRESS: "localhost"
       USERS: pixl|longpassword|/home/pixl

From 122c253a62b5f0c0ecaa6a4dfb39ff19abb02e56 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Fri, 2 Feb 2024 16:11:24 +0000
Subject: [PATCH 59/60] Put "when" and "then" statements on their own lines

---
 test/README.md | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/test/README.md b/test/README.md
index 04c87a008..a1f8ffdb0 100644
--- a/test/README.md
+++ b/test/README.md
@@ -5,9 +5,12 @@ functionality of the full PIXL system.
 
 **Given** a DICOM image in an Orthanc instance (mocked vendor neutral archive, VNA) and a single
 patient with the same identifier in a postgres instance (mocked EMAP database, star schema).
+
 **When** a message containing the patient and study identifier is added to the queue and the
-consumers started. **Then** a row in the "anon" EMAP data instance of the PIXL postgres instance
-exists and the DICOM study exists in the "anon" PIXL Orthanc instance.
+consumers started.
+
+**Then** a row in the "anon" EMAP data instance of the PIXL postgres instance exists and the DICOM
+study exists in the "anon" PIXL Orthanc instance.
 
 You can run the system test with:
 

From 83bc46e8e82a03a45d06d0ec05d428859a840ae1 Mon Sep 17 00:00:00 2001
From: Milan Malfait <m.malfait@ucl.ac.uk>
Date: Fri, 2 Feb 2024 16:14:16 +0000
Subject: [PATCH 60/60] docs: SSL certificates are now copied instead of
 mounted

---
 test/README.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/README.md b/test/README.md
index a1f8ffdb0..f524ed1d7 100644
--- a/test/README.md
+++ b/test/README.md
@@ -57,10 +57,10 @@ as the FTP client. The Docker container requires the following environment varia
 > work, as the address name is only checked for protected operations such as listing and transfering
 > files._
 
-**Volumes**:
+**Volume**: to check succesful uploads, we mount a local data directory to `/home/${FTP_USERNAME}/`
 
--   To check succesful uploads, we mount a local data directory to `/home/${FTP_USERNAME}/`
--   The `TLS_*` files should be mounted to the `/etc/ssl/private` directory.
+**SSL certifcates**: the SSL certificate files are defined in `test/dummy-services/ftp-server/ssl`
+and are copied into `/etc/ssl/private` when building the Docker container.
 
 ### Resources