From 4d01044d22427de00279752d82be36c09dd0a8f3 Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Fri, 26 Apr 2024 15:40:20 +0100 Subject: [PATCH 01/57] Rework dicomweb --- pixl_core/src/core/uploader/_dicomweb.py | 35 +++++++++++++----------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index b10545ab6..387db0500 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -36,13 +36,15 @@ def __init__(self, project_slug: str, keyvault_alias: Optional[str]) -> None: def _set_config(self) -> None: # Use the Azure KV alias as prefix if it exists, otherwise use the project name az_prefix = self.keyvault_alias - az_prefix = az_prefix if az_prefix else self.project_slug + self.az_prefix = az_prefix if az_prefix else self.project_slug - self.user = self.keyvault.fetch_secret(f"{az_prefix}--dicomweb--username") - self.password = self.keyvault.fetch_secret(f"{az_prefix}--dicomweb--password") + self.orthanc_user = config("ORTHANC_ANON_USERNAME") + self.orthanc_password = config("ORTHANC_ANON_PASSWORD") self.orthanc_url = config("ORTHANC_URL") - self.endpoint_name = self.keyvault.fetch_secret(f"{az_prefix}--dicomweb--url") - self.url = self.orthanc_url + "/dicom-web/servers/" + self.endpoint_name + self.endpoint_user = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--username") + self.endpoint_password = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--password") + self.endpoint_url = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--url") + self.orthanc_dicomweb_url = self.orthanc_url + "/dicom-web/servers/" + self.az_prefix def upload_dicom_image(self) -> None: msg = "Currently not implemented. Use `send_via_stow()` instead." @@ -59,8 +61,8 @@ def send_via_stow(self, resource_id: str) -> requests.Response: try: response = requests.post( - self.url + "/stow", - auth=(self.user, self.password), + self.orthanc_dicomweb_url + "/stow", + auth=(self.orthanc_user, self.orthanc_password), headers=headers, data=json.dumps(payload), timeout=30, @@ -75,7 +77,9 @@ def send_via_stow(self, resource_id: str) -> requests.Response: def _check_dicomweb_server(self) -> bool: """Checks if the dicomweb server exists.""" - response = requests.get(self.url, auth=(self.user, self.password), timeout=30) + response = requests.get( + self.orthanc_dicomweb_url, auth=(self.orthanc_user, self.orthanc_password), timeout=30 + ) success_code = 200 if response.status_code != success_code: return False @@ -87,13 +91,12 @@ def _setup_dicomweb_credentials(self) -> None: This dyniamically creates a new endpoint in Orthanc with the necessary credentials, so we can avoid hardcoding the credentials in the Orthanc configuration at build time. """ - DICOM_ENDPOINT_URL = config("DICOM_ENDPOINT_URL") HTTP_TIMEOUT = int(config("HTTP_TIMEOUT", default=30)) dicomweb_config = { - "Url": DICOM_ENDPOINT_URL, - "Username": self.user, - "Password": self.password, + "Url": self.endpoint_url, + "Username": self.endpoint_user, + "Password": self.endpoint_password, "HasDelete": True, "Timeout": HTTP_TIMEOUT, } @@ -101,17 +104,17 @@ def _setup_dicomweb_credentials(self) -> None: headers = {"content-type": "application/json"} try: requests.put( - self.url, - auth=(self.user, self.password), + self.orthanc_dicomweb_url, + auth=(self.orthanc_user, self.orthanc_password), headers=headers, data=json.dumps(dicomweb_config), timeout=10, ) except requests.exceptions.RequestException: - logger.error("Failed to update DICOMweb token") + logger.error("Failed to update DICOMweb config for {}", self.orthanc_dicomweb_url) raise else: - logger.info("DICOMweb token updated") + logger.info("DICOMweb config for {}", self.orthanc_dicomweb_url) def upload_parquet_files(self) -> None: msg = "DICOMWeb uploader does not support parquet files" From ce6d3cf924d265b779e80b76938e954c3da48884 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Fri, 26 Apr 2024 18:55:00 +0100 Subject: [PATCH 02/57] Update dicomweb tests after rework --- pixl_core/tests/conftest.py | 6 ++- pixl_core/tests/docker-compose.dicomweb.yml | 6 +-- pixl_core/tests/uploader/test_dicomweb.py | 43 +++++++++++++++------ 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py index 771aa2d40..49bf356ab 100644 --- a/pixl_core/tests/conftest.py +++ b/pixl_core/tests/conftest.py @@ -53,9 +53,11 @@ os.environ["ORTHANC_URL"] = "http://localhost:8043" os.environ["ORTHANC_USERNAME"] = "orthanc" os.environ["ORTHANC_PASSWORD"] = "orthanc" # noqa: S105, hardcoded password -os.environ["DICOM_ENDPOINT_NAME"] = "test" # Endpoint for DICOMWeb server as seen from within Orthanc -os.environ["DICOM_ENDPOINT_URL"] = "http://localhost:8042/dicom-web/" +os.environ["DICOM_ENDPOINT_URL"] = "http://localhost:8042/dicom-web" +os.environ["DICOMWEB_USERNAME"] = "orthanc_dicomweb" +os.environ["DICOMWEB_PASSWORD"] = "orthanc_dicomweb" # noqa: S105, hardcoded password +os.environ["DICOMWEB_URL"] = "http://localhost:8044/dicom-web" @pytest.fixture(scope="package") diff --git a/pixl_core/tests/docker-compose.dicomweb.yml b/pixl_core/tests/docker-compose.dicomweb.yml index 620a4def1..392aaf65d 100644 --- a/pixl_core/tests/docker-compose.dicomweb.yml +++ b/pixl_core/tests/docker-compose.dicomweb.yml @@ -47,12 +47,12 @@ services: platform: linux/amd64 environment: ORTHANC_NAME: "dicomweb" - ORTHANC_USERNAME: "orthanc" - ORTHANC_PASSWORD: "orthanc" + ORTHANC_USERNAME: "orthanc_dicomweb" + ORTHANC_PASSWORD: "orthanc_dicomweb" ORTHANC_AE_TITLE: "DICOMWEB" RAW_AE_TITLE: ORHTANCRAW RAW_DICOM_PORT: "4242" - RAW_IP_ADDR: "orthanc-raw" # aka. hostname + RAW_IP_ADDR: "dicom-web" # aka. hostname DICOM_WEB_PLUGIN_ENABLED: true ports: - "4244:4242" diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index a8c56ef21..0f946a13f 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -23,9 +23,11 @@ from decouple import config # type: ignore [import-untyped] ORTHANC_URL = config("ORTHANC_URL") -DICOM_ENDPOINT_NAME = config("DICOM_ENDPOINT_NAME") ORTHANC_USERNAME = config("ORTHANC_USERNAME") ORTHANC_PASSWORD = config("ORTHANC_PASSWORD") +DICOMWEB_USERNAME = config("DICOMWEB_USERNAME") +DICOMWEB_PASSWORD = config("DICOMWEB_PASSWORD") +DICOMWEB_URL = config("DICOMWEB_URL") class MockDicomWebUploader(DicomWebUploader): @@ -33,11 +35,14 @@ class MockDicomWebUploader(DicomWebUploader): def __init__(self) -> None: """Initialise the mock uploader.""" - self.user = ORTHANC_USERNAME - self.password = ORTHANC_PASSWORD - self.endpoint_name = DICOM_ENDPOINT_NAME + self.az_prefix = "test" + self.orthanc_user = ORTHANC_USERNAME + self.orthanc_password = ORTHANC_PASSWORD self.orthanc_url = ORTHANC_URL - self.url = self.orthanc_url + "/dicom-web/servers/" + self.endpoint_name + self.endpoint_user = DICOMWEB_USERNAME + self.endpoint_password = DICOMWEB_PASSWORD + self.endpoint_url = DICOMWEB_URL + self.orthanc_dicomweb_url = self.orthanc_url + "/dicom-web/servers/" + self.az_prefix @pytest.fixture() @@ -46,29 +51,43 @@ def dicomweb_uploader() -> MockDicomWebUploader: return MockDicomWebUploader() -def _do_get_request(endpoint: str, data: Optional[dict] = None) -> requests.Response: +def _do_get_request(url: str, data: Optional[dict] = None) -> requests.Response: """Perform a GET request to the specified endpoint.""" return requests.get( - ORTHANC_URL + endpoint, + url, auth=(ORTHANC_USERNAME, ORTHANC_PASSWORD), data=data, timeout=30, ) +def test_dicomweb_server_config(run_dicomweb_containers, dicomweb_uploader) -> None: + """Tests that the DICOMWeb server is configured correctly in Orthanc""" + dicomweb_uploader._setup_dicomweb_credentials() # noqa: SLF001, private method + servers_response = requests.get( + ORTHANC_URL + "/dicom-web/servers", + auth=(ORTHANC_USERNAME, ORTHANC_PASSWORD), + timeout=30, + ) + + assert "test" in servers_response.json() + + def test_upload_dicom_image(study_id, run_dicomweb_containers, dicomweb_uploader) -> None: """Tests that DICOM image can be uploaded to a DICOMWeb server""" - # ARRANGE - # ACT stow_response = dicomweb_uploader.send_via_stow(study_id) - studies_response = _do_get_request("/dicom-web/studies", data={"Uri": "/instances"}) - servers_response = _do_get_request("/dicom-web/servers") + studies_response = requests.get( + DICOMWEB_URL + "/studies", + auth=(DICOMWEB_USERNAME, DICOMWEB_PASSWORD), + data={"Uri": "/instances"}, + timeout=30, + ) # ASSERT # Check if dicom-web server is set up correctly - assert DICOM_ENDPOINT_NAME in servers_response.json() assert stow_response.status_code == 200 # succesful upload # Taken from https://orthanc.uclouvain.be/hg/orthanc-dicomweb/file/default/Resources/Samples/Python/SendStow.py # Check that instance has not been discarded + assert studies_response.status_code == 200 assert "00081190" in studies_response.json()[0] From d224ea5e7a2acb64e429002d1d76b41af5fdfe1f Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 09:54:32 +0200 Subject: [PATCH 03/57] Docker compose `version` is obsolete --- pixl_core/tests/docker-compose.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pixl_core/tests/docker-compose.yml b/pixl_core/tests/docker-compose.yml index 98f9ebc24..0af1d79a9 100644 --- a/pixl_core/tests/docker-compose.yml +++ b/pixl_core/tests/docker-compose.yml @@ -11,9 +11,6 @@ # 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. -version: "3.8" - - services: queue: From 19ec2c1cabb471c5f3c7e5b2979aa75d5bcea446 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 12:40:28 +0200 Subject: [PATCH 04/57] Fix dicomweb-server healthcheck; use correct credentials --- pixl_core/tests/docker-compose.dicomweb.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pixl_core/tests/docker-compose.dicomweb.yml b/pixl_core/tests/docker-compose.dicomweb.yml index 392aaf65d..ac194483f 100644 --- a/pixl_core/tests/docker-compose.dicomweb.yml +++ b/pixl_core/tests/docker-compose.dicomweb.yml @@ -62,7 +62,11 @@ services: networks: - pixl-test healthcheck: - test: ["CMD-SHELL", "/probes/test-aliveness.py --user=orthanc --pwd=orthanc"] + test: + [ + "CMD-SHELL", + "/probes/test-aliveness.py --user=orthanc_dicomweb --pwd=orthanc_dicomweb", + ] start_period: 10s retries: 2 interval: 3s From bdbb1dfe995eaff58cce2367bcfd6b719e1b861b Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 12:40:53 +0200 Subject: [PATCH 05/57] Fix the DicomWebUploader tests --- pixl_core/tests/conftest.py | 10 +++++----- pixl_core/tests/uploader/test_dicomweb.py | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py index 49bf356ab..35640ed2b 100644 --- a/pixl_core/tests/conftest.py +++ b/pixl_core/tests/conftest.py @@ -54,10 +54,9 @@ os.environ["ORTHANC_USERNAME"] = "orthanc" os.environ["ORTHANC_PASSWORD"] = "orthanc" # noqa: S105, hardcoded password # Endpoint for DICOMWeb server as seen from within Orthanc -os.environ["DICOM_ENDPOINT_URL"] = "http://localhost:8042/dicom-web" +os.environ["DICOMWEB_URL"] = "http://dicomweb-server:8042/dicom-web" os.environ["DICOMWEB_USERNAME"] = "orthanc_dicomweb" os.environ["DICOMWEB_PASSWORD"] = "orthanc_dicomweb" # noqa: S105, hardcoded password -os.environ["DICOMWEB_URL"] = "http://localhost:8044/dicom-web" @pytest.fixture(scope="package") @@ -86,18 +85,19 @@ def run_dicomweb_containers() -> Generator[subprocess.CompletedProcess[bytes], N Spins up 2 Orthanc containers, one that acts as the base storage, mimicking our orthanc-anon or orthanc-raw servers, and the other one as a DICOMweb server to upload DICOM files to. """ + docker_compose_file = TEST_DIR / "docker-compose.dicomweb.yml" run_subprocess( - shlex.split("docker compose down --volumes"), + shlex.split(f"docker compose -f {docker_compose_file} down --volumes"), TEST_DIR, timeout=60, ) yield run_subprocess( - shlex.split("docker compose -f docker-compose.dicomweb.yml up --build --wait"), + shlex.split(f"docker compose -f {docker_compose_file} up --build --wait"), TEST_DIR, timeout=60, ) run_subprocess( - shlex.split("docker compose down --volumes"), + shlex.split(f"docker compose -f {docker_compose_file} down --volumes"), TEST_DIR, timeout=60, ) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 0f946a13f..79d2293b4 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -15,6 +15,7 @@ from __future__ import annotations +import time from typing import Optional import pytest @@ -29,6 +30,8 @@ DICOMWEB_PASSWORD = config("DICOMWEB_PASSWORD") DICOMWEB_URL = config("DICOMWEB_URL") +LOCOL_DICOMWEB_URL = "http://localhost:8044" + class MockDicomWebUploader(DicomWebUploader): """Mock DicomWebUploader for testing.""" @@ -75,19 +78,16 @@ def test_dicomweb_server_config(run_dicomweb_containers, dicomweb_uploader) -> N def test_upload_dicom_image(study_id, run_dicomweb_containers, dicomweb_uploader) -> None: """Tests that DICOM image can be uploaded to a DICOMWeb server""" - # ACT stow_response = dicomweb_uploader.send_via_stow(study_id) + + # Check that the instance has arrived on the DICOMweb server + time.sleep(5) studies_response = requests.get( - DICOMWEB_URL + "/studies", + LOCOL_DICOMWEB_URL + "/studies", auth=(DICOMWEB_USERNAME, DICOMWEB_PASSWORD), - data={"Uri": "/instances"}, timeout=30, ) - # ASSERT - # Check if dicom-web server is set up correctly assert stow_response.status_code == 200 # succesful upload - # Taken from https://orthanc.uclouvain.be/hg/orthanc-dicomweb/file/default/Resources/Samples/Python/SendStow.py - # Check that instance has not been discarded assert studies_response.status_code == 200 - assert "00081190" in studies_response.json()[0] + assert len(studies_response.json()) == 1 From 2a9105464aeb1aa43986b7523bae962805bab216 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 13:01:17 +0200 Subject: [PATCH 06/57] Simplify services setup for core tests --- pixl_core/tests/conftest.py | 26 +------ pixl_core/tests/docker-compose.dicomweb.yml | 73 ------------------ pixl_core/tests/docker-compose.yml | 84 +++++++++++++++++---- pixl_core/tests/uploader/test_dicomweb.py | 4 +- 4 files changed, 73 insertions(+), 114 deletions(-) delete mode 100644 pixl_core/tests/docker-compose.dicomweb.yml diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py index 35640ed2b..009490b4e 100644 --- a/pixl_core/tests/conftest.py +++ b/pixl_core/tests/conftest.py @@ -80,31 +80,7 @@ def run_containers() -> Generator[subprocess.CompletedProcess[bytes], None, None @pytest.fixture(scope="package") -def run_dicomweb_containers() -> Generator[subprocess.CompletedProcess[bytes], None, None]: - """ - Spins up 2 Orthanc containers, one that acts as the base storage, mimicking our orthanc-anon - or orthanc-raw servers, and the other one as a DICOMweb server to upload DICOM files to. - """ - docker_compose_file = TEST_DIR / "docker-compose.dicomweb.yml" - run_subprocess( - shlex.split(f"docker compose -f {docker_compose_file} down --volumes"), - TEST_DIR, - timeout=60, - ) - yield run_subprocess( - shlex.split(f"docker compose -f {docker_compose_file} up --build --wait"), - TEST_DIR, - timeout=60, - ) - run_subprocess( - shlex.split(f"docker compose -f {docker_compose_file} down --volumes"), - TEST_DIR, - timeout=60, - ) - - -@pytest.fixture(scope="package") -def study_id(run_dicomweb_containers) -> str: +def study_id(run_containers) -> str: """Uploads a DICOM file to the Orthanc server and returns the study ID.""" DCM_FILE = Path(__file__).parents[2] / "test" / "resources" / "Dicom1.dcm" ORTHANC_URL = os.environ["ORTHANC_URL"] diff --git a/pixl_core/tests/docker-compose.dicomweb.yml b/pixl_core/tests/docker-compose.dicomweb.yml deleted file mode 100644 index ac194483f..000000000 --- a/pixl_core/tests/docker-compose.dicomweb.yml +++ /dev/null @@ -1,73 +0,0 @@ -# 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. - -networks: - pixl-test: - -services: - orthanc: - image: orthancteam/orthanc:24.4.0 - platform: linux/amd64 - environment: - ORTHANC_NAME: "orthanc" - ORTHANC_USERNAME: "orthanc" - ORTHANC_PASSWORD: "orthanc" - ORTHANC_AE_TITLE: "orthanc" - RAW_AE_TITLE: ORHTANCRAW - RAW_DICOM_PORT: "4242" - RAW_IP_ADDR: "orthanc-raw" # aka. hostname - DICOM_WEB_PLUGIN_ENABLED: true - ports: - - "4243:4242" - - "8043:8042" - volumes: - - ${PWD}/dicomweb_config/:/run/secrets:ro - networks: - - pixl-test - healthcheck: - test: ["CMD-SHELL", "/probes/test-aliveness.py --user=orthanc --pwd=orthanc"] - start_period: 10s - retries: 2 - interval: 3s - timeout: 2s - - dicomweb-server: - image: orthancteam/orthanc:24.4.0 - platform: linux/amd64 - environment: - ORTHANC_NAME: "dicomweb" - ORTHANC_USERNAME: "orthanc_dicomweb" - ORTHANC_PASSWORD: "orthanc_dicomweb" - ORTHANC_AE_TITLE: "DICOMWEB" - RAW_AE_TITLE: ORHTANCRAW - RAW_DICOM_PORT: "4242" - RAW_IP_ADDR: "dicom-web" # aka. hostname - DICOM_WEB_PLUGIN_ENABLED: true - ports: - - "4244:4242" - - "8044:8042" - volumes: - - ${PWD}/../../test/dicomweb_config/:/run/secrets:ro - networks: - - pixl-test - healthcheck: - test: - [ - "CMD-SHELL", - "/probes/test-aliveness.py --user=orthanc_dicomweb --pwd=orthanc_dicomweb", - ] - start_period: 10s - retries: 2 - interval: 3s - timeout: 2s diff --git a/pixl_core/tests/docker-compose.yml b/pixl_core/tests/docker-compose.yml index 0af1d79a9..15fed9e29 100644 --- a/pixl_core/tests/docker-compose.yml +++ b/pixl_core/tests/docker-compose.yml @@ -11,19 +11,75 @@ # 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. +networks: + pixl-test: + services: + queue: + container_name: pixl-test-queue + image: rabbitmq:3.12.9-management + environment: + RABBITMQ_DEFAULT_USER: guest + RABBITMQ_DEFAULT_PASS: guest + ports: + - "25672:5672" + - "35672:15672" + healthcheck: + test: rabbitmq-diagnostics -q check_running + interval: 10s + timeout: 5s + retries: 5 + + orthanc: + image: orthancteam/orthanc:24.4.0 + platform: linux/amd64 + environment: + ORTHANC_NAME: "orthanc" + ORTHANC_USERNAME: "orthanc" + ORTHANC_PASSWORD: "orthanc" + ORTHANC_AE_TITLE: "orthanc" + RAW_AE_TITLE: ORHTANCRAW + RAW_DICOM_PORT: "4242" + RAW_IP_ADDR: "orthanc-raw" # aka. hostname + DICOM_WEB_PLUGIN_ENABLED: true + ports: + - "4243:4242" + - "8043:8042" + networks: + - pixl-test + healthcheck: + test: ["CMD-SHELL", "/probes/test-aliveness.py --user=orthanc --pwd=orthanc"] + start_period: 10s + retries: 2 + interval: 3s + timeout: 2s - queue: - container_name: pixl-test-queue - image: rabbitmq:3.12.9-management - environment: - RABBITMQ_DEFAULT_USER: guest - RABBITMQ_DEFAULT_PASS: guest - ports: - - "25672:5672" - - "35672:15672" - healthcheck: - test: rabbitmq-diagnostics -q check_running - interval: 10s - timeout: 5s - retries: 5 + dicomweb-server: + image: orthancteam/orthanc:24.4.0 + platform: linux/amd64 + environment: + ORTHANC_NAME: "dicomweb" + ORTHANC_USERNAME: "orthanc_dicomweb" + ORTHANC_PASSWORD: "orthanc_dicomweb" + ORTHANC_AE_TITLE: "DICOMWEB" + RAW_AE_TITLE: ORHTANCRAW + RAW_DICOM_PORT: "4242" + RAW_IP_ADDR: "dicom-web" # aka. hostname + DICOM_WEB_PLUGIN_ENABLED: true + ports: + - "4244:4242" + - "8044:8042" + volumes: + - ${PWD}/../../test/dicomweb_config/:/run/secrets:ro + networks: + - pixl-test + healthcheck: + test: + [ + "CMD-SHELL", + "/probes/test-aliveness.py --user=orthanc_dicomweb --pwd=orthanc_dicomweb", + ] + start_period: 10s + retries: 2 + interval: 3s + timeout: 2s diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 79d2293b4..0aa7256d1 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -64,7 +64,7 @@ def _do_get_request(url: str, data: Optional[dict] = None) -> requests.Response: ) -def test_dicomweb_server_config(run_dicomweb_containers, dicomweb_uploader) -> None: +def test_dicomweb_server_config(run_containers, dicomweb_uploader) -> None: """Tests that the DICOMWeb server is configured correctly in Orthanc""" dicomweb_uploader._setup_dicomweb_credentials() # noqa: SLF001, private method servers_response = requests.get( @@ -76,7 +76,7 @@ def test_dicomweb_server_config(run_dicomweb_containers, dicomweb_uploader) -> N assert "test" in servers_response.json() -def test_upload_dicom_image(study_id, run_dicomweb_containers, dicomweb_uploader) -> None: +def test_upload_dicom_image(study_id, run_containers, dicomweb_uploader) -> None: """Tests that DICOM image can be uploaded to a DICOMWeb server""" stow_response = dicomweb_uploader.send_via_stow(study_id) From 70182afb3e3a7857bc701f7980e374dcefc21eae Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 13:29:43 +0200 Subject: [PATCH 07/57] Fix volume path Make the path relative to where the Docker compose file lives, not to the working directory. So we can run the tests from any aribtrary directory. --- pixl_core/tests/docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixl_core/tests/docker-compose.yml b/pixl_core/tests/docker-compose.yml index 15fed9e29..cd3d1155c 100644 --- a/pixl_core/tests/docker-compose.yml +++ b/pixl_core/tests/docker-compose.yml @@ -70,7 +70,7 @@ services: - "4244:4242" - "8044:8042" volumes: - - ${PWD}/../../test/dicomweb_config/:/run/secrets:ro + - ../../test/dicomweb_config/:/run/secrets:ro networks: - pixl-test healthcheck: From 2f6ff49643358460ee2d98b085725040f0071332 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 13:30:54 +0200 Subject: [PATCH 08/57] Clean up orphan containers at startup Gets rid of some warnings --- pixl_core/tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py index 009490b4e..f626d3a7f 100644 --- a/pixl_core/tests/conftest.py +++ b/pixl_core/tests/conftest.py @@ -68,7 +68,7 @@ def run_containers() -> Generator[subprocess.CompletedProcess[bytes], None, None timeout=60, ) yield run_subprocess( - shlex.split("docker compose up --build --wait"), + shlex.split("docker compose up --build --wait --remove-orphans"), TEST_DIR, timeout=60, ) From 262ffeeaaa274265e689d6cb09266dea7359064c Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 13:35:07 +0200 Subject: [PATCH 09/57] Fix typo --- pixl_core/tests/uploader/test_dicomweb.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 0aa7256d1..4ef233581 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -30,7 +30,7 @@ DICOMWEB_PASSWORD = config("DICOMWEB_PASSWORD") DICOMWEB_URL = config("DICOMWEB_URL") -LOCOL_DICOMWEB_URL = "http://localhost:8044" +LOCAL_DICOMWEB_URL = "http://localhost:8044" class MockDicomWebUploader(DicomWebUploader): @@ -81,9 +81,9 @@ def test_upload_dicom_image(study_id, run_containers, dicomweb_uploader) -> None stow_response = dicomweb_uploader.send_via_stow(study_id) # Check that the instance has arrived on the DICOMweb server - time.sleep(5) + time.sleep(2) studies_response = requests.get( - LOCOL_DICOMWEB_URL + "/studies", + LOCAL_DICOMWEB_URL + "/studies", auth=(DICOMWEB_USERNAME, DICOMWEB_PASSWORD), timeout=30, ) From be155761e423180286ea1b24299ac1c1ea078e06 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 13:51:45 +0200 Subject: [PATCH 10/57] Add negative checks in case of wrong DICOMweb credentials --- pixl_core/tests/uploader/test_dicomweb.py | 44 ++++++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 4ef233581..3bc2103d1 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -76,18 +76,52 @@ def test_dicomweb_server_config(run_containers, dicomweb_uploader) -> None: assert "test" in servers_response.json() +def _check_study_present_on_dicomweb(study_id: str) -> bool: + """Check if a study is present on the DICOMWeb server.""" + response = requests.get( + LOCAL_DICOMWEB_URL + "/studies", + auth=(DICOMWEB_USERNAME, DICOMWEB_PASSWORD), + timeout=30, + ) + return any(study == study_id for study in response.json()) + + def test_upload_dicom_image(study_id, run_containers, dicomweb_uploader) -> None: """Tests that DICOM image can be uploaded to a DICOMWeb server""" stow_response = dicomweb_uploader.send_via_stow(study_id) + assert stow_response.status_code == 200 # succesful upload # Check that the instance has arrived on the DICOMweb server time.sleep(2) - studies_response = requests.get( - LOCAL_DICOMWEB_URL + "/studies", + assert _check_study_present_on_dicomweb(study_id) + + # Clean up + requests.delete( + LOCAL_DICOMWEB_URL + "/studies/" + study_id, auth=(DICOMWEB_USERNAME, DICOMWEB_PASSWORD), timeout=30, ) - assert stow_response.status_code == 200 # succesful upload - assert studies_response.status_code == 200 - assert len(studies_response.json()) == 1 + +def test_dicomweb_upload_fails_with_wrong_credentials( + study_id, run_containers, dicomweb_uploader +) -> None: + """Tests that the DICOMWeb uploader fails when given wrong credentials.""" + dicomweb_uploader.endpoint_user = "wrong" + dicomweb_uploader.endpoint_password = "wrong" # noqa: S105, hardcoded password + + # Force updating of the credentials, so we don't need to restart the container + dicomweb_uploader._setup_dicomweb_credentials() # noqa: SLF001, private method + + dicomweb_uploader.send_via_stow(study_id) + assert not _check_study_present_on_dicomweb(study_id) + + +def test_dicomweb_upload_fails_with_wrong_url(study_id, run_containers, dicomweb_uploader) -> None: + """Tests that the DICOMWeb uploader fails when given wrong URL.""" + dicomweb_uploader.endpoint_url = "wrong" + # Force updating of the credentials, so we don't need to restart the container + dicomweb_uploader._setup_dicomweb_credentials() # noqa: SLF001, private method + + dicomweb_uploader.send_via_stow(study_id) + assert not _check_study_present_on_dicomweb(study_id) From 0e1aa3a88ed32cc1f0c7bcfef1f9504acbbc4653 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 15:39:01 +0200 Subject: [PATCH 11/57] Remove obsolete function --- pixl_core/tests/uploader/test_dicomweb.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 3bc2103d1..ffcdca295 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -16,7 +16,6 @@ from __future__ import annotations import time -from typing import Optional import pytest import requests @@ -54,16 +53,6 @@ def dicomweb_uploader() -> MockDicomWebUploader: return MockDicomWebUploader() -def _do_get_request(url: str, data: Optional[dict] = None) -> requests.Response: - """Perform a GET request to the specified endpoint.""" - return requests.get( - url, - auth=(ORTHANC_USERNAME, ORTHANC_PASSWORD), - data=data, - timeout=30, - ) - - def test_dicomweb_server_config(run_containers, dicomweb_uploader) -> None: """Tests that the DICOMWeb server is configured correctly in Orthanc""" dicomweb_uploader._setup_dicomweb_credentials() # noqa: SLF001, private method From a0c7f50dc2630ed7dd2e949684cba78568555536 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 16:22:45 +0200 Subject: [PATCH 12/57] Fix orthanc credentials These need to use the mapped environment variables inside the Orthanc container --- pixl_core/src/core/uploader/_dicomweb.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index 387db0500..3b34e8960 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -38,8 +38,8 @@ def _set_config(self) -> None: az_prefix = self.keyvault_alias self.az_prefix = az_prefix if az_prefix else self.project_slug - self.orthanc_user = config("ORTHANC_ANON_USERNAME") - self.orthanc_password = config("ORTHANC_ANON_PASSWORD") + self.orthanc_user = config("ORTHANC_USERNAME") + self.orthanc_password = config("ORTHANC_PASSWORD") self.orthanc_url = config("ORTHANC_URL") self.endpoint_user = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--username") self.endpoint_password = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--password") From baf54e97d9c159ab46421132c16cc944edcf3f50 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 16:23:10 +0200 Subject: [PATCH 13/57] Use different credentials for DICOMweb test server and fix volume paths --- test/docker-compose.yml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/docker-compose.yml b/test/docker-compose.yml index e4fa41db2..acae7d0b9 100644 --- a/test/docker-compose.yml +++ b/test/docker-compose.yml @@ -35,7 +35,7 @@ services: - "4243:4242" - "8043:8042" volumes: - - ${PWD}/vna_config/:/run/secrets:ro + - ./vna_config/:/run/secrets:ro healthcheck: test: ["CMD-SHELL", "/probes/test-aliveness.py --user=orthanc --pwd=orthanc"] start_period: 10s @@ -82,8 +82,8 @@ services: platform: linux/amd64 environment: ORTHANC_NAME: "dicomweb" - ORTHANC_USERNAME: "orthanc" - ORTHANC_PASSWORD: "orthanc" + ORTHANC_USERNAME: "orthanc_dicomweb" + ORTHANC_PASSWORD: "orthanc_dicomweb" ORTHANC_AE_TITLE: "DICOMWEB" RAW_AE_TITLE: ORHTANCRAW RAW_DICOM_PORT: "4242" @@ -93,11 +93,15 @@ services: - "4244:4242" - "8044:8042" volumes: - - ${PWD}/dicomweb_config/:/run/secrets:ro + - ./dicomweb_config/:/run/secrets:ro networks: pixl-net: healthcheck: - test: ["CMD-SHELL", "/probes/test-aliveness.py --user=orthanc --pwd=orthanc"] + test: + [ + "CMD-SHELL", + "/probes/test-aliveness.py --user=orthanc_dicomweb --pwd=orthanc_dicomweb", + ] start_period: 10s retries: 2 interval: 3s From 53fb47f593023cfd10b845ea0b4dc9fe9aab5f79 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 16:23:41 +0200 Subject: [PATCH 14/57] Fix system test: actually check for presence of studies on DICOMweb --- test/system_test.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/test/system_test.py b/test/system_test.py index 05e2c4bb3..1a2dbe282 100644 --- a/test/system_test.py +++ b/test/system_test.py @@ -109,7 +109,10 @@ def two_zip_files_present() -> bool: self._check_dcm_tags_from_zip(z, unzip_dir, expected_studies) def _check_dcm_tags_from_zip( - self, zip_path: Path, unzip_dir: Path, expected_studies: dict[str, set[tuple[str, str]]] + self, + zip_path: Path, + unzip_dir: Path, + expected_studies: dict[str, set[tuple[str, str]]], ) -> None: """Check that private tag has survived anonymisation with the correct value.""" expected_instances = expected_studies[zip_path.stem] @@ -176,16 +179,26 @@ def exists_two_rows() -> bool: def test_dicomweb_upload() -> None: """Check upload to DICOMweb server was successful""" # This should point to the orthanc-anon server - ORTHANC_URL = "http://localhost:7003" + LOCAL_DICOMWEB_URL = "http://localhost:8044" - def check_dicomweb_study_present() -> bool: + dicomweb_studies: list[str] = [] + + def dicomweb_studies_list() -> str: + return f"DICOMweb studies found: {dicomweb_studies}" + + def two_studies_present_on_dicomweb() -> bool: + nonlocal dicomweb_studies response = requests.get( - ORTHANC_URL + "/dicom-web/studies", - auth=("orthanc_anon_username", "orthanc_anon_password"), - data={"Uri": "/instances"}, + LOCAL_DICOMWEB_URL + "/studies", + auth=("orthanc_dicomweb", "orthanc_dicomweb"), timeout=30, ) - # Taken from https://orthanc.uclouvain.be/hg/orthanc-dicomweb/file/default/Resources/Samples/Python/SendStow.py - return response.status_code == 200 and "00081190" in response.json()[0] - - wait_for_condition(check_dicomweb_study_present) + dicomweb_studies = response.json() + return len(dicomweb_studies) == 2 + + wait_for_condition( + two_studies_present_on_dicomweb, + seconds_max=121, + seconds_interval=10, + progress_string_fn=dicomweb_studies_list, + ) From 95145217c0c33459684a6b2fd01df843a344dd87 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 16:55:21 +0200 Subject: [PATCH 15/57] Better log message --- pixl_core/src/core/uploader/_dicomweb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index 3b34e8960..eb233d248 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -114,7 +114,7 @@ def _setup_dicomweb_credentials(self) -> None: logger.error("Failed to update DICOMweb config for {}", self.orthanc_dicomweb_url) raise else: - logger.info("DICOMweb config for {}", self.orthanc_dicomweb_url) + logger.info("Set up DICOMweb config for {}", self.orthanc_dicomweb_url) def upload_parquet_files(self) -> None: msg = "DICOMWeb uploader does not support parquet files" From ebfbe1b8f027fcf0c5a7ab4815a2e175e1b5731f Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 19:46:21 +0200 Subject: [PATCH 16/57] Fix module docstring --- pixl_core/src/core/uploader/_dicomweb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index eb233d248..8797ca7e7 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Uploader subclass for FTPS uploads.""" +"""Uploader subclass for DICOMweb uploads.""" from __future__ import annotations From 3215d3a24ee33d3ca2386eb158e1014ec2741020 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 19:50:27 +0200 Subject: [PATCH 17/57] Add clarifying comment Co-authored-by: Jeremy Stein --- pixl_core/src/core/uploader/_dicomweb.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index 8797ca7e7..3cfe04503 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -44,6 +44,7 @@ def _set_config(self) -> None: self.endpoint_user = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--username") self.endpoint_password = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--password") self.endpoint_url = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--url") + # the url of the DicomWeb service on orthanc anon (not to be confused with the downstream DicomWeb server that we're trying to upload to) self.orthanc_dicomweb_url = self.orthanc_url + "/dicom-web/servers/" + self.az_prefix def upload_dicom_image(self) -> None: From 0a05664de6ad1e2ecfbe168d54b6d48fdd1a51d1 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Mon, 29 Apr 2024 19:54:39 +0200 Subject: [PATCH 18/57] Use lowercase for local variable Co-authored-by: Jeremy Stein --- pixl_core/src/core/uploader/_dicomweb.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index 3cfe04503..c5a5b8a7e 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -92,14 +92,14 @@ def _setup_dicomweb_credentials(self) -> None: This dyniamically creates a new endpoint in Orthanc with the necessary credentials, so we can avoid hardcoding the credentials in the Orthanc configuration at build time. """ - HTTP_TIMEOUT = int(config("HTTP_TIMEOUT", default=30)) + http_timeout = int(config("HTTP_TIMEOUT", default=30)) dicomweb_config = { "Url": self.endpoint_url, "Username": self.endpoint_user, "Password": self.endpoint_password, "HasDelete": True, - "Timeout": HTTP_TIMEOUT, + "Timeout": http_timeout, } headers = {"content-type": "application/json"} From 59e36b8bf0a3b078259849774bf316d69c8f8b02 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 09:20:07 +0200 Subject: [PATCH 19/57] Fix outdated comment --- test/system_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/system_test.py b/test/system_test.py index 1a2dbe282..af5f91dd8 100644 --- a/test/system_test.py +++ b/test/system_test.py @@ -178,7 +178,7 @@ def exists_two_rows() -> bool: @pytest.mark.usefixtures("_setup_pixl_cli_dicomweb") def test_dicomweb_upload() -> None: """Check upload to DICOMweb server was successful""" - # This should point to the orthanc-anon server + # This should point to the dicomweb server, as seen from the local host machine LOCAL_DICOMWEB_URL = "http://localhost:8044" dicomweb_studies: list[str] = [] From b840885edcbe33e3570c6cf54f8b1f314621ccfa Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 09:31:28 +0200 Subject: [PATCH 20/57] Docker compose 'version is obsolete' https://docs.docker.com/compose/compose-file/04-version-and-name/ --- docker-compose.yml | 2 -- test/docker-compose.yml | 2 -- 2 files changed, 4 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 32d8978df..0168800b8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -11,8 +11,6 @@ # 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. -version: "3.8" - ################################################################################ # Common diff --git a/test/docker-compose.yml b/test/docker-compose.yml index acae7d0b9..7b4d1da59 100644 --- a/test/docker-compose.yml +++ b/test/docker-compose.yml @@ -11,8 +11,6 @@ # 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. -version: "3.8" - volumes: vna-qr-data: From 131ba35b0159ba0a5db73ca5d04b8b8a9324ad19 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 09:31:47 +0200 Subject: [PATCH 21/57] Formatting: 4-space indents --- docker-compose.yml | 626 +++++++++++++++++++++++---------------------- 1 file changed, 314 insertions(+), 312 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 0168800b8..34d61ef22 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,343 +18,345 @@ x-http-proxy: &http-proxy ${HTTP_PROXY} x-https-proxy: &https-proxy ${HTTPS_PROXY} x-no-proxy: &no-proxy localhost,0.0.0.0,127.0.0.1,uclvlddpragae07,hasher-api,orthanc-raw x-proxy-common: &proxy-common - HTTP_PROXY: *http-proxy - http_proxy: *http-proxy - HTTPS_PROXY: *https-proxy - https_proxy: *https-proxy - NO_PROXY: *no-proxy - no_proxy: *no-proxy + HTTP_PROXY: *http-proxy + http_proxy: *http-proxy + HTTPS_PROXY: *https-proxy + https_proxy: *https-proxy + NO_PROXY: *no-proxy + no_proxy: *no-proxy x-build-args-common: &build-args-common - <<: [*proxy-common] + <<: [*proxy-common] x-pixl-common-env: &pixl-common-env - DEBUG: ${DEBUG} - LOG_LEVEL: ${LOG_LEVEL} + DEBUG: ${DEBUG} + LOG_LEVEL: ${LOG_LEVEL} x-pixl-rabbit-mq: &pixl-rabbit-mq - RABBITMQ_HOST: "queue" # Name of the queue service - RABBITMQ_PORT: "5672" - RABBITMQ_USERNAME: ${RABBITMQ_USERNAME} - RABBITMQ_PASSWORD: ${RABBITMQ_PASSWORD} + RABBITMQ_HOST: "queue" # Name of the queue service + RABBITMQ_PORT: "5672" + RABBITMQ_USERNAME: ${RABBITMQ_USERNAME} + RABBITMQ_PASSWORD: ${RABBITMQ_PASSWORD} x-emap-db: &emap-db - EMAP_UDS_HOST: ${EMAP_UDS_HOST} - EMAP_UDS_PORT: ${EMAP_UDS_PORT} - EMAP_UDS_NAME: ${EMAP_UDS_NAME} - EMAP_UDS_USER: ${EMAP_UDS_USER} - EMAP_UDS_PASSWORD: ${EMAP_UDS_PASSWORD} - EMAP_UDS_SCHEMA_NAME: ${EMAP_UDS_SCHEMA_NAME} + EMAP_UDS_HOST: ${EMAP_UDS_HOST} + EMAP_UDS_PORT: ${EMAP_UDS_PORT} + EMAP_UDS_NAME: ${EMAP_UDS_NAME} + EMAP_UDS_USER: ${EMAP_UDS_USER} + EMAP_UDS_PASSWORD: ${EMAP_UDS_PASSWORD} + EMAP_UDS_SCHEMA_NAME: ${EMAP_UDS_SCHEMA_NAME} x-pixl-db: &pixl-db - PIXL_DB_HOST: ${PIXL_DB_HOST} - PIXL_DB_PORT: ${PIXL_DB_PORT} - PIXL_DB_USER: ${PIXL_DB_USER} - PIXL_DB_PASSWORD: ${PIXL_DB_PASSWORD} - PIXL_DB_NAME: ${PIXL_DB_NAME} + PIXL_DB_HOST: ${PIXL_DB_HOST} + PIXL_DB_PORT: ${PIXL_DB_PORT} + PIXL_DB_USER: ${PIXL_DB_USER} + PIXL_DB_PASSWORD: ${PIXL_DB_PASSWORD} + PIXL_DB_NAME: ${PIXL_DB_NAME} x-azure-keyvault: &azure-keyvault - AZURE_CLIENT_ID: ${EXPORT_AZ_CLIENT_ID} - AZURE_CLIENT_SECRET: ${EXPORT_AZ_CLIENT_PASSWORD} - AZURE_TENANT_ID: ${EXPORT_AZ_TENANT_ID} - AZURE_KEY_VAULT_NAME: ${EXPORT_AZ_KEY_VAULT_NAME} + AZURE_CLIENT_ID: ${EXPORT_AZ_CLIENT_ID} + AZURE_CLIENT_SECRET: ${EXPORT_AZ_CLIENT_PASSWORD} + AZURE_TENANT_ID: ${EXPORT_AZ_TENANT_ID} + AZURE_KEY_VAULT_NAME: ${EXPORT_AZ_KEY_VAULT_NAME} x-logs-volume: &logs-volume - type: volume - source: logs - target: /logs + type: volume + source: logs + target: /logs volumes: - logs: - orthanc-anon-data: - orthanc-raw-data: - postgres-data: - exports: - rabbitmq: + logs: + orthanc-anon-data: + orthanc-raw-data: + postgres-data: + exports: + rabbitmq: networks: - pixl-net: + pixl-net: ################################################################################ # Services services: - hasher-api: - build: - context: . - dockerfile: ./docker/hasher-api/Dockerfile - args: - <<: *build-args-common - environment: - <<: [*proxy-common, *pixl-common-env] - AZURE_CLIENT_ID: ${HASHER_API_AZ_CLIENT_ID} - AZURE_CLIENT_SECRET: ${HASHER_API_AZ_CLIENT_PASSWORD} - AZURE_TENANT_ID: ${HASHER_API_AZ_TENANT_ID} - AZURE_KEY_VAULT_NAME: ${HASHER_API_AZ_KEY_VAULT_NAME} - AZURE_KEY_VAULT_SECRET_NAME: ${HASHER_API_AZ_KEY_VAULT_SECRET_NAME} - env_file: - - ./docker/common.env - ports: - - "${HASHER_API_PORT}:8000" - volumes: - - *logs-volume - networks: - - pixl-net - healthcheck: - test: ["CMD", "curl", "-f", "http://hasher-api:8000/heart-beat"] - interval: 10s - timeout: 30s - retries: 5 - restart: "no" + hasher-api: + build: + context: . + dockerfile: ./docker/hasher-api/Dockerfile + args: + <<: *build-args-common + environment: + <<: [*proxy-common, *pixl-common-env] + AZURE_CLIENT_ID: ${HASHER_API_AZ_CLIENT_ID} + AZURE_CLIENT_SECRET: ${HASHER_API_AZ_CLIENT_PASSWORD} + AZURE_TENANT_ID: ${HASHER_API_AZ_TENANT_ID} + AZURE_KEY_VAULT_NAME: ${HASHER_API_AZ_KEY_VAULT_NAME} + AZURE_KEY_VAULT_SECRET_NAME: ${HASHER_API_AZ_KEY_VAULT_SECRET_NAME} + env_file: + - ./docker/common.env + ports: + - "${HASHER_API_PORT}:8000" + volumes: + - *logs-volume + networks: + - pixl-net + healthcheck: + test: ["CMD", "curl", "-f", "http://hasher-api:8000/heart-beat"] + interval: 10s + timeout: 30s + retries: 5 + restart: "no" - orthanc-anon: - build: - context: . - dockerfile: ./docker/orthanc-anon/Dockerfile - args: - <<: *build-args-common - platform: linux/amd64 - command: /run/secrets - environment: - <<: [*proxy-common, *pixl-common-env, *azure-keyvault] - ORTHANC_NAME: "PIXL: Anon" - ORTHANC_USERNAME: ${ORTHANC_ANON_USERNAME} - ORTHANC_PASSWORD: ${ORTHANC_ANON_PASSWORD} - ORTHANC_ANON_AE_TITLE: ${ORTHANC_ANON_AE_TITLE} - 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" - ORTHANC_URL: "http://localhost:8042" - PIXL_DB_HOST: ${PIXL_DB_HOST} - PIXL_DB_PORT: ${PIXL_DB_PORT} - PIXL_DB_NAME: ${PIXL_DB_NAME} - PIXL_DB_USER: ${PIXL_DB_USER} - PIXL_DB_PASSWORD: ${PIXL_DB_PASSWORD} - DICOM_WEB_PLUGIN_ENABLED: ${ENABLE_DICOM_WEB} - HASHER_API_AZ_NAME: "hasher-api" - HASHER_API_PORT: 8000 - HTTP_TIMEOUT: ${ORTHANC_ANON_HTTP_TIMEOUT} - AZ_DICOM_ENDPOINT_NAME: ${AZ_DICOM_ENDPOINT_NAME} - AZ_DICOM_ENDPOINT_URL: ${AZ_DICOM_ENDPOINT_URL} - AZ_DICOM_ENDPOINT_TOKEN: ${AZ_DICOM_ENDPOINT_TOKEN} - AZ_DICOM_ENDPOINT_CLIENT_ID: ${AZ_DICOM_ENDPOINT_CLIENT_ID} - AZ_DICOM_ENDPOINT_CLIENT_SECRET: ${AZ_DICOM_ENDPOINT_CLIENT_SECRET} - AZ_DICOM_ENDPOINT_TENANT_ID: ${AZ_DICOM_ENDPOINT_TENANT_ID} - AZ_DICOM_TOKEN_REFRESH_SECS: "600" - TIME_OFFSET: "${STUDY_TIME_OFFSET}" - SALT_VALUE: ${SALT_VALUE}" - PROJECT_CONFIGS_DIR: /${PROJECT_CONFIGS_DIR:-/projects/configs} - ports: - - "${ORTHANC_ANON_DICOM_PORT}:4242" - - "${ORTHANC_ANON_WEB_PORT}:8042" - volumes: - - type: volume - source: orthanc-anon-data - target: /var/lib/orthanc/db - - ${PWD}/orthanc/orthanc-anon/config:/run/secrets:ro - - ${PWD}/projects/configs:/${PROJECT_CONFIGS_DIR:-/projects/configs}:ro - networks: - - pixl-net - # needed for same reason as ehr-api - extra_hosts: - - "host.docker.internal:host-gateway" - depends_on: - postgres: - condition: service_healthy - healthcheck: - test: - [ - "CMD-SHELL", "/probes/test-aliveness.py --user=$ORTHANC_USERNAME --pwd=$ORTHANC_PASSWORD" - ] - start_period: 10s - retries: 2 - interval: 3s - timeout: 2s - restart: "no" + orthanc-anon: + build: + context: . + dockerfile: ./docker/orthanc-anon/Dockerfile + args: + <<: *build-args-common + platform: linux/amd64 + command: /run/secrets + environment: + <<: [*proxy-common, *pixl-common-env, *azure-keyvault] + ORTHANC_NAME: "PIXL: Anon" + ORTHANC_USERNAME: ${ORTHANC_ANON_USERNAME} + ORTHANC_PASSWORD: ${ORTHANC_ANON_PASSWORD} + ORTHANC_ANON_AE_TITLE: ${ORTHANC_ANON_AE_TITLE} + 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" + ORTHANC_URL: "http://localhost:8042" + PIXL_DB_HOST: ${PIXL_DB_HOST} + PIXL_DB_PORT: ${PIXL_DB_PORT} + PIXL_DB_NAME: ${PIXL_DB_NAME} + PIXL_DB_USER: ${PIXL_DB_USER} + PIXL_DB_PASSWORD: ${PIXL_DB_PASSWORD} + DICOM_WEB_PLUGIN_ENABLED: ${ENABLE_DICOM_WEB} + HASHER_API_AZ_NAME: "hasher-api" + HASHER_API_PORT: 8000 + HTTP_TIMEOUT: ${ORTHANC_ANON_HTTP_TIMEOUT} + AZ_DICOM_ENDPOINT_NAME: ${AZ_DICOM_ENDPOINT_NAME} + AZ_DICOM_ENDPOINT_URL: ${AZ_DICOM_ENDPOINT_URL} + AZ_DICOM_ENDPOINT_TOKEN: ${AZ_DICOM_ENDPOINT_TOKEN} + AZ_DICOM_ENDPOINT_CLIENT_ID: ${AZ_DICOM_ENDPOINT_CLIENT_ID} + AZ_DICOM_ENDPOINT_CLIENT_SECRET: ${AZ_DICOM_ENDPOINT_CLIENT_SECRET} + AZ_DICOM_ENDPOINT_TENANT_ID: ${AZ_DICOM_ENDPOINT_TENANT_ID} + AZ_DICOM_TOKEN_REFRESH_SECS: "600" + TIME_OFFSET: "${STUDY_TIME_OFFSET}" + SALT_VALUE: ${SALT_VALUE}" + PROJECT_CONFIGS_DIR: /${PROJECT_CONFIGS_DIR:-/projects/configs} + ports: + - "${ORTHANC_ANON_DICOM_PORT}:4242" + - "${ORTHANC_ANON_WEB_PORT}:8042" + volumes: + - type: volume + source: orthanc-anon-data + target: /var/lib/orthanc/db + - ${PWD}/orthanc/orthanc-anon/config:/run/secrets:ro + - ${PWD}/projects/configs:/${PROJECT_CONFIGS_DIR:-/projects/configs}:ro + networks: + - pixl-net + # needed for same reason as ehr-api + extra_hosts: + - "host.docker.internal:host-gateway" + depends_on: + postgres: + condition: service_healthy + healthcheck: + test: + [ + "CMD-SHELL", + "/probes/test-aliveness.py --user=$ORTHANC_USERNAME --pwd=$ORTHANC_PASSWORD", + ] + start_period: 10s + retries: 2 + interval: 3s + timeout: 2s + restart: "no" - orthanc-raw: - build: - context: . - dockerfile: ./docker/orthanc-raw/Dockerfile - args: - <<: *build-args-common - ORTHANC_RAW_MAXIMUM_STORAGE_SIZE: ${ORTHANC_RAW_MAXIMUM_STORAGE_SIZE} - ORTHANC_RAW_JOB_HISTORY_SIZE: ${ORTHANC_RAW_JOB_HISTORY_SIZE} - ORTHANC_RAW_CONCURRENT_JOBS: ${ORTHANC_RAW_CONCURRENT_JOBS} - platform: linux/amd64 - command: /run/secrets - environment: - <<: [*pixl-db, *proxy-common, *pixl-common-env] - ORTHANC_NAME: "PIXL: Raw" - ORTHANC_USERNAME: ${ORTHANC_RAW_USERNAME} - ORTHANC_PASSWORD: ${ORTHANC_RAW_PASSWORD} - ORTHANC_RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE} - ORTHANC_AUTOROUTE_RAW_TO_ANON: ${ORTHANC_AUTOROUTE_RAW_TO_ANON} - ORTHANC_RAW_RECORD_HEADERS: ${ORTHANC_RAW_RECORD_HEADERS} - ORTHANC_RAW_HEADER_LOG_PATH: ${ORTHANC_RAW_HEADER_LOG_PATH} - VNAQR_AE_TITLE : ${VNAQR_AE_TITLE} - VNAQR_DICOM_PORT: ${VNAQR_DICOM_PORT} - VNAQR_IP_ADDR: ${VNAQR_IP_ADDR} - ORTHANC_ANON_AE_TITLE: ${ORTHANC_ANON_AE_TITLE} - ORTHANC_ANON_DICOM_PORT: "4242" - ORTHANC_ANON_HOSTNAME: "orthanc-anon" - PROJECT_CONFIGS_DIR: /${PROJECT_CONFIGS_DIR:-/projects/configs} - ports: - - "${ORTHANC_RAW_DICOM_PORT}:4242" - - "${ORTHANC_RAW_WEB_PORT}:8042" - volumes: - - type: volume - source: orthanc-raw-data - target: /var/lib/orthanc/db - - ${PWD}/projects/configs:/${PROJECT_CONFIGS_DIR:-/projects/configs}:ro - networks: - - pixl-net - depends_on: - postgres: - condition: service_healthy - orthanc-anon: - condition: service_started - healthcheck: - test: - [ - "CMD-SHELL", "/probes/test-aliveness.py --user=$ORTHANC_USERNAME --pwd=$ORTHANC_PASSWORD" - ] - start_period: 10s - retries: 10 - interval: 3s - timeout: 2s - restart: "no" + orthanc-raw: + build: + context: . + dockerfile: ./docker/orthanc-raw/Dockerfile + args: + <<: *build-args-common + ORTHANC_RAW_MAXIMUM_STORAGE_SIZE: ${ORTHANC_RAW_MAXIMUM_STORAGE_SIZE} + ORTHANC_RAW_JOB_HISTORY_SIZE: ${ORTHANC_RAW_JOB_HISTORY_SIZE} + ORTHANC_RAW_CONCURRENT_JOBS: ${ORTHANC_RAW_CONCURRENT_JOBS} + platform: linux/amd64 + command: /run/secrets + environment: + <<: [*pixl-db, *proxy-common, *pixl-common-env] + ORTHANC_NAME: "PIXL: Raw" + ORTHANC_USERNAME: ${ORTHANC_RAW_USERNAME} + ORTHANC_PASSWORD: ${ORTHANC_RAW_PASSWORD} + ORTHANC_RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE} + ORTHANC_AUTOROUTE_RAW_TO_ANON: ${ORTHANC_AUTOROUTE_RAW_TO_ANON} + ORTHANC_RAW_RECORD_HEADERS: ${ORTHANC_RAW_RECORD_HEADERS} + ORTHANC_RAW_HEADER_LOG_PATH: ${ORTHANC_RAW_HEADER_LOG_PATH} + VNAQR_AE_TITLE: ${VNAQR_AE_TITLE} + VNAQR_DICOM_PORT: ${VNAQR_DICOM_PORT} + VNAQR_IP_ADDR: ${VNAQR_IP_ADDR} + ORTHANC_ANON_AE_TITLE: ${ORTHANC_ANON_AE_TITLE} + ORTHANC_ANON_DICOM_PORT: "4242" + ORTHANC_ANON_HOSTNAME: "orthanc-anon" + PROJECT_CONFIGS_DIR: /${PROJECT_CONFIGS_DIR:-/projects/configs} + ports: + - "${ORTHANC_RAW_DICOM_PORT}:4242" + - "${ORTHANC_RAW_WEB_PORT}:8042" + volumes: + - type: volume + source: orthanc-raw-data + target: /var/lib/orthanc/db + - ${PWD}/projects/configs:/${PROJECT_CONFIGS_DIR:-/projects/configs}:ro + networks: + - pixl-net + depends_on: + postgres: + condition: service_healthy + orthanc-anon: + condition: service_started + healthcheck: + test: + [ + "CMD-SHELL", + "/probes/test-aliveness.py --user=$ORTHANC_USERNAME --pwd=$ORTHANC_PASSWORD", + ] + start_period: 10s + retries: 10 + interval: 3s + timeout: 2s + restart: "no" - queue: - image: rabbitmq:3.12.9-management - hostname: queue-host - environment: - RABBITMQ_DEFAULT_USER: ${RABBITMQ_USERNAME} - RABBITMQ_DEFAULT_PASS: ${RABBITMQ_PASSWORD} - RABBITMQ_NODENAME: "rabbit@queue-host" - healthcheck: - test: rabbitmq-diagnostics -q check_running - interval: 30s - timeout: 30s - retries: 3 - ports: - - "${RABBITMQ_PORT}:5672" - - "${RABBITMQ_ADMIN_PORT}:15672" - networks: - - pixl-net - volumes: - - rabbitmq:/var/lib/rabbitmq/mnesia + queue: + image: rabbitmq:3.12.9-management + hostname: queue-host + environment: + RABBITMQ_DEFAULT_USER: ${RABBITMQ_USERNAME} + RABBITMQ_DEFAULT_PASS: ${RABBITMQ_PASSWORD} + RABBITMQ_NODENAME: "rabbit@queue-host" + healthcheck: + test: rabbitmq-diagnostics -q check_running + interval: 30s + timeout: 30s + retries: 3 + ports: + - "${RABBITMQ_PORT}:5672" + - "${RABBITMQ_ADMIN_PORT}:15672" + networks: + - pixl-net + volumes: + - rabbitmq:/var/lib/rabbitmq/mnesia - ehr-api: - build: - context: . - dockerfile: ./docker/ehr-api/Dockerfile - args: - <<: *build-args-common - environment: - <<: - [ - *pixl-db, - *emap-db, - *proxy-common, - *pixl-common-env, - *pixl-rabbit-mq, - *azure-keyvault, - ] - AZ_STORAGE_ACCOUNT_NAME: ${PIXL_EHR_API_AZ_STORAGE_ACCOUNT_NAME} - AZ_STORAGE_CONTAINER_NAME: ${PIXL_EHR_API_AZ_STORAGE_CONTAINER_NAME} - COGSTACK_REDACT_URL: ${PIXL_EHR_COGSTACK_REDACT_URL} - PROJECT_CONFIGS_DIR: /${PROJECT_CONFIGS_DIR:-/projects/configs} - PIXL_MAX_MESSAGES_IN_FLIGHT: ${PIXL_MAX_MESSAGES_IN_FLIGHT} - env_file: - - ./docker/common.env - depends_on: - queue: - condition: service_healthy - postgres: - condition: service_healthy - hasher-api: - condition: service_healthy - ports: - - "${PIXL_EHR_API_PORT}:8000" - healthcheck: - interval: 10s - timeout: 30s - retries: 5 - networks: - - pixl-net - # needed for testing under GHA (linux), so this container - # can reach the test FTP server running on the docker host - extra_hosts: - - "host.docker.internal:host-gateway" - volumes: - - ${PWD}/projects/exports:/run/projects/exports - - ${PWD}/projects/configs:/${PROJECT_CONFIGS_DIR:-/projects/configs}:ro + ehr-api: + build: + context: . + dockerfile: ./docker/ehr-api/Dockerfile + args: + <<: *build-args-common + environment: + <<: + [ + *pixl-db, + *emap-db, + *proxy-common, + *pixl-common-env, + *pixl-rabbit-mq, + *azure-keyvault, + ] + AZ_STORAGE_ACCOUNT_NAME: ${PIXL_EHR_API_AZ_STORAGE_ACCOUNT_NAME} + AZ_STORAGE_CONTAINER_NAME: ${PIXL_EHR_API_AZ_STORAGE_CONTAINER_NAME} + COGSTACK_REDACT_URL: ${PIXL_EHR_COGSTACK_REDACT_URL} + PROJECT_CONFIGS_DIR: /${PROJECT_CONFIGS_DIR:-/projects/configs} + PIXL_MAX_MESSAGES_IN_FLIGHT: ${PIXL_MAX_MESSAGES_IN_FLIGHT} + env_file: + - ./docker/common.env + depends_on: + queue: + condition: service_healthy + postgres: + condition: service_healthy + hasher-api: + condition: service_healthy + ports: + - "${PIXL_EHR_API_PORT}:8000" + healthcheck: + interval: 10s + timeout: 30s + retries: 5 + networks: + - pixl-net + # needed for testing under GHA (linux), so this container + # can reach the test FTP server running on the docker host + extra_hosts: + - "host.docker.internal:host-gateway" + volumes: + - ${PWD}/projects/exports:/run/projects/exports + - ${PWD}/projects/configs:/${PROJECT_CONFIGS_DIR:-/projects/configs}:ro - imaging-api: - build: - context: . - dockerfile: ./docker/imaging-api/Dockerfile - args: - <<: *build-args-common - depends_on: - queue: - condition: service_healthy - orthanc-raw: - condition: service_healthy - healthcheck: - test: curl -f http://0.0.0.0:8000/heart-beat - interval: 10s - timeout: 30s - retries: 5 - networks: - - pixl-net - environment: - <<: [*pixl-rabbit-mq, *proxy-common, *pixl-common-env] - ORTHANC_RAW_URL: ${ORTHANC_RAW_URL} - ORTHANC_RAW_USERNAME: ${ORTHANC_RAW_USERNAME} - ORTHANC_RAW_PASSWORD: ${ORTHANC_RAW_PASSWORD} - ORTHANC_RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE} - VNAQR_MODALITY: ${VNAQR_MODALITY} - SKIP_ALEMBIC: ${SKIP_ALEMBIC} - PIXL_DB_HOST: ${PIXL_DB_HOST} - PIXL_DB_PORT: ${PIXL_DB_PORT} - PIXL_DB_NAME: ${PIXL_DB_NAME} - PIXL_DB_USER: ${PIXL_DB_USER} - PIXL_DB_PASSWORD: ${PIXL_DB_PASSWORD} - PIXL_DICOM_TRANSFER_TIMEOUT: ${PIXL_DICOM_TRANSFER_TIMEOUT} - PIXL_QUERY_TIMEOUT: ${PIXL_QUERY_TIMEOUT} - PIXL_MAX_MESSAGES_IN_FLIGHT: ${PIXL_MAX_MESSAGES_IN_FLIGHT} - ports: - - "${PIXL_IMAGING_API_PORT}:8000" + imaging-api: + build: + context: . + dockerfile: ./docker/imaging-api/Dockerfile + args: + <<: *build-args-common + depends_on: + queue: + condition: service_healthy + orthanc-raw: + condition: service_healthy + healthcheck: + test: curl -f http://0.0.0.0:8000/heart-beat + interval: 10s + timeout: 30s + retries: 5 + networks: + - pixl-net + environment: + <<: [*pixl-rabbit-mq, *proxy-common, *pixl-common-env] + ORTHANC_RAW_URL: ${ORTHANC_RAW_URL} + ORTHANC_RAW_USERNAME: ${ORTHANC_RAW_USERNAME} + ORTHANC_RAW_PASSWORD: ${ORTHANC_RAW_PASSWORD} + ORTHANC_RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE} + VNAQR_MODALITY: ${VNAQR_MODALITY} + SKIP_ALEMBIC: ${SKIP_ALEMBIC} + PIXL_DB_HOST: ${PIXL_DB_HOST} + PIXL_DB_PORT: ${PIXL_DB_PORT} + PIXL_DB_NAME: ${PIXL_DB_NAME} + PIXL_DB_USER: ${PIXL_DB_USER} + PIXL_DB_PASSWORD: ${PIXL_DB_PASSWORD} + PIXL_DICOM_TRANSFER_TIMEOUT: ${PIXL_DICOM_TRANSFER_TIMEOUT} + PIXL_QUERY_TIMEOUT: ${PIXL_QUERY_TIMEOUT} + PIXL_MAX_MESSAGES_IN_FLIGHT: ${PIXL_MAX_MESSAGES_IN_FLIGHT} + ports: + - "${PIXL_IMAGING_API_PORT}:8000" - ################################################################################ - # Data Stores - postgres: - build: - context: . - dockerfile: ./docker/postgres/Dockerfile - args: - <<: *build-args-common - environment: - POSTGRES_USER: ${PIXL_DB_USER} - POSTGRES_PASSWORD: ${PIXL_DB_PASSWORD} - POSTGRES_DB: ${PIXL_DB_NAME} - PGTZ: Europe/London - env_file: - - ./docker/common.env - command: postgres -c 'config_file=/etc/postgresql/postgresql.conf' - volumes: - - type: volume - source: postgres-data - target: /var/lib/postgresql/data - ports: - - "${POSTGRES_PORT}:5432" - healthcheck: - test: ["CMD", "pg_isready", "-U", "${PIXL_DB_USER}"] - interval: 10s - timeout: 30s - retries: 5 - restart: always - networks: - - pixl-net + ################################################################################ + # Data Stores + postgres: + build: + context: . + dockerfile: ./docker/postgres/Dockerfile + args: + <<: *build-args-common + environment: + POSTGRES_USER: ${PIXL_DB_USER} + POSTGRES_PASSWORD: ${PIXL_DB_PASSWORD} + POSTGRES_DB: ${PIXL_DB_NAME} + PGTZ: Europe/London + env_file: + - ./docker/common.env + command: postgres -c 'config_file=/etc/postgresql/postgresql.conf' + volumes: + - type: volume + source: postgres-data + target: /var/lib/postgresql/data + ports: + - "${POSTGRES_PORT}:5432" + healthcheck: + test: ["CMD", "pg_isready", "-U", "${PIXL_DB_USER}"] + interval: 10s + timeout: 30s + retries: 5 + restart: always + networks: + - pixl-net From 53c5686968c0ed4c2dc57994cb1947abdea2c097 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 09:42:48 +0200 Subject: [PATCH 22/57] Use content type json for stow request --- pixl_core/src/core/uploader/_dicomweb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index c5a5b8a7e..1e5696080 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -57,7 +57,7 @@ def send_via_stow(self, resource_id: str) -> requests.Response: logger.info("Creating new DICOMWeb credentials") self._setup_dicomweb_credentials() - headers = {"content-type": "application/dicom", "accept": "application/dicom+json"} + headers = {"content-type": "application/json", "accept": "application/dicom+json"} payload = {"Resources": [resource_id], "Synchronous": False} try: From deaedd73bca7511d73228f5812fae1fea60e1b8c Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 09:43:01 +0200 Subject: [PATCH 23/57] Update comment about the `orthanc_dicomweb_url` --- pixl_core/src/core/uploader/_dicomweb.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index 1e5696080..0f20410fc 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -44,7 +44,9 @@ def _set_config(self) -> None: self.endpoint_user = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--username") self.endpoint_password = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--password") self.endpoint_url = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--url") - # the url of the DicomWeb service on orthanc anon (not to be confused with the downstream DicomWeb server that we're trying to upload to) + # The DICOMweb API endpoint on the Orthanc server, used by Orthanc to interact with the + # DICOMweb server. Note that this is different from the endpoint_url, which is the URL of + # the DICOMweb server itself. self.orthanc_dicomweb_url = self.orthanc_url + "/dicom-web/servers/" + self.az_prefix def upload_dicom_image(self) -> None: From ce2182a2e3a04f82621f1be7f10e6cc4cb39aad4 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 09:45:56 +0200 Subject: [PATCH 24/57] Fix typos --- pixl_core/src/core/uploader/_dicomweb.py | 2 +- pixl_core/tests/docker-compose.yml | 4 ++-- test/docker-compose.yml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index 0f20410fc..5c4c2d293 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -91,7 +91,7 @@ def _check_dicomweb_server(self) -> bool: def _setup_dicomweb_credentials(self) -> None: """ Add the necessary credentials to the DicomWeb server in Orthanc. - This dyniamically creates a new endpoint in Orthanc with the necessary credentials, so we + This dynamically creates a new endpoint in Orthanc with the necessary credentials, so we can avoid hardcoding the credentials in the Orthanc configuration at build time. """ http_timeout = int(config("HTTP_TIMEOUT", default=30)) diff --git a/pixl_core/tests/docker-compose.yml b/pixl_core/tests/docker-compose.yml index cd3d1155c..ed9270086 100644 --- a/pixl_core/tests/docker-compose.yml +++ b/pixl_core/tests/docker-compose.yml @@ -38,7 +38,7 @@ services: ORTHANC_USERNAME: "orthanc" ORTHANC_PASSWORD: "orthanc" ORTHANC_AE_TITLE: "orthanc" - RAW_AE_TITLE: ORHTANCRAW + RAW_AE_TITLE: ORTHANCRAW RAW_DICOM_PORT: "4242" RAW_IP_ADDR: "orthanc-raw" # aka. hostname DICOM_WEB_PLUGIN_ENABLED: true @@ -62,7 +62,7 @@ services: ORTHANC_USERNAME: "orthanc_dicomweb" ORTHANC_PASSWORD: "orthanc_dicomweb" ORTHANC_AE_TITLE: "DICOMWEB" - RAW_AE_TITLE: ORHTANCRAW + RAW_AE_TITLE: ORTHANCRAW RAW_DICOM_PORT: "4242" RAW_IP_ADDR: "dicom-web" # aka. hostname DICOM_WEB_PLUGIN_ENABLED: true diff --git a/test/docker-compose.yml b/test/docker-compose.yml index 7b4d1da59..4e2645577 100644 --- a/test/docker-compose.yml +++ b/test/docker-compose.yml @@ -83,7 +83,7 @@ services: ORTHANC_USERNAME: "orthanc_dicomweb" ORTHANC_PASSWORD: "orthanc_dicomweb" ORTHANC_AE_TITLE: "DICOMWEB" - RAW_AE_TITLE: ORHTANCRAW + RAW_AE_TITLE: ORTHANCRAW RAW_DICOM_PORT: "4242" RAW_IP_ADDR: "dicomweb-server" # aka. hostname DICOM_WEB_PLUGIN_ENABLED: true From f161d16e58193ffaffd2667d49b9cbeacab77f20 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 09:58:43 +0200 Subject: [PATCH 25/57] Add `raise_for_status()` calls in tests --- pixl_core/tests/uploader/test_dicomweb.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index ffcdca295..130d4d21c 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -61,7 +61,7 @@ def test_dicomweb_server_config(run_containers, dicomweb_uploader) -> None: auth=(ORTHANC_USERNAME, ORTHANC_PASSWORD), timeout=30, ) - + servers_response.raise_for_status() assert "test" in servers_response.json() @@ -72,6 +72,7 @@ def _check_study_present_on_dicomweb(study_id: str) -> bool: auth=(DICOMWEB_USERNAME, DICOMWEB_PASSWORD), timeout=30, ) + response.raise_for_status() return any(study == study_id for study in response.json()) From bde47de848e28d05b558cb03b44121cd4a68ba1f Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 10:00:39 +0200 Subject: [PATCH 26/57] Another `raise_for_status()` --- pixl_core/tests/uploader/test_dicomweb.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 130d4d21c..6b4a7b5a7 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -86,11 +86,12 @@ def test_upload_dicom_image(study_id, run_containers, dicomweb_uploader) -> None assert _check_study_present_on_dicomweb(study_id) # Clean up - requests.delete( + response = requests.delete( LOCAL_DICOMWEB_URL + "/studies/" + study_id, auth=(DICOMWEB_USERNAME, DICOMWEB_PASSWORD), timeout=30, ) + response.raise_for_status() def test_dicomweb_upload_fails_with_wrong_credentials( From 36005d654ee0ed5130168c6c28f42ac29dc3f4d0 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 10:15:36 +0200 Subject: [PATCH 27/57] Query for study ID directly --- pixl_core/tests/uploader/test_dicomweb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 6b4a7b5a7..b26b0e45d 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -73,7 +73,7 @@ def _check_study_present_on_dicomweb(study_id: str) -> bool: timeout=30, ) response.raise_for_status() - return any(study == study_id for study in response.json()) + return study_id in response.json() def test_upload_dicom_image(study_id, run_containers, dicomweb_uploader) -> None: From 0aae10a3dfba563c04a857e1396c655179fb305f Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 10:18:20 +0200 Subject: [PATCH 28/57] Set http timeout as class field for `DicomWebUploader` --- pixl_core/src/core/uploader/_dicomweb.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index 5c4c2d293..6e359b785 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -48,6 +48,7 @@ def _set_config(self) -> None: # DICOMweb server. Note that this is different from the endpoint_url, which is the URL of # the DICOMweb server itself. self.orthanc_dicomweb_url = self.orthanc_url + "/dicom-web/servers/" + self.az_prefix + self.http_timeout = int(config("HTTP_TIMEOUT", default=30)) def upload_dicom_image(self) -> None: msg = "Currently not implemented. Use `send_via_stow()` instead." @@ -68,7 +69,7 @@ def send_via_stow(self, resource_id: str) -> requests.Response: auth=(self.orthanc_user, self.orthanc_password), headers=headers, data=json.dumps(payload), - timeout=30, + timeout=self.http_timeout, ) response.raise_for_status() except requests.exceptions.RequestException: @@ -81,7 +82,9 @@ def send_via_stow(self, resource_id: str) -> requests.Response: def _check_dicomweb_server(self) -> bool: """Checks if the dicomweb server exists.""" response = requests.get( - self.orthanc_dicomweb_url, auth=(self.orthanc_user, self.orthanc_password), timeout=30 + self.orthanc_dicomweb_url, + auth=(self.orthanc_user, self.orthanc_password), + timeout=self.http_timeout, ) success_code = 200 if response.status_code != success_code: @@ -94,14 +97,12 @@ def _setup_dicomweb_credentials(self) -> None: This dynamically creates a new endpoint in Orthanc with the necessary credentials, so we can avoid hardcoding the credentials in the Orthanc configuration at build time. """ - http_timeout = int(config("HTTP_TIMEOUT", default=30)) - dicomweb_config = { "Url": self.endpoint_url, "Username": self.endpoint_user, "Password": self.endpoint_password, "HasDelete": True, - "Timeout": http_timeout, + "Timeout": self.http_timeout, } headers = {"content-type": "application/json"} From d8205496502bd27b88659d709642ff7b328ea15a Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 10:23:42 +0200 Subject: [PATCH 29/57] Hardcode DICOMweb config values in tests instead of using envvars --- pixl_core/tests/conftest.py | 4 ---- pixl_core/tests/uploader/test_dicomweb.py | 9 +++++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py index f626d3a7f..abaf4363d 100644 --- a/pixl_core/tests/conftest.py +++ b/pixl_core/tests/conftest.py @@ -53,10 +53,6 @@ os.environ["ORTHANC_URL"] = "http://localhost:8043" os.environ["ORTHANC_USERNAME"] = "orthanc" os.environ["ORTHANC_PASSWORD"] = "orthanc" # noqa: S105, hardcoded password -# Endpoint for DICOMWeb server as seen from within Orthanc -os.environ["DICOMWEB_URL"] = "http://dicomweb-server:8042/dicom-web" -os.environ["DICOMWEB_USERNAME"] = "orthanc_dicomweb" -os.environ["DICOMWEB_PASSWORD"] = "orthanc_dicomweb" # noqa: S105, hardcoded password @pytest.fixture(scope="package") diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index b26b0e45d..af0969b22 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -25,9 +25,9 @@ ORTHANC_URL = config("ORTHANC_URL") ORTHANC_USERNAME = config("ORTHANC_USERNAME") ORTHANC_PASSWORD = config("ORTHANC_PASSWORD") -DICOMWEB_USERNAME = config("DICOMWEB_USERNAME") -DICOMWEB_PASSWORD = config("DICOMWEB_PASSWORD") -DICOMWEB_URL = config("DICOMWEB_URL") + +DICOMWEB_USERNAME = "orthanc_dicomweb" +DICOMWEB_PASSWORD = "orthanc_dicomweb" # noqa: S105, hardcoded password LOCAL_DICOMWEB_URL = "http://localhost:8044" @@ -43,7 +43,8 @@ def __init__(self) -> None: self.orthanc_url = ORTHANC_URL self.endpoint_user = DICOMWEB_USERNAME self.endpoint_password = DICOMWEB_PASSWORD - self.endpoint_url = DICOMWEB_URL + # Endpoint for DICOMWeb server as seen from within Orthanc + self.endpoint_url = "http://dicomweb-server:8042/dicom-web" self.orthanc_dicomweb_url = self.orthanc_url + "/dicom-web/servers/" + self.az_prefix From 365173d2a02a848da8916fdb2060da98f11fec6f Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 10:24:04 +0200 Subject: [PATCH 30/57] Set `http_timeout` for mock Dicomweb uploader --- pixl_core/tests/uploader/test_dicomweb.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index af0969b22..2ad85e1bf 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -46,6 +46,7 @@ def __init__(self) -> None: # Endpoint for DICOMWeb server as seen from within Orthanc self.endpoint_url = "http://dicomweb-server:8042/dicom-web" self.orthanc_dicomweb_url = self.orthanc_url + "/dicom-web/servers/" + self.az_prefix + self.http_timeout = 30 @pytest.fixture() From 94d34d404177b22fd8051aca0046f14cccbe17e0 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 12:17:51 +0200 Subject: [PATCH 31/57] Validate DICOMweb server before attempting upload --- pixl_core/src/core/uploader/_dicomweb.py | 29 +++++++++++++++++++++-- pixl_core/tests/uploader/test_dicomweb.py | 17 +++++-------- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index 6e359b785..ab835e975 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -56,10 +56,12 @@ def upload_dicom_image(self) -> None: def send_via_stow(self, resource_id: str) -> requests.Response: """Upload a Dicom resource to the DicomWeb server from within Orthanc.""" - if not self._check_dicomweb_server(): + if not self._check_dicomweb_server_exists(): logger.info("Creating new DICOMWeb credentials") self._setup_dicomweb_credentials() + self._validate_dicomweb_server() + headers = {"content-type": "application/json", "accept": "application/dicom+json"} payload = {"Resources": [resource_id], "Synchronous": False} @@ -79,7 +81,7 @@ def send_via_stow(self, resource_id: str) -> requests.Response: logger.info("Dicom resource {} sent via stow", resource_id) return response - def _check_dicomweb_server(self) -> bool: + def _check_dicomweb_server_exists(self) -> bool: """Checks if the dicomweb server exists.""" response = requests.get( self.orthanc_dicomweb_url, @@ -91,6 +93,27 @@ def _check_dicomweb_server(self) -> bool: return False return True + def _validate_dicomweb_server(self) -> None: + """Check if the DICOMweb server is reachable from within the Orthanc instance.""" + connection_error = requests.exceptions.ConnectionError("DICOMweb server not reachable") + if not self._check_dicomweb_server_exists(): + logger.error("DICOMweb server not configured") + raise connection_error + + # If dicomweb server configured, check if we can reach it + try: + response = requests.post( + self.orthanc_dicomweb_url + "/get", + auth=(self.orthanc_user, self.orthanc_password), + data='{"Uri": "/studies"}', + headers={"content-type": "application/x-www-form-urlencoded"}, + timeout=self.http_timeout, + ) + response.raise_for_status() + except requests.exceptions.RequestException as e: + logger.error("Failed to reach DICOMweb server") + raise connection_error from e + def _setup_dicomweb_credentials(self) -> None: """ Add the necessary credentials to the DicomWeb server in Orthanc. @@ -120,6 +143,8 @@ def _setup_dicomweb_credentials(self) -> None: else: logger.info("Set up DICOMweb config for {}", self.orthanc_dicomweb_url) + self._validate_dicomweb_server() + def upload_parquet_files(self) -> None: msg = "DICOMWeb uploader does not support parquet files" raise NotImplementedError(msg) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 2ad85e1bf..fd92b08fd 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -81,7 +81,7 @@ def _check_study_present_on_dicomweb(study_id: str) -> bool: def test_upload_dicom_image(study_id, run_containers, dicomweb_uploader) -> None: """Tests that DICOM image can be uploaded to a DICOMWeb server""" stow_response = dicomweb_uploader.send_via_stow(study_id) - assert stow_response.status_code == 200 # succesful upload + stow_response.raise_for_status() # Check that the instance has arrived on the DICOMweb server time.sleep(2) @@ -103,18 +103,13 @@ def test_dicomweb_upload_fails_with_wrong_credentials( dicomweb_uploader.endpoint_user = "wrong" dicomweb_uploader.endpoint_password = "wrong" # noqa: S105, hardcoded password - # Force updating of the credentials, so we don't need to restart the container - dicomweb_uploader._setup_dicomweb_credentials() # noqa: SLF001, private method - - dicomweb_uploader.send_via_stow(study_id) - assert not _check_study_present_on_dicomweb(study_id) + with pytest.raises(requests.exceptions.ConnectionError): + dicomweb_uploader._setup_dicomweb_credentials() # noqa: SLF001, private method def test_dicomweb_upload_fails_with_wrong_url(study_id, run_containers, dicomweb_uploader) -> None: """Tests that the DICOMWeb uploader fails when given wrong URL.""" - dicomweb_uploader.endpoint_url = "wrong" - # Force updating of the credentials, so we don't need to restart the container - dicomweb_uploader._setup_dicomweb_credentials() # noqa: SLF001, private method + dicomweb_uploader.endpoint_url = "http://wrong" - dicomweb_uploader.send_via_stow(study_id) - assert not _check_study_present_on_dicomweb(study_id) + with pytest.raises(requests.exceptions.ConnectionError): + dicomweb_uploader._setup_dicomweb_credentials() # noqa: SLF001, private method From 918ba87a1c3c967bb7feeaa4b07252a6b258e8fe Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 14:48:06 +0200 Subject: [PATCH 32/57] Update comment Addressing Stef's suggestion --- pixl_core/tests/uploader/test_dicomweb.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index fd92b08fd..243c44347 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -43,7 +43,8 @@ def __init__(self) -> None: self.orthanc_url = ORTHANC_URL self.endpoint_user = DICOMWEB_USERNAME self.endpoint_password = DICOMWEB_PASSWORD - # Endpoint for DICOMWeb server as seen from within Orthanc + # URL for DICOMWeb server as seen from within Orthanc, i.e. the address of the dicomweb + # server within the Docker compose network self.endpoint_url = "http://dicomweb-server:8042/dicom-web" self.orthanc_dicomweb_url = self.orthanc_url + "/dicom-web/servers/" + self.az_prefix self.http_timeout = 30 From 76c6a171899d48c12c0015826ae72f70ffb47d60 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 14:52:56 +0200 Subject: [PATCH 33/57] Add another `response.raise_status()` Co-authored-by: Jeremy Stein --- pixl_core/src/core/uploader/_dicomweb.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index ab835e975..c6d319d52 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -130,13 +130,14 @@ def _setup_dicomweb_credentials(self) -> None: headers = {"content-type": "application/json"} try: - requests.put( + response = requests.put( self.orthanc_dicomweb_url, auth=(self.orthanc_user, self.orthanc_password), headers=headers, data=json.dumps(dicomweb_config), timeout=10, ) + response.raise_for_status() except requests.exceptions.RequestException: logger.error("Failed to update DICOMweb config for {}", self.orthanc_dicomweb_url) raise From bbcc133ce6580dbaff1a2740fa333685c44e038a Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 15:38:40 +0200 Subject: [PATCH 34/57] Refactor: move checks for already exported images to base uploader --- pixl_core/src/core/uploader/_ftps.py | 10 ++-------- pixl_core/src/core/uploader/base.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 3d7fbf8a0..02b124e90 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -18,12 +18,10 @@ import ftplib import ssl -from datetime import datetime, timezone from ftplib import FTP_TLS from pathlib import Path from typing import TYPE_CHECKING, Any, BinaryIO, Optional -from core.db.queries import have_already_exported_image, update_exported_at from core.uploader.base import Uploader if TYPE_CHECKING: @@ -85,10 +83,7 @@ def upload_dicom_image( """Upload a DICOM image to the FTPS server.""" logger.info("Starting FTPS upload of '{}'", pseudo_anon_image_id) - # name destination to {project-slug}/{study-pseudonymised-id}.zip - if have_already_exported_image(pseudo_anon_image_id): - msg = "Image already exported" - raise RuntimeError(msg) + super().check_already_exported(pseudo_anon_image_id) # Create the remote directory if it doesn't exist ftp = _connect_to_ftp(self.host, self.port, self.user, self.password) @@ -107,8 +102,7 @@ def upload_dicom_image( # Close the FTP connection ftp.quit() - # Update the exported_at timestamp in the PIXL database - update_exported_at(pseudo_anon_image_id, datetime.now(tz=timezone.utc)) + super().update_exported_timestamp(pseudo_anon_image_id) logger.info("Finished FTPS upload of '{}'", pseudo_anon_image_id) def upload_parquet_files(self, parquet_export: ParquetExport) -> None: diff --git a/pixl_core/src/core/uploader/base.py b/pixl_core/src/core/uploader/base.py index 948722ee3..42a534ddb 100644 --- a/pixl_core/src/core/uploader/base.py +++ b/pixl_core/src/core/uploader/base.py @@ -16,8 +16,10 @@ from __future__ import annotations from abc import ABC, abstractmethod +from datetime import datetime, timezone from typing import Any, Optional +from core.db.queries import have_already_exported_image, update_exported_at from core.project_config.secrets import AzureKeyVault @@ -60,3 +62,13 @@ def upload_parquet_files(self, *args: Any, **kwargs: Any) -> None: If an upload strategy does not support parquet files, this method should raise a NotImplementedError. """ + + def check_already_exported(self, pseudo_anon_image_id: str) -> None: + """Check if the image has already been exported.""" + if have_already_exported_image(pseudo_anon_image_id): + msg = "Image already exported" + raise RuntimeError(msg) + + def update_exported_timestamp(self, pseudo_anon_image_id: str) -> None: + """Update the exported_at timestamp in the PIXL database for the given image.""" + update_exported_at(pseudo_anon_image_id, datetime.now(tz=timezone.utc)) From afbb78fa404063ab82fee5deeb37a07a4e203222 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 15:39:29 +0200 Subject: [PATCH 35/57] Add test for DICOMweb uploader to check failure if image already exported --- pixl_core/tests/uploader/test_dicomweb.py | 36 ++++++++++++++++------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 243c44347..4c73b083c 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -79,16 +79,8 @@ def _check_study_present_on_dicomweb(study_id: str) -> bool: return study_id in response.json() -def test_upload_dicom_image(study_id, run_containers, dicomweb_uploader) -> None: - """Tests that DICOM image can be uploaded to a DICOMWeb server""" - stow_response = dicomweb_uploader.send_via_stow(study_id) - stow_response.raise_for_status() - - # Check that the instance has arrived on the DICOMweb server - time.sleep(2) - assert _check_study_present_on_dicomweb(study_id) - - # Clean up +def _clean_up_dicomweb(study_id: str) -> None: + """Clean up the DICOMWeb server.""" response = requests.delete( LOCAL_DICOMWEB_URL + "/studies/" + study_id, auth=(DICOMWEB_USERNAME, DICOMWEB_PASSWORD), @@ -97,6 +89,30 @@ def test_upload_dicom_image(study_id, run_containers, dicomweb_uploader) -> None response.raise_for_status() +def test_upload_dicom_image( + study_id, run_containers, dicomweb_uploader, not_yet_exported_dicom_image +) -> None: + """Tests that DICOM image can be uploaded to a DICOMWeb server""" + response = dicomweb_uploader.send_via_stow( + study_id, not_yet_exported_dicom_image.hashed_identifier + ) + response.raise_for_status() + + # Check that the instance has arrived on the DICOMweb server + time.sleep(2) + assert _check_study_present_on_dicomweb(study_id) + + _clean_up_dicomweb(study_id) + + +def test_upload_dicom_image_already_exported( + study_id, run_containers, dicomweb_uploader, already_exported_dicom_image +) -> None: + """Tests that exception thrown if DICOM image already exported""" + with pytest.raises(RuntimeError, match="Image already exported"): + dicomweb_uploader.send_via_stow(study_id, already_exported_dicom_image.hashed_identifier) + + def test_dicomweb_upload_fails_with_wrong_credentials( study_id, run_containers, dicomweb_uploader ) -> None: From db9233d2e885c4b38f087d2a02a6a0c4dd8244ab Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 15:55:22 +0200 Subject: [PATCH 36/57] Implement checking for image already uploaded in dicomweb uploader --- orthanc/orthanc-anon/plugin/pixl.py | 2 +- pixl_core/src/core/uploader/_dicomweb.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/orthanc/orthanc-anon/plugin/pixl.py b/orthanc/orthanc-anon/plugin/pixl.py index 3cc39ebbf..d8302f710 100644 --- a/orthanc/orthanc-anon/plugin/pixl.py +++ b/orthanc/orthanc-anon/plugin/pixl.py @@ -164,7 +164,7 @@ def Send(study_id: str) -> None: # NOTE: temporary workaround until new export-api is ready if destination == "dicomweb": - uploader.send_via_stow(study_id) + uploader.send_via_stow(study_id, hashed_image_id) else: uploader.upload_dicom_image(zip_content, hashed_image_id, project_slug) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index c6d319d52..c0da6b2f2 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -54,13 +54,14 @@ def upload_dicom_image(self) -> None: msg = "Currently not implemented. Use `send_via_stow()` instead." raise NotImplementedError(msg) - def send_via_stow(self, resource_id: str) -> requests.Response: + def send_via_stow(self, resource_id: str, pseudo_anon_image_id: str) -> requests.Response: """Upload a Dicom resource to the DicomWeb server from within Orthanc.""" if not self._check_dicomweb_server_exists(): logger.info("Creating new DICOMWeb credentials") self._setup_dicomweb_credentials() self._validate_dicomweb_server() + super().check_already_exported(pseudo_anon_image_id) headers = {"content-type": "application/json", "accept": "application/dicom+json"} payload = {"Resources": [resource_id], "Synchronous": False} @@ -78,6 +79,7 @@ def send_via_stow(self, resource_id: str) -> requests.Response: logger.error("Failed to send via stow") raise else: + super().update_exported_timestamp(pseudo_anon_image_id) logger.info("Dicom resource {} sent via stow", resource_id) return response From f886059d942e822bc33719512c5f99d47c765456 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 17:51:32 +0200 Subject: [PATCH 37/57] Move Orthanc helpers to `core.uploader` --- .../pixl_export => pixl_core/src/core/uploader}/_orthanc.py | 0 pixl_export/src/pixl_export/main.py | 3 +-- 2 files changed, 1 insertion(+), 2 deletions(-) rename {pixl_export/src/pixl_export => pixl_core/src/core/uploader}/_orthanc.py (100%) diff --git a/pixl_export/src/pixl_export/_orthanc.py b/pixl_core/src/core/uploader/_orthanc.py similarity index 100% rename from pixl_export/src/pixl_export/_orthanc.py rename to pixl_core/src/core/uploader/_orthanc.py diff --git a/pixl_export/src/pixl_export/main.py b/pixl_export/src/pixl_export/main.py index 66e8b8823..11d727468 100644 --- a/pixl_export/src/pixl_export/main.py +++ b/pixl_export/src/pixl_export/main.py @@ -22,6 +22,7 @@ ) from pathlib import Path +from core._orthanc import get_study_zip_archive, get_tags_by_study from core.exports import ParquetExport from core.project_config import load_project_config from core.rest_api.router import router @@ -32,8 +33,6 @@ from loguru import logger from pydantic import BaseModel -from ._orthanc import get_study_zip_archive, get_tags_by_study - # Set up logging as main entry point logger.remove() # Remove all handlers added so far, including the default one. logging_level = config("LOG_LEVEL", default="INFO") From 9ba5775efc784e6bc7b786e87c3cc631739b0382 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 18:04:31 +0200 Subject: [PATCH 38/57] Make `Uploader.upload_dicom_image` methods work with Orthanc study ID as only input arg --- pixl_core/src/core/uploader/_dicomweb.py | 7 ++++++- pixl_core/src/core/uploader/_ftps.py | 13 ++++++------- pixl_core/src/core/uploader/base.py | 2 +- pixl_export/src/pixl_export/main.py | 7 +++---- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index c0da6b2f2..d1f151001 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -25,6 +25,8 @@ from core.uploader.base import Uploader +from ._orthanc import get_tags_by_study + class DicomWebUploader(Uploader): """Upload strategy for a DicomWeb server.""" @@ -50,7 +52,10 @@ def _set_config(self) -> None: self.orthanc_dicomweb_url = self.orthanc_url + "/dicom-web/servers/" + self.az_prefix self.http_timeout = int(config("HTTP_TIMEOUT", default=30)) - def upload_dicom_image(self) -> None: + def upload_dicom_image(self, study_id: str) -> None: + pseudo_anon_image_id, remote_directory = get_tags_by_study(study_id) + logger.info("Starting FTPS upload of '{}'", pseudo_anon_image_id) + msg = "Currently not implemented. Use `send_via_stow()` instead." raise NotImplementedError(msg) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index b641ea2d2..70bbd5a32 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -20,10 +20,12 @@ import ssl from ftplib import FTP_TLS from pathlib import Path -from typing import TYPE_CHECKING, Any, BinaryIO, Optional +from typing import TYPE_CHECKING, Any, Optional from core.uploader.base import Uploader +from ._orthanc import get_study_zip_archive, get_tags_by_study + if TYPE_CHECKING: from socket import socket @@ -74,13 +76,9 @@ def _set_config(self) -> None: self.password = self.keyvault.fetch_secret(f"{az_prefix}--ftp--password") self.port = int(self.keyvault.fetch_secret(f"{az_prefix}--ftp--port")) - def upload_dicom_image( - self, - zip_content: BinaryIO, - pseudo_anon_image_id: str, - remote_directory: str, - ) -> None: + def upload_dicom_image(self, study_id: str) -> None: """Upload a DICOM image to the FTPS server.""" + pseudo_anon_image_id, remote_directory = get_tags_by_study(study_id) logger.info("Starting FTPS upload of '{}'", pseudo_anon_image_id) super().check_already_exported(pseudo_anon_image_id) @@ -92,6 +90,7 @@ def upload_dicom_image( logger.debug("Running {}", command) # Store the file using a binary handler + zip_content = get_study_zip_archive(study_id) try: ftp.storbinary(command, zip_content) except ftplib.all_errors as ftp_error: diff --git a/pixl_core/src/core/uploader/base.py b/pixl_core/src/core/uploader/base.py index 42a534ddb..4a1e4fe91 100644 --- a/pixl_core/src/core/uploader/base.py +++ b/pixl_core/src/core/uploader/base.py @@ -48,7 +48,7 @@ def _set_config(self) -> None: """Set the configuration for the uploader.""" @abstractmethod - def upload_dicom_image(self, *args: Any, **kwargs: Any) -> None: + def upload_dicom_image(self, study_id: str) -> None: """ Abstract method to upload DICOM images. To be overwritten by child classes. If an upload strategy does not support DICOM images, this method should raise a diff --git a/pixl_export/src/pixl_export/main.py b/pixl_export/src/pixl_export/main.py index 11d727468..c63fcc824 100644 --- a/pixl_export/src/pixl_export/main.py +++ b/pixl_export/src/pixl_export/main.py @@ -22,7 +22,7 @@ ) from pathlib import Path -from core._orthanc import get_study_zip_archive, get_tags_by_study +from core._orthanc import get_tags_by_study from core.exports import ParquetExport from core.project_config import load_project_config from core.rest_api.router import router @@ -106,11 +106,10 @@ def export_dicom_from_orthanc(study_data: StudyData) -> None: the hashed image ID (MRN + Accession number). """ study_id = study_data.study_id - hashed_image_id, project_slug = get_tags_by_study(study_id) + _, project_slug = get_tags_by_study(study_id) project_config = load_project_config(project_slug) destination = project_config.destination.dicom uploader = get_uploader(project_slug, destination, project_config.project.azure_kv_alias) logger.debug("Sending {} via '{}'", study_id, destination) - zip_content = get_study_zip_archive(study_id) - uploader.upload_dicom_image(zip_content, hashed_image_id, project_slug) + uploader.upload_dicom_image(study_id) From 5ede1a6ddba85cd80a37cf76bf83f902851929e7 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 18:16:51 +0200 Subject: [PATCH 39/57] Move FTPS-specific fixtures to its own test file Avoid some premature checking of envvars that haven't been set yet --- pixl_core/tests/conftest.py | 48 ++----------------------- pixl_core/tests/uploader/test_ftps.py | 52 ++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 49 deletions(-) diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py index abaf4363d..676ed8bed 100644 --- a/pixl_core/tests/conftest.py +++ b/pixl_core/tests/conftest.py @@ -18,14 +18,12 @@ import pathlib import shlex from pathlib import Path -from typing import TYPE_CHECKING, BinaryIO +from typing import TYPE_CHECKING import pytest import requests from core.db.models import Base, Extract, Image -from core.uploader._ftps import FTPSUploader from pytest_pixl.helpers import run_subprocess -from pytest_pixl.plugin import FtpHostAddress from sqlalchemy import Engine, create_engine from sqlalchemy.orm import Session, sessionmaker @@ -51,8 +49,8 @@ os.environ["FTP_PORT"] = "20021" os.environ["ORTHANC_URL"] = "http://localhost:8043" -os.environ["ORTHANC_USERNAME"] = "orthanc" -os.environ["ORTHANC_PASSWORD"] = "orthanc" # noqa: S105, hardcoded password +os.environ["ORTHANC_ANON_USERNAME"] = "orthanc" +os.environ["ORTHANC_ANON_PASSWORD"] = "orthanc" # noqa: S105, hardcoded password @pytest.fixture(scope="package") @@ -93,46 +91,6 @@ def study_id(run_containers) -> str: return response.json()["ParentStudy"] -class MockFTPSUploader(FTPSUploader): - """Mock FTPSUploader for testing.""" - - def __init__(self) -> None: - """Initialise the mock uploader with hardcoded values for FTPS config.""" - self.host = os.environ["FTP_HOST"] - self.user = os.environ["FTP_USER_NAME"] - self.password = os.environ["FTP_PASSWORD"] - self.port = int(os.environ["FTP_PORT"]) - - -@pytest.fixture() -def ftps_uploader() -> MockFTPSUploader: - """Return a MockFTPSUploader object.""" - return MockFTPSUploader() - - -@pytest.fixture() -def ftps_home_dir(ftps_server) -> Path: - """ - Return the FTPS server home directory, the ftps_server fixture already uses - pytest.tmp_path_factory, so no need to clean up. - """ - return Path(ftps_server.home_dir) - - -@pytest.fixture(scope="session") -def ftp_host_address(): - """Run FTP on localhost - no docker containers need to access it""" - return FtpHostAddress.LOCALHOST - - -@pytest.fixture() -def test_zip_content() -> BinaryIO: - """Directory containing the test data for uploading to the ftp server.""" - test_zip_file = TEST_DIR / "data" / "public.zip" - with test_zip_file.open("rb") as file_content: - yield file_content - - @pytest.fixture(scope="module") def monkeymodule(): """Module level monkey patch.""" diff --git a/pixl_core/tests/uploader/test_ftps.py b/pixl_core/tests/uploader/test_ftps.py index a9c53a26b..ecb60686c 100644 --- a/pixl_core/tests/uploader/test_ftps.py +++ b/pixl_core/tests/uploader/test_ftps.py @@ -14,16 +14,62 @@ """Test functionality to upload files to an FTPS endpoint.""" import filecmp -import pathlib +import os from datetime import datetime, timezone +from pathlib import Path +from typing import BinaryIO import pandas as pd import pytest from core.db.models import Image from core.db.queries import update_exported_at from core.exports import ParquetExport +from core.uploader._ftps import FTPSUploader +from pytest_pixl.plugin import FtpHostAddress from sqlalchemy.exc import NoResultFound +TEST_DIR = Path(__file__).parent + + +class MockFTPSUploader(FTPSUploader): + """Mock FTPSUploader for testing.""" + + def __init__(self) -> None: + """Initialise the mock uploader with hardcoded values for FTPS config.""" + self.host = os.environ["FTP_HOST"] + self.user = os.environ["FTP_USER_NAME"] + self.password = os.environ["FTP_PASSWORD"] + self.port = int(os.environ["FTP_PORT"]) + + +@pytest.fixture() +def ftps_uploader() -> MockFTPSUploader: + """Return a MockFTPSUploader object.""" + return MockFTPSUploader() + + +@pytest.fixture() +def ftps_home_dir(ftps_server) -> Path: + """ + Return the FTPS server home directory, the ftps_server fixture already uses + pytest.tmp_path_factory, so no need to clean up. + """ + return Path(ftps_server.home_dir) + + +@pytest.fixture(scope="session") +def ftp_host_address(): + """Run FTP on localhost - no docker containers need to access it""" + return FtpHostAddress.LOCALHOST + + +@pytest.fixture() +def test_zip_content() -> BinaryIO: + """Directory containing the test data for uploading to the ftp server.""" + test_zip_file = TEST_DIR / "data" / "public.zip" + with test_zip_file.open("rb") as file_content: + yield file_content + @pytest.mark.usefixtures("ftps_server") def test_upload_dicom_image( @@ -113,9 +159,7 @@ def test_upload_parquet(parquet_export, ftps_home_dir, ftps_uploader) -> None: """Tests that parquet files are uploaded to the correct location (but ignore their contents)""" # ARRANGE - parquet_export.copy_to_exports( - pathlib.Path(__file__).parents[3] / "test" / "resources" / "omop" - ) + parquet_export.copy_to_exports(Path(__file__).parents[3] / "test" / "resources" / "omop") parquet_export.export_radiology_linker(pd.DataFrame(list("dummy"), columns=["D"])) # ACT From abf25ff0bf688a827b6d1fbf37b7ce964e9e85e5 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 18:19:48 +0200 Subject: [PATCH 40/57] Fix orthanc-anon related envvar references --- pixl_core/src/core/uploader/_dicomweb.py | 4 ++-- pixl_core/tests/uploader/test_dicomweb.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index d1f151001..d6d4c5c21 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -40,8 +40,8 @@ def _set_config(self) -> None: az_prefix = self.keyvault_alias self.az_prefix = az_prefix if az_prefix else self.project_slug - self.orthanc_user = config("ORTHANC_USERNAME") - self.orthanc_password = config("ORTHANC_PASSWORD") + self.orthanc_user = config("ORTHANC_ANON_USERNAME") + self.orthanc_password = config("ORTHANC_ANON_PASSWORD") self.orthanc_url = config("ORTHANC_URL") self.endpoint_user = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--username") self.endpoint_password = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--password") diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 4c73b083c..17c82c3c3 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -23,8 +23,8 @@ from decouple import config # type: ignore [import-untyped] ORTHANC_URL = config("ORTHANC_URL") -ORTHANC_USERNAME = config("ORTHANC_USERNAME") -ORTHANC_PASSWORD = config("ORTHANC_PASSWORD") +ORTHANC_USERNAME = config("ORTHANC_ANON_USERNAME") +ORTHANC_PASSWORD = config("ORTHANC_ANON_PASSWORD") DICOMWEB_USERNAME = "orthanc_dicomweb" DICOMWEB_PASSWORD = "orthanc_dicomweb" # noqa: S105, hardcoded password From c177902480150a7c81f8217020e06e255c9e15b2 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 18:24:09 +0200 Subject: [PATCH 41/57] `ORTHANC_URL` -> `ORTHANC_ANON_URL`, which is more accurate --- docker-compose.yml | 2 +- pixl_core/README.md | 2 +- pixl_core/src/core/uploader/_dicomweb.py | 2 +- pixl_core/tests/conftest.py | 8 ++++---- pixl_core/tests/uploader/test_dicomweb.py | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 34d61ef22..410053bdd 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -124,7 +124,7 @@ services: ORTHANC_RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE} ORTHANC_RAW_DICOM_PORT: "4242" ORTHANC_RAW_HOSTNAME: "orthanc-raw" - ORTHANC_URL: "http://localhost:8042" + ORTHANC_ANON_URL: "http://localhost:8042" PIXL_DB_HOST: ${PIXL_DB_HOST} PIXL_DB_PORT: ${PIXL_DB_PORT} PIXL_DB_NAME: ${PIXL_DB_NAME} diff --git a/pixl_core/README.md b/pixl_core/README.md index e72705497..a346de4a3 100644 --- a/pixl_core/README.md +++ b/pixl_core/README.md @@ -150,7 +150,7 @@ to implement this. The configuration for the DICOMweb server is controlled by the following environment variables and secrets: -- `"ORTHANC_URL"`: The URL of the Orthanc server from _where_ the upload will happen, this will typically be the `orthanc-anon` instance +- `"ORTHANC_ANON_URL"`: The URL of the Orthanc server from _where_ the upload will happen, this will typically be the `orthanc-anon` instance - The `"--dicomweb--username"` and `"--dicomweb--password"` for authentication, which are fetched from the [Azure Keyvault](../docs/setup/azure-keyvault.md) - The `"--dicomweb--url"` to define the DICOMweb endpoint in Orthanc, also fetched from the Azure Keyvault diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index d6d4c5c21..6fdd0e95a 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -42,7 +42,7 @@ def _set_config(self) -> None: self.orthanc_user = config("ORTHANC_ANON_USERNAME") self.orthanc_password = config("ORTHANC_ANON_PASSWORD") - self.orthanc_url = config("ORTHANC_URL") + self.orthanc_url = config("ORTHANC_ANON_URL") self.endpoint_user = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--username") self.endpoint_password = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--password") self.endpoint_url = self.keyvault.fetch_secret(f"{self.az_prefix}--dicomweb--url") diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py index 676ed8bed..a0e88d814 100644 --- a/pixl_core/tests/conftest.py +++ b/pixl_core/tests/conftest.py @@ -48,7 +48,7 @@ os.environ["FTP_PASSWORD"] = "longpassword" # noqa: S105 Hardcoding password os.environ["FTP_PORT"] = "20021" -os.environ["ORTHANC_URL"] = "http://localhost:8043" +os.environ["ORTHANC_ANON_URL"] = "http://localhost:8043" os.environ["ORTHANC_ANON_USERNAME"] = "orthanc" os.environ["ORTHANC_ANON_PASSWORD"] = "orthanc" # noqa: S105, hardcoded password @@ -77,15 +77,15 @@ def run_containers() -> Generator[subprocess.CompletedProcess[bytes], None, None def study_id(run_containers) -> str: """Uploads a DICOM file to the Orthanc server and returns the study ID.""" DCM_FILE = Path(__file__).parents[2] / "test" / "resources" / "Dicom1.dcm" - ORTHANC_URL = os.environ["ORTHANC_URL"] + ORTHANC_ANON_URL = os.environ["ORTHANC_ANON_URL"] headers = {"content-type": "application/dicom"} data = DCM_FILE.read_bytes() response = requests.post( - f"{ORTHANC_URL}/instances", + f"{ORTHANC_ANON_URL}/instances", data=data, headers=headers, - auth=(os.environ["ORTHANC_USERNAME"], os.environ["ORTHANC_PASSWORD"]), + auth=(os.environ["ORTHANC_ANON_USERNAME"], os.environ["ORTHANC_ANON_PASSWORD"]), timeout=60, ) return response.json()["ParentStudy"] diff --git a/pixl_core/tests/uploader/test_dicomweb.py b/pixl_core/tests/uploader/test_dicomweb.py index 17c82c3c3..43cbd1645 100644 --- a/pixl_core/tests/uploader/test_dicomweb.py +++ b/pixl_core/tests/uploader/test_dicomweb.py @@ -22,7 +22,7 @@ from core.uploader._dicomweb import DicomWebUploader from decouple import config # type: ignore [import-untyped] -ORTHANC_URL = config("ORTHANC_URL") +ORTHANC_ANON_URL = config("ORTHANC_ANON_URL") ORTHANC_USERNAME = config("ORTHANC_ANON_USERNAME") ORTHANC_PASSWORD = config("ORTHANC_ANON_PASSWORD") @@ -40,7 +40,7 @@ def __init__(self) -> None: self.az_prefix = "test" self.orthanc_user = ORTHANC_USERNAME self.orthanc_password = ORTHANC_PASSWORD - self.orthanc_url = ORTHANC_URL + self.orthanc_url = ORTHANC_ANON_URL self.endpoint_user = DICOMWEB_USERNAME self.endpoint_password = DICOMWEB_PASSWORD # URL for DICOMWeb server as seen from within Orthanc, i.e. the address of the dicomweb @@ -60,7 +60,7 @@ def test_dicomweb_server_config(run_containers, dicomweb_uploader) -> None: """Tests that the DICOMWeb server is configured correctly in Orthanc""" dicomweb_uploader._setup_dicomweb_credentials() # noqa: SLF001, private method servers_response = requests.get( - ORTHANC_URL + "/dicom-web/servers", + ORTHANC_ANON_URL + "/dicom-web/servers", auth=(ORTHANC_USERNAME, ORTHANC_PASSWORD), timeout=30, ) From d469acfcd8ce05f9dc9bf5306c19b89b19cd7de8 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 18:31:54 +0200 Subject: [PATCH 42/57] Fix `TEST_DIR` path --- pixl_core/tests/uploader/test_ftps.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixl_core/tests/uploader/test_ftps.py b/pixl_core/tests/uploader/test_ftps.py index ecb60686c..5daa3b707 100644 --- a/pixl_core/tests/uploader/test_ftps.py +++ b/pixl_core/tests/uploader/test_ftps.py @@ -28,7 +28,7 @@ from pytest_pixl.plugin import FtpHostAddress from sqlalchemy.exc import NoResultFound -TEST_DIR = Path(__file__).parent +TEST_DIR = Path(__file__).parents[1] class MockFTPSUploader(FTPSUploader): From 1ece0ee7db0cc0e8c48c9ad034377030b0d5b139 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 18:32:47 +0200 Subject: [PATCH 43/57] Fix return type for fixture --- pixl_core/tests/uploader/test_ftps.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pixl_core/tests/uploader/test_ftps.py b/pixl_core/tests/uploader/test_ftps.py index 5daa3b707..259ffaddd 100644 --- a/pixl_core/tests/uploader/test_ftps.py +++ b/pixl_core/tests/uploader/test_ftps.py @@ -15,9 +15,9 @@ import filecmp import os +from collections.abc import Generator from datetime import datetime, timezone from pathlib import Path -from typing import BinaryIO import pandas as pd import pytest @@ -64,7 +64,7 @@ def ftp_host_address(): @pytest.fixture() -def test_zip_content() -> BinaryIO: +def test_zip_content() -> Generator: """Directory containing the test data for uploading to the ftp server.""" test_zip_file = TEST_DIR / "data" / "public.zip" with test_zip_file.open("rb") as file_content: From 6a401d8b9458e82ec6e5f0d47ea1397ee19e9113 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 18:40:10 +0200 Subject: [PATCH 44/57] Refactor `FTPSUploader.upload_dicom_image()`; extract FTPS specific functionality for testing --- pixl_core/src/core/uploader/_ftps.py | 18 ++++++++++++------ pixl_core/tests/uploader/test_ftps.py | 8 ++++---- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 70bbd5a32..15f2f4e36 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -20,7 +20,7 @@ import ssl from ftplib import FTP_TLS from pathlib import Path -from typing import TYPE_CHECKING, Any, Optional +from typing import TYPE_CHECKING, Any, BinaryIO, Optional from core.uploader.base import Uploader @@ -78,11 +78,21 @@ def _set_config(self) -> None: def upload_dicom_image(self, study_id: str) -> None: """Upload a DICOM image to the FTPS server.""" - pseudo_anon_image_id, remote_directory = get_tags_by_study(study_id) + pseudo_anon_image_id, project_slug = get_tags_by_study(study_id) logger.info("Starting FTPS upload of '{}'", pseudo_anon_image_id) super().check_already_exported(pseudo_anon_image_id) + zip_content = get_study_zip_archive(study_id) + self.send_via_ftps(zip_content, pseudo_anon_image_id, remote_directory=project_slug) + + super().update_exported_timestamp(pseudo_anon_image_id) + logger.info("Finished FTPS upload of '{}'", pseudo_anon_image_id) + def send_via_ftps( + self, zip_content: BinaryIO, pseudo_anon_image_id: str, remote_directory: str + ) -> None: + """Send the zip content to the FTPS server.""" + # # Create the remote directory if it doesn't exist ftp = _connect_to_ftp(self.host, self.port, self.user, self.password) _create_and_set_as_cwd(ftp, remote_directory) @@ -90,7 +100,6 @@ def upload_dicom_image(self, study_id: str) -> None: logger.debug("Running {}", command) # Store the file using a binary handler - zip_content = get_study_zip_archive(study_id) try: ftp.storbinary(command, zip_content) except ftplib.all_errors as ftp_error: @@ -101,9 +110,6 @@ def upload_dicom_image(self, study_id: str) -> None: # Close the FTP connection ftp.quit() - super().update_exported_timestamp(pseudo_anon_image_id) - logger.info("Finished FTPS upload of '{}'", pseudo_anon_image_id) - def upload_parquet_files(self, parquet_export: ParquetExport) -> None: """ Upload parquet to FTPS under //parquet. diff --git a/pixl_core/tests/uploader/test_ftps.py b/pixl_core/tests/uploader/test_ftps.py index 259ffaddd..87e278c48 100644 --- a/pixl_core/tests/uploader/test_ftps.py +++ b/pixl_core/tests/uploader/test_ftps.py @@ -72,7 +72,7 @@ def test_zip_content() -> Generator: @pytest.mark.usefixtures("ftps_server") -def test_upload_dicom_image( +def test_send_via_ftps( test_zip_content, not_yet_exported_dicom_image, ftps_uploader, ftps_home_dir ) -> None: """Tests that DICOM image can be uploaded to the correct location""" @@ -83,7 +83,7 @@ def test_upload_dicom_image( expected_output_file = ftps_home_dir / project_slug / (pseudo_anon_id + ".zip") # ACT - ftps_uploader.upload_dicom_image(test_zip_content, pseudo_anon_id, project_slug) + ftps_uploader.send_via_ftps(test_zip_content, pseudo_anon_id, project_slug) # ASSERT assert expected_output_file.exists() @@ -101,7 +101,7 @@ def test_upload_dicom_image_already_exported( # ASSERT with pytest.raises(RuntimeError, match="Image already exported"): - ftps_uploader.upload_dicom_image(test_zip_content, pseudo_anon_id, project_slug) + ftps_uploader.send_via_ftps(test_zip_content, pseudo_anon_id, project_slug) @pytest.mark.usefixtures("ftps_server") @@ -118,7 +118,7 @@ def test_upload_dicom_image_unknown(test_zip_content, ftps_uploader) -> None: # ASSERT with pytest.raises(NoResultFound): - ftps_uploader.upload_dicom_image(test_zip_content, pseudo_anon_id, project_slug) + ftps_uploader.send_via_ftps(test_zip_content, pseudo_anon_id, project_slug) def test_update_exported_and_save(rows_in_session) -> None: From 34cc6efc43c60f5a0027a7a358b3b00eb36a4eb9 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 18:55:04 +0200 Subject: [PATCH 45/57] Fix return types for fixtures --- pixl_core/tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pixl_core/tests/conftest.py b/pixl_core/tests/conftest.py index a0e88d814..c6fb50bfd 100644 --- a/pixl_core/tests/conftest.py +++ b/pixl_core/tests/conftest.py @@ -102,7 +102,7 @@ def monkeymodule(): @pytest.fixture(autouse=True, scope="module") -def db_engine(monkeymodule) -> Engine: +def db_engine(monkeymodule) -> Generator[Engine, None, None]: """ Patches the database engine with an in memory database @@ -125,7 +125,7 @@ def db_engine(monkeymodule) -> Engine: @pytest.fixture() -def db_session(db_engine) -> Session: +def db_session(db_engine) -> Generator[Session, None, None]: """ Creates a session for interacting with an in memory database. From 45c056a98b6b1f670e8cbc00a499db1ed9fea438 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 18:55:58 +0200 Subject: [PATCH 46/57] Rename fixture so it doesn't get detected as a test --- pixl_core/tests/uploader/test_ftps.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pixl_core/tests/uploader/test_ftps.py b/pixl_core/tests/uploader/test_ftps.py index 87e278c48..06e7bfe66 100644 --- a/pixl_core/tests/uploader/test_ftps.py +++ b/pixl_core/tests/uploader/test_ftps.py @@ -64,7 +64,7 @@ def ftp_host_address(): @pytest.fixture() -def test_zip_content() -> Generator: +def zip_content() -> Generator: """Directory containing the test data for uploading to the ftp server.""" test_zip_file = TEST_DIR / "data" / "public.zip" with test_zip_file.open("rb") as file_content: @@ -73,7 +73,7 @@ def test_zip_content() -> Generator: @pytest.mark.usefixtures("ftps_server") def test_send_via_ftps( - test_zip_content, not_yet_exported_dicom_image, ftps_uploader, ftps_home_dir + zip_content, not_yet_exported_dicom_image, ftps_uploader, ftps_home_dir ) -> None: """Tests that DICOM image can be uploaded to the correct location""" # ARRANGE @@ -83,7 +83,7 @@ def test_send_via_ftps( expected_output_file = ftps_home_dir / project_slug / (pseudo_anon_id + ".zip") # ACT - ftps_uploader.send_via_ftps(test_zip_content, pseudo_anon_id, project_slug) + ftps_uploader.send_via_ftps(zip_content, pseudo_anon_id, project_slug) # ASSERT assert expected_output_file.exists() @@ -91,7 +91,7 @@ def test_send_via_ftps( @pytest.mark.usefixtures("ftps_server") def test_upload_dicom_image_already_exported( - test_zip_content, already_exported_dicom_image, ftps_uploader + zip_content, already_exported_dicom_image, ftps_uploader ) -> None: """Tests that exception thrown if DICOM image already exported""" # ARRANGE @@ -101,11 +101,11 @@ def test_upload_dicom_image_already_exported( # ASSERT with pytest.raises(RuntimeError, match="Image already exported"): - ftps_uploader.send_via_ftps(test_zip_content, pseudo_anon_id, project_slug) + ftps_uploader.send_via_ftps(zip_content, pseudo_anon_id, project_slug) @pytest.mark.usefixtures("ftps_server") -def test_upload_dicom_image_unknown(test_zip_content, ftps_uploader) -> None: +def test_upload_dicom_image_unknown(zip_content, ftps_uploader) -> None: """ Tests that a different exception is thrown if image is not recognised in the PIXL DB. @@ -118,7 +118,7 @@ def test_upload_dicom_image_unknown(test_zip_content, ftps_uploader) -> None: # ASSERT with pytest.raises(NoResultFound): - ftps_uploader.send_via_ftps(test_zip_content, pseudo_anon_id, project_slug) + ftps_uploader.send_via_ftps(zip_content, pseudo_anon_id, project_slug) def test_update_exported_and_save(rows_in_session) -> None: From 0b27ae6f8ec3fb84052d4d25a8b2ba29bb4cd645 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 18:56:36 +0200 Subject: [PATCH 47/57] Move 'already exported' check to `send_via_ftps()` For easier testing --- pixl_core/src/core/uploader/_ftps.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 15f2f4e36..61a95827d 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -81,18 +81,16 @@ def upload_dicom_image(self, study_id: str) -> None: pseudo_anon_image_id, project_slug = get_tags_by_study(study_id) logger.info("Starting FTPS upload of '{}'", pseudo_anon_image_id) - super().check_already_exported(pseudo_anon_image_id) zip_content = get_study_zip_archive(study_id) self.send_via_ftps(zip_content, pseudo_anon_image_id, remote_directory=project_slug) - - super().update_exported_timestamp(pseudo_anon_image_id) logger.info("Finished FTPS upload of '{}'", pseudo_anon_image_id) def send_via_ftps( self, zip_content: BinaryIO, pseudo_anon_image_id: str, remote_directory: str ) -> None: """Send the zip content to the FTPS server.""" - # + super().check_already_exported(pseudo_anon_image_id) + # Create the remote directory if it doesn't exist ftp = _connect_to_ftp(self.host, self.port, self.user, self.password) _create_and_set_as_cwd(ftp, remote_directory) @@ -109,6 +107,7 @@ def send_via_ftps( # Close the FTP connection ftp.quit() + super().update_exported_timestamp(pseudo_anon_image_id) def upload_parquet_files(self, parquet_export: ParquetExport) -> None: """ From 16f57bc5393e13d04bb8e0a72180d7ea51872e00 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 19:13:26 +0200 Subject: [PATCH 48/57] Refactor: let `get_uploader()` work with just the project slug --- pixl_core/src/core/exports.py | 4 +--- pixl_core/src/core/uploader/__init__.py | 15 ++++++++++----- pixl_export/src/pixl_export/main.py | 7 ++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pixl_core/src/core/exports.py b/pixl_core/src/core/exports.py index aa71500f1..c884b6d99 100644 --- a/pixl_core/src/core/exports.py +++ b/pixl_core/src/core/exports.py @@ -133,9 +133,7 @@ def upload(self) -> None: logger.info(msg) else: - uploader = get_uploader( - self.project_slug, destination, project_config.project.azure_kv_alias - ) + uploader = get_uploader(self.project_slug) msg = f"Uploading parquet files for project {self.project_slug} via '{destination}'" logger.info(msg) diff --git a/pixl_core/src/core/uploader/__init__.py b/pixl_core/src/core/uploader/__init__.py index 1b3dca2dc..6d4be3568 100644 --- a/pixl_core/src/core/uploader/__init__.py +++ b/pixl_core/src/core/uploader/__init__.py @@ -23,21 +23,26 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING -from core.uploader._dicomweb import DicomWebUploader -from core.uploader._ftps import FTPSUploader +from core.project_config import load_project_config + +from ._dicomweb import DicomWebUploader +from ._ftps import FTPSUploader if TYPE_CHECKING: from core.uploader.base import Uploader # Intenitonally defined in __init__.py to avoid circular imports -def get_uploader(project_slug: str, destination: str, keyvault_alias: Optional[str]) -> Uploader: +def get_uploader(project_slug: str) -> Uploader: """Uploader Factory, returns uploader instance based on destination.""" choices: dict[str, type[Uploader]] = {"ftps": FTPSUploader, "dicomweb": DicomWebUploader} + project_config = load_project_config(project_slug) + destination = project_config.destination.dicom + try: - return choices[destination](project_slug, keyvault_alias) + return choices[destination](project_slug, project_config.project.azure_kv_alias) except KeyError: error_msg = f"Destination '{destination}' is currently not supported" diff --git a/pixl_export/src/pixl_export/main.py b/pixl_export/src/pixl_export/main.py index c63fcc824..8ef118435 100644 --- a/pixl_export/src/pixl_export/main.py +++ b/pixl_export/src/pixl_export/main.py @@ -24,7 +24,6 @@ from core._orthanc import get_tags_by_study from core.exports import ParquetExport -from core.project_config import load_project_config from core.rest_api.router import router from core.uploader import get_uploader from decouple import config # type: ignore [import-untyped] @@ -107,9 +106,7 @@ def export_dicom_from_orthanc(study_data: StudyData) -> None: """ study_id = study_data.study_id _, project_slug = get_tags_by_study(study_id) - project_config = load_project_config(project_slug) - destination = project_config.destination.dicom - uploader = get_uploader(project_slug, destination, project_config.project.azure_kv_alias) - logger.debug("Sending {} via '{}'", study_id, destination) + uploader = get_uploader(project_slug) + logger.debug("Sending {} via '{}'", study_id, type(uploader).__name__) uploader.upload_dicom_image(study_id) From b500442dbd8d8b5586d5c9f2953c9d0b8f1c56b3 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 19:15:12 +0200 Subject: [PATCH 49/57] Add missing envvars for tests --- cli/tests/conftest.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cli/tests/conftest.py b/cli/tests/conftest.py index 958ec5369..8e90d24b2 100644 --- a/cli/tests/conftest.py +++ b/cli/tests/conftest.py @@ -45,6 +45,9 @@ os.environ["POSTGRES_PORT"] = "7001" os.environ["PIXL_DB_NAME"] = "pixl" +os.environ["ORTHANC_ANON_USERNAME"] = "orthanc" +os.environ["ORTHANC_ANON_PASSWORD"] = "orthanc" # noqa: S105, hardcoded password + @pytest.fixture(autouse=True) def export_dir(tmp_path_factory: pytest.TempPathFactory) -> Path: From a10fa3d47069cde4836303c4acbfa07848ec44f1 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 19:15:52 +0200 Subject: [PATCH 50/57] Fix import --- pixl_export/src/pixl_export/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixl_export/src/pixl_export/main.py b/pixl_export/src/pixl_export/main.py index 8ef118435..55c0f7c12 100644 --- a/pixl_export/src/pixl_export/main.py +++ b/pixl_export/src/pixl_export/main.py @@ -22,10 +22,10 @@ ) from pathlib import Path -from core._orthanc import get_tags_by_study from core.exports import ParquetExport from core.rest_api.router import router from core.uploader import get_uploader +from core.uploader._orthanc import get_tags_by_study from decouple import config # type: ignore [import-untyped] from fastapi import FastAPI, HTTPException from fastapi.responses import JSONResponse From f438a1a44036c519c08e08f5ef84d15167b661e7 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 19:23:39 +0200 Subject: [PATCH 51/57] Fix docker compose, messed up during merge --- docker-compose.yml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 410053bdd..bb26bf49a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -155,7 +155,7 @@ services: - ${PWD}/projects/configs:/${PROJECT_CONFIGS_DIR:-/projects/configs}:ro networks: - pixl-net - # needed for same reason as ehr-api + # needed for same reason as export-api extra_hosts: - "host.docker.internal:host-gateway" depends_on: @@ -247,10 +247,10 @@ services: volumes: - rabbitmq:/var/lib/rabbitmq/mnesia - ehr-api: + export-api: build: context: . - dockerfile: ./docker/ehr-api/Dockerfile + dockerfile: ./docker/export-api/Dockerfile args: <<: *build-args-common environment: @@ -263,9 +263,8 @@ services: *pixl-rabbit-mq, *azure-keyvault, ] - AZ_STORAGE_ACCOUNT_NAME: ${PIXL_EHR_API_AZ_STORAGE_ACCOUNT_NAME} - AZ_STORAGE_CONTAINER_NAME: ${PIXL_EHR_API_AZ_STORAGE_CONTAINER_NAME} - COGSTACK_REDACT_URL: ${PIXL_EHR_COGSTACK_REDACT_URL} + ORTHANC_ANON_USERNAME: ${ORTHANC_ANON_USERNAME} + ORTHANC_ANON_PASSWORD: ${ORTHANC_ANON_PASSWORD} PROJECT_CONFIGS_DIR: /${PROJECT_CONFIGS_DIR:-/projects/configs} PIXL_MAX_MESSAGES_IN_FLIGHT: ${PIXL_MAX_MESSAGES_IN_FLIGHT} env_file: @@ -278,7 +277,7 @@ services: hasher-api: condition: service_healthy ports: - - "${PIXL_EHR_API_PORT}:8000" + - "${PIXL_EXPORT_API_PORT}:8000" healthcheck: interval: 10s timeout: 30s From 00ed2daaacc0be7f56b852333241d55cf652a36a Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 19:24:00 +0200 Subject: [PATCH 52/57] Implement `DicomWebUploader.upload_dicom_image()` --- pixl_core/src/core/uploader/_dicomweb.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index 6fdd0e95a..db99bebf0 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -53,11 +53,10 @@ def _set_config(self) -> None: self.http_timeout = int(config("HTTP_TIMEOUT", default=30)) def upload_dicom_image(self, study_id: str) -> None: - pseudo_anon_image_id, remote_directory = get_tags_by_study(study_id) - logger.info("Starting FTPS upload of '{}'", pseudo_anon_image_id) - - msg = "Currently not implemented. Use `send_via_stow()` instead." - raise NotImplementedError(msg) + pseudo_anon_image_id, _ = get_tags_by_study(study_id) + logger.info("Starting DICOMweb upload of '{}'", pseudo_anon_image_id) + self.send_via_stow(study_id, pseudo_anon_image_id) + logger.info("Finished DICOMweb upload of '{}'", pseudo_anon_image_id) def send_via_stow(self, resource_id: str, pseudo_anon_image_id: str) -> requests.Response: """Upload a Dicom resource to the DicomWeb server from within Orthanc.""" From 4d6eeda472fe1f78525a0e7e86ba122298e2c54e Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 19:34:53 +0200 Subject: [PATCH 53/57] Set `ORTHANC_ANON_*` envvars in the `orthanc-anon` container It's ugly but it works --- docker-compose.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index bb26bf49a..00630f22b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -124,7 +124,10 @@ services: ORTHANC_RAW_AE_TITLE: ${ORTHANC_RAW_AE_TITLE} ORTHANC_RAW_DICOM_PORT: "4242" ORTHANC_RAW_HOSTNAME: "orthanc-raw" + # For the export API ORTHANC_ANON_URL: "http://localhost:8042" + ORTHANC_ANON_USERNAME: ${ORTHANC_ANON_USERNAME} + ORTHANC_ANON_PASSWORD: ${ORTHANC_ANON_PASSWORD} PIXL_DB_HOST: ${PIXL_DB_HOST} PIXL_DB_PORT: ${PIXL_DB_PORT} PIXL_DB_NAME: ${PIXL_DB_NAME} From 5cf8d394ac787200f85f2bb14f35b25e560f906d Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 19:40:37 +0200 Subject: [PATCH 54/57] Avoid loading envvars when they're not needed --- pixl_core/src/core/uploader/_orthanc.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pixl_core/src/core/uploader/_orthanc.py b/pixl_core/src/core/uploader/_orthanc.py index e7423e5cc..9e84a0d0f 100644 --- a/pixl_core/src/core/uploader/_orthanc.py +++ b/pixl_core/src/core/uploader/_orthanc.py @@ -25,7 +25,7 @@ def get_study_zip_archive(resourceId: str) -> BytesIO: # Download zip archive of the DICOM resource - query = f"{ORTHANC_ANON_URL}/studies/{resourceId}/archive" + query = f'{config("ORTHANC_ANON_USERNAME")}/studies/{resourceId}/archive' fail_msg = "Could not download archive of resource '%s'" response_study = _query_orthanc_anon(resourceId, query, fail_msg) @@ -53,7 +53,9 @@ def get_tags_by_study(study_id: str) -> tuple[str, str]: def _query_orthanc_anon(resourceId: str, query: str, fail_msg: str) -> requests.Response: try: response = requests.get( - query, auth=(ORTHANC_ANON_USERNAME, ORTHANC_ANON_PASSWORD), timeout=10 + query, + auth=(config("ORTHANC_ANON_USERNAME"), config("ORTHANC_ANON_PASSWORD")), + timeout=10, ) response.raise_for_status() except requests.exceptions.RequestException: @@ -63,6 +65,4 @@ def _query_orthanc_anon(resourceId: str, query: str, fail_msg: str) -> requests. return response -ORTHANC_ANON_USERNAME = config("ORTHANC_ANON_USERNAME") -ORTHANC_ANON_PASSWORD = config("ORTHANC_ANON_PASSWORD") ORTHANC_ANON_URL = "http://orthanc-anon:8042" From a8d8d1714a2edb87f907e9bfad74587b71c9d2b9 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 20:16:03 +0200 Subject: [PATCH 55/57] And another missing envvar --- docker-compose.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/docker-compose.yml b/docker-compose.yml index 00630f22b..4125666b2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -266,6 +266,7 @@ services: *pixl-rabbit-mq, *azure-keyvault, ] + ORTHANC_ANON_URL: "http://orthanc-anon:8042" ORTHANC_ANON_USERNAME: ${ORTHANC_ANON_USERNAME} ORTHANC_ANON_PASSWORD: ${ORTHANC_ANON_PASSWORD} PROJECT_CONFIGS_DIR: /${PROJECT_CONFIGS_DIR:-/projects/configs} From 5ba4f59bd9bac532fe4d3501064a568ae7325cb3 Mon Sep 17 00:00:00 2001 From: Milan Malfait Date: Tue, 30 Apr 2024 20:24:10 +0200 Subject: [PATCH 56/57] Why did I even do this --- pixl_core/src/core/uploader/_orthanc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixl_core/src/core/uploader/_orthanc.py b/pixl_core/src/core/uploader/_orthanc.py index 9e84a0d0f..05dfd5a3c 100644 --- a/pixl_core/src/core/uploader/_orthanc.py +++ b/pixl_core/src/core/uploader/_orthanc.py @@ -25,7 +25,7 @@ def get_study_zip_archive(resourceId: str) -> BytesIO: # Download zip archive of the DICOM resource - query = f'{config("ORTHANC_ANON_USERNAME")}/studies/{resourceId}/archive' + query = f"{ORTHANC_ANON_URL}/studies/{resourceId}/archive" fail_msg = "Could not download archive of resource '%s'" response_study = _query_orthanc_anon(resourceId, query, fail_msg) From 2641ae2377cd4c06dd340a79e30187e67a388104 Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Wed, 1 May 2024 14:35:50 +0100 Subject: [PATCH 57/57] Update pixl_core/src/core/uploader/_dicomweb.py --- pixl_core/src/core/uploader/_dicomweb.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pixl_core/src/core/uploader/_dicomweb.py b/pixl_core/src/core/uploader/_dicomweb.py index db99bebf0..7e2d84813 100644 --- a/pixl_core/src/core/uploader/_dicomweb.py +++ b/pixl_core/src/core/uploader/_dicomweb.py @@ -82,9 +82,8 @@ def send_via_stow(self, resource_id: str, pseudo_anon_image_id: str) -> requests except requests.exceptions.RequestException: logger.error("Failed to send via stow") raise - else: - super().update_exported_timestamp(pseudo_anon_image_id) - logger.info("Dicom resource {} sent via stow", resource_id) + super().update_exported_timestamp(pseudo_anon_image_id) + logger.info("Dicom resource {} sent via stow", resource_id) return response def _check_dicomweb_server_exists(self) -> bool: