From 724563b2d468141bab2892ebd84f633b5e2577e1 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 4 Jun 2025 13:18:49 +0100 Subject: [PATCH 1/8] Update CLI to enable it to be installed in non-editable mode Add 'PIXL_ROOT', 'HOST_EXPORT_ROOT_DIR', and 'HOST_EXPORT_ROOT_DIR_MOUNT' environment variables --- cli/src/pixl_cli/_config.py | 15 +++++++++++++++ cli/src/pixl_cli/_docker_commands.py | 4 +--- cli/src/pixl_cli/_io.py | 6 +----- cli/src/pixl_cli/main.py | 14 ++++++++------ docker-compose.yml | 2 +- pytest-pixl/src/pytest_pixl/ftpserver.py | 5 +++-- 6 files changed, 29 insertions(+), 17 deletions(-) diff --git a/cli/src/pixl_cli/_config.py b/cli/src/pixl_cli/_config.py index 0a7a199c8..999b273e8 100644 --- a/cli/src/pixl_cli/_config.py +++ b/cli/src/pixl_cli/_config.py @@ -21,6 +21,21 @@ env_file = Path.cwd() / ".env" config = Config(RepositoryEnv(env_file)) if env_file.exists() else Config(RepositoryEmpty()) +# The PIXL root and export root directories from the point of view of the docker host (which +# is where the CLI runs). For the export directory within the export-api container, see +# pixl_export/main.py: EXPORT_API_EXPORT_ROOT_DIR +PIXL_ROOT = config( + "PIXL_ROOT", + default=Path(__file__).parents[3], + cast=Path, +).resolve() + +HOST_EXPORT_ROOT_DIR = config( + "HOST_EXPORT_ROOT_DIR", + default=Path(__file__).parents[3] / "projects" / "exports", + cast=Path, +).resolve() + SERVICE_SETTINGS = { "rabbitmq": { "host": config("RABBITMQ_HOST"), diff --git a/cli/src/pixl_cli/_docker_commands.py b/cli/src/pixl_cli/_docker_commands.py index ba9406be1..e67c73ac9 100644 --- a/cli/src/pixl_cli/_docker_commands.py +++ b/cli/src/pixl_cli/_docker_commands.py @@ -18,11 +18,9 @@ from typing import Optional import click -from decouple import config from loguru import logger -PIXL_ROOT = Path(__file__).parents[3].resolve() - +from pixl_cli._config import PIXL_ROOT, config # Required to allow passing unkown options to docker-compose # https://click.palletsprojects.com/en/8.1.x/advanced/#forwarding-unknown-options diff --git a/cli/src/pixl_cli/_io.py b/cli/src/pixl_cli/_io.py index a551b4dff..983ce5ca4 100644 --- a/cli/src/pixl_cli/_io.py +++ b/cli/src/pixl_cli/_io.py @@ -25,15 +25,11 @@ import pandas as pd from core.exports import ParquetExport from loguru import logger +from pixl_cli._config import HOST_EXPORT_ROOT_DIR if TYPE_CHECKING: from core.db.models import Image -# The export root dir from the point of view of the docker host (which is where the CLI runs) -# For the view from inside, see pixl_export/main.py: EXPORT_API_EXPORT_ROOT_DIR -HOST_EXPORT_ROOT_DIR = Path(__file__).parents[3] / "projects" / "exports" - - def project_info(resources_path: Path) -> tuple[str, datetime]: """ Get the project name and extract timestamp from the extract summary log file. diff --git a/cli/src/pixl_cli/main.py b/cli/src/pixl_cli/main.py index 97dbafdc3..986b2ccf4 100644 --- a/cli/src/pixl_cli/main.py +++ b/cli/src/pixl_cli/main.py @@ -25,14 +25,19 @@ import requests from core.exports import ParquetExport from core.patient_queue.producer import PixlProducer -from decouple import RepositoryEnv, UndefinedValueError, config +from decouple import RepositoryEnv, UndefinedValueError, AutoConfig from loguru import logger -from pixl_cli._config import SERVICE_SETTINGS, api_config_for_queue +from pixl_cli._config import ( + PIXL_ROOT, + HOST_EXPORT_ROOT_DIR, + SERVICE_SETTINGS, + api_config_for_queue, + config, +) from pixl_cli._database import exported_images_for_project from pixl_cli._docker_commands import dc from pixl_cli._io import ( - HOST_EXPORT_ROOT_DIR, make_radiology_linker_table, project_info, read_patient_info, @@ -45,9 +50,6 @@ # localhost needs to be added to the NO_PROXY environment variables on GAEs os.environ["NO_PROXY"] = os.environ["no_proxy"] = "localhost" -PIXL_ROOT = Path(__file__).parents[3].resolve() - - @click.group() @click.option("--debug/--no-debug", default=False) def cli(*, debug: bool) -> None: diff --git a/docker-compose.yml b/docker-compose.yml index df27cf192..86563b248 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -305,7 +305,7 @@ services: extra_hosts: - "host.docker.internal:host-gateway" volumes: - - ${PWD}/projects/exports:/run/projects/exports + - ${HOST_EXPORT_ROOT_DIR_MOUNT:-${PWD}/projects/exports}:/run/projects/exports - ${PWD}/projects/configs:/${PROJECT_CONFIGS_DIR:-/projects/configs}:ro imaging-api: diff --git a/pytest-pixl/src/pytest_pixl/ftpserver.py b/pytest-pixl/src/pytest_pixl/ftpserver.py index 14c385384..a6b9c385a 100644 --- a/pytest-pixl/src/pytest_pixl/ftpserver.py +++ b/pytest-pixl/src/pytest_pixl/ftpserver.py @@ -13,6 +13,7 @@ # limitations under the License. """A ligthweight FTPS server supporting implicit SSL for use in PIXL tests.""" +import importlib.resources from pathlib import Path from decouple import config @@ -89,8 +90,8 @@ def __init__(self, home_root: Path, host_address: str) -> None: self.home_dir: Path = home_root / self.user_name self.home_dir.mkdir() - self.certfile = Path(__file__).parents[1] / "resources" / "ssl" / "localhost.crt" - self.keyfile = Path(__file__).parents[1] / "resources" / "ssl" / "localhost.key" + self.certfile = importlib.resources.files("pytest-pixl") / "src" / "resources" / "ssl" / "localhost.crt" + self.keyfile = importlib.resources.files("pytest-pixl") / "src" / "resources" / "ssl" / "localhost.key" self.authorizer = DummyAuthorizer() self.handler = SSLImplicitFTPHandler From 65814a4f913a7d4da7299fe5e6c097d654768925 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 4 Jun 2025 13:19:49 +0100 Subject: [PATCH 2/8] Add docs on installing PIXL in non-editable mode --- cli/README.md | 29 +++++++++++++++++++++++++++++ docs/setup/developer.md | 12 ++++++++++++ 2 files changed, 41 insertions(+) diff --git a/cli/README.md b/cli/README.md index f2002f133..d806c12e6 100644 --- a/cli/README.md +++ b/cli/README.md @@ -41,8 +41,13 @@ pixl dc up will run `docker compose --project pixl_{pixl_env} up --wait --build --remove-orphans`, where `pixl_env` is determined by the `ENV` environment variable. +Note, if your `.env` file is in a different directory from the `docker-compose.yml` file, you can pass in the +path to you `.env` file using the `--env-file` argument. + ### Configuration +#### Rabbit MQ and PostgreSQL + The `rabbitmq` and PIXL DB `postgres` services are configured by setting the following environment variables (default values shown): @@ -69,6 +74,30 @@ PIXL_IMAGING_API_RATE=1 where the `*_RATE` variables set the default querying rate for the message queues. +#### Host directories + +The PIXL root and export directories on the host machine can be configured using +the following environment variables: + +```sh +PIXL_ROOT=../ +HOST_EXPORT_ROOT_DIR=../projects/configs +HOST_EXPORT_ROOT_DIR_MOUNT=./projects/configs +``` + +The `PIXL_ROOT` directory must contain the `docker-compose.yml` file and `projects/configs` folders +from the top-level directory of this repository. `PIXL_ROOT` must also contain the `.sample.env` file +if you would like to use the `pixl check_env` command. This path can be absolute or relative to your +`.env` file (which must be in your current working directory). This variable is used by the PIXL CLI +when running PIXL. + +The `HOST_EXPORT_ROOT_DIR` is the directory on your host machine that data will be exported to. This path +can be absolute or relative to your `.env` file. This variable is used by the PIXL CLI when running PIXL. + +The `HOST_EXPORT_ROOT_DIR_MOUNT` is the directory on your host machine that will be mounted for exporting data to. +The path can be absolute or relative to the `PIXL_ROOT`. This variable is used by docker-compose to mount the +export directory when starting PIXL. + ### Running the pipeline Populate queue for Imaging using parquet files: diff --git a/docs/setup/developer.md b/docs/setup/developer.md index 7f8ae9764..e962ccb48 100644 --- a/docs/setup/developer.md +++ b/docs/setup/developer.md @@ -30,6 +30,18 @@ uv sync See each service's README for instructions for individual developing and testing instructions. +### Non-editable installation + +By default, `uv` will install `pixl` and its workspace members in editable mode. To install in non-editable mode: + +```shell +uv sync --no-editable +``` + +You will need to set the `PIXL_ROOT`, `HOST_EXPORT_ROOT_DIR`, and `HOST_EXPORT_ROOT_DIR_MOUNT` environment +variables if you install PIXL in non-editable mode. See the [`cli` docs](../../cli/README.md#host-directories) +for more info. + ## Testing ### Module-level testing From f1ccc9c1e2074edd2ead3f5a2807867a2a9c455b Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 4 Jun 2025 13:20:51 +0100 Subject: [PATCH 3/8] Update system tests to run with PIXL installed in non-editable mode --- test/.env | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/.env b/test/.env index a85658401..334d93eb6 100644 --- a/test/.env +++ b/test/.env @@ -89,3 +89,8 @@ FTP_USER_PASSWORD=longpassword # Project configs directory PROJECT_CONFIGS_DIR=projects/configs + +# Host env variables +PIXL_ROOT=../ +HOST_EXPORT_ROOT_DIR=../projects/exports +HOST_EXPORT_ROOT_DIR_MOUNT=./projects/exports From a78be3ad09eda635ec12290081af307bd88e2b7a Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 4 Jun 2025 13:21:24 +0100 Subject: [PATCH 4/8] Run systems tests with PIXL installed in non-editable mode in CI --- .github/workflows/main.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4cc3b8ccb..2ee4c0bc5 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -101,10 +101,8 @@ jobs: enable-cache: true - name: Install Python dependencies - # Editable install required here due to the way the - # code determines the export directory. See issue #318. run: | - uv sync + uv sync --no-editable - name: Create .secrets.env run: touch test/.secrets.env From 71e9bd6dad12269d1388f4b2882d15ee87717d56 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 4 Jun 2025 14:13:49 +0100 Subject: [PATCH 5/8] Fix example path to project export folder in CLI readme --- cli/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/README.md b/cli/README.md index d806c12e6..9a7f56db4 100644 --- a/cli/README.md +++ b/cli/README.md @@ -81,8 +81,8 @@ the following environment variables: ```sh PIXL_ROOT=../ -HOST_EXPORT_ROOT_DIR=../projects/configs -HOST_EXPORT_ROOT_DIR_MOUNT=./projects/configs +HOST_EXPORT_ROOT_DIR=../projects/exports +HOST_EXPORT_ROOT_DIR_MOUNT=./projects/exports ``` The `PIXL_ROOT` directory must contain the `docker-compose.yml` file and `projects/configs` folders From 476ad2960cca506b77d109b704234463e3faa8f7 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 4 Jun 2025 14:55:19 +0100 Subject: [PATCH 6/8] Make linters happy --- cli/src/pixl_cli/_docker_commands.py | 1 + cli/src/pixl_cli/_io.py | 5 ++++- cli/src/pixl_cli/main.py | 5 +++-- pytest-pixl/src/pytest_pixl/ftpserver.py | 10 +++++----- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/cli/src/pixl_cli/_docker_commands.py b/cli/src/pixl_cli/_docker_commands.py index e67c73ac9..98f59ffec 100644 --- a/cli/src/pixl_cli/_docker_commands.py +++ b/cli/src/pixl_cli/_docker_commands.py @@ -22,6 +22,7 @@ from pixl_cli._config import PIXL_ROOT, config + # Required to allow passing unkown options to docker-compose # https://click.palletsprojects.com/en/8.1.x/advanced/#forwarding-unknown-options @click.command(context_settings={"ignore_unknown_options": True}) diff --git a/cli/src/pixl_cli/_io.py b/cli/src/pixl_cli/_io.py index 983ce5ca4..75969a2db 100644 --- a/cli/src/pixl_cli/_io.py +++ b/cli/src/pixl_cli/_io.py @@ -18,18 +18,21 @@ import json from datetime import datetime from enum import StrEnum, auto -from pathlib import Path from typing import TYPE_CHECKING import numpy as np import pandas as pd from core.exports import ParquetExport from loguru import logger + from pixl_cli._config import HOST_EXPORT_ROOT_DIR if TYPE_CHECKING: + from pathlib import Path + from core.db.models import Image + def project_info(resources_path: Path) -> tuple[str, datetime]: """ Get the project name and extract timestamp from the extract summary log file. diff --git a/cli/src/pixl_cli/main.py b/cli/src/pixl_cli/main.py index 986b2ccf4..edb83da8b 100644 --- a/cli/src/pixl_cli/main.py +++ b/cli/src/pixl_cli/main.py @@ -25,12 +25,12 @@ import requests from core.exports import ParquetExport from core.patient_queue.producer import PixlProducer -from decouple import RepositoryEnv, UndefinedValueError, AutoConfig +from decouple import RepositoryEnv, UndefinedValueError from loguru import logger from pixl_cli._config import ( - PIXL_ROOT, HOST_EXPORT_ROOT_DIR, + PIXL_ROOT, SERVICE_SETTINGS, api_config_for_queue, config, @@ -50,6 +50,7 @@ # localhost needs to be added to the NO_PROXY environment variables on GAEs os.environ["NO_PROXY"] = os.environ["no_proxy"] = "localhost" + @click.group() @click.option("--debug/--no-debug", default=False) def cli(*, debug: bool) -> None: diff --git a/pytest-pixl/src/pytest_pixl/ftpserver.py b/pytest-pixl/src/pytest_pixl/ftpserver.py index a6b9c385a..4923706f0 100644 --- a/pytest-pixl/src/pytest_pixl/ftpserver.py +++ b/pytest-pixl/src/pytest_pixl/ftpserver.py @@ -90,12 +90,12 @@ def __init__(self, home_root: Path, host_address: str) -> None: self.home_dir: Path = home_root / self.user_name self.home_dir.mkdir() - self.certfile = importlib.resources.files("pytest-pixl") / "src" / "resources" / "ssl" / "localhost.crt" - self.keyfile = importlib.resources.files("pytest-pixl") / "src" / "resources" / "ssl" / "localhost.key" + ssl_dir = importlib.resources.files("pytest_pixl") / "resources" / "ssl" + self.certfile = Path(str(ssl_dir.joinpath("localhost.crt"))) + self.keyfile = Path(str(ssl_dir.joinpath("localhost.key"))) self.authorizer = DummyAuthorizer() self.handler = SSLImplicitFTPHandler - self._add_user() self._setup_TLS_handler() self._create_server() @@ -117,8 +117,8 @@ def _setup_TLS_handler(self) -> None: def _check_ssl_files(self) -> None: # Make sure we have access to the SSL certificates - certfile_path = Path(self.certfile) - keyfile_path = Path(self.keyfile) + certfile_path = self.certfile + keyfile_path = self.keyfile assert certfile_path.exists(), f"Could not find certfile at {certfile_path.absolute()}" assert keyfile_path.exists(), f"Could not find keyfile at {keyfile_path.absolute()}" From 89cc5be42df5809666b5f3a7913250957780dd09 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 4 Jun 2025 15:06:12 +0100 Subject: [PATCH 7/8] Fix path to ssl dir in pytest-pixl --- pytest-pixl/src/pytest_pixl/ftpserver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest-pixl/src/pytest_pixl/ftpserver.py b/pytest-pixl/src/pytest_pixl/ftpserver.py index 4923706f0..72d96b0cb 100644 --- a/pytest-pixl/src/pytest_pixl/ftpserver.py +++ b/pytest-pixl/src/pytest_pixl/ftpserver.py @@ -90,7 +90,7 @@ def __init__(self, home_root: Path, host_address: str) -> None: self.home_dir: Path = home_root / self.user_name self.home_dir.mkdir() - ssl_dir = importlib.resources.files("pytest_pixl") / "resources" / "ssl" + ssl_dir = importlib.resources.files("pytest-pixl") / "src" / "resources" / "ssl" self.certfile = Path(str(ssl_dir.joinpath("localhost.crt"))) self.keyfile = Path(str(ssl_dir.joinpath("localhost.key"))) From f51e7fc573511c369735a147783909d16b8e0d45 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 4 Jun 2025 15:20:50 +0100 Subject: [PATCH 8/8] Exclude type checking blocks from coverage --- pyproject.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 600eeb928..662c449e7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,3 +67,7 @@ dev = [ "nibabel==5.3.2", ] +[tool.coverage.report] +exclude_also = [ + "if TYPE_CHECKING:" +]