From ab6cdeac1c5c247c3b93a635e3c775b36369ef41 Mon Sep 17 00:00:00 2001 From: mgcam Date: Tue, 16 Apr 2024 11:49:15 +0100 Subject: [PATCH 01/25] Added a file for logging changes --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..bfaa4d1 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,8 @@ +# Change Log for npg_porch Project + +The format is based on [Keep a Changelog](http://keepachangelog.com/). +This project adheres to [Semantic Versioning](http://semver.org/). + +## [Unreleased] + +## [1.0.0] - 2023-10-16 From 0d7045e02dc3497afde3747a65db9e4376d5b7e9 Mon Sep 17 00:00:00 2001 From: mgcam Date: Thu, 18 Apr 2024 10:51:15 +0100 Subject: [PATCH 02/25] Renamed the top-level source code directory. Renamed the top-level source code directory from `server` to `src` to be compliant with the team's practice and to allow for a namespace for CLI tools in the same source tree. --- README.md | 10 +++++----- setup.cfg | 4 ++-- {server => src}/__init__.py | 0 {server => src}/logging.json | 0 {server => src}/npg/__init__.py | 0 {server => src}/npg/main.py | 0 {server => src}/npg/porch/__init__.py | 0 {server => src}/npg/porch/auth/__init__.py | 0 {server => src}/npg/porch/auth/token.py | 0 {server => src}/npg/porch/endpoints/__init__.py | 0 {server => src}/npg/porch/endpoints/pipelines.py | 0 {server => src}/npg/porch/endpoints/tasks.py | 0 {server => src}/npg/porch/models/__init__.py | 0 {server => src}/npg/porch/models/permission.py | 0 {server => src}/npg/porch/models/pipeline.py | 0 {server => src}/npg/porch/models/task.py | 0 {server => src}/npg/porchdb/__init__.py | 0 {server => src}/npg/porchdb/auth.py | 0 {server => src}/npg/porchdb/connection.py | 0 {server => src}/npg/porchdb/data_access.py | 0 {server => src}/npg/porchdb/models/__init__.py | 0 {server => src}/npg/porchdb/models/base.py | 0 {server => src}/npg/porchdb/models/event.py | 0 {server => src}/npg/porchdb/models/pipeline.py | 0 {server => src}/npg/porchdb/models/task.py | 0 {server => src}/npg/porchdb/models/token.py | 0 26 files changed, 7 insertions(+), 7 deletions(-) rename {server => src}/__init__.py (100%) rename {server => src}/logging.json (100%) rename {server => src}/npg/__init__.py (100%) rename {server => src}/npg/main.py (100%) rename {server => src}/npg/porch/__init__.py (100%) rename {server => src}/npg/porch/auth/__init__.py (100%) rename {server => src}/npg/porch/auth/token.py (100%) rename {server => src}/npg/porch/endpoints/__init__.py (100%) rename {server => src}/npg/porch/endpoints/pipelines.py (100%) rename {server => src}/npg/porch/endpoints/tasks.py (100%) rename {server => src}/npg/porch/models/__init__.py (100%) rename {server => src}/npg/porch/models/permission.py (100%) rename {server => src}/npg/porch/models/pipeline.py (100%) rename {server => src}/npg/porch/models/task.py (100%) rename {server => src}/npg/porchdb/__init__.py (100%) rename {server => src}/npg/porchdb/auth.py (100%) rename {server => src}/npg/porchdb/connection.py (100%) rename {server => src}/npg/porchdb/data_access.py (100%) rename {server => src}/npg/porchdb/models/__init__.py (100%) rename {server => src}/npg/porchdb/models/base.py (100%) rename {server => src}/npg/porchdb/models/event.py (100%) rename {server => src}/npg/porchdb/models/pipeline.py (100%) rename {server => src}/npg/porchdb/models/task.py (100%) rename {server => src}/npg/porchdb/models/token.py (100%) diff --git a/README.md b/README.md index 37de3cd..6ee5cdf 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ To run the server, please execute the following from the root directory: ```bash bash pip3 install -e . -cd server +cd src mkdir -p logs export DB_URL=postgresql+asyncpg://npg_rw:$PASS@npg_porch_db:$PORT/$DATABASE export DB_SCHEMA='non_default' @@ -62,7 +62,7 @@ Some notes on arguments: --host: 0.0.0.0 = bind to all network interfaces. Reliable but greedy in some situations ---log-config: Refers to a JSON file for python logging library. An example file is found in /server/logging.json. Uvicorn provides its own logging configuration via `uvicorn.access` and `uvicorn.error`. These may behave undesirably, and can be overridden in the JSON file with an alternate config. Likewise, fastapi logs to `fastapi` if that needs filtering. For logging to files, set `use_colors = False` in the relevant handlers or shell colour settings will appear as garbage in the logs. +--log-config: Refers to a JSON file for python logging library. An example file is found in /src/logging.json. Uvicorn provides its own logging configuration via `uvicorn.access` and `uvicorn.error`. These may behave undesirably, and can be overridden in the JSON file with an alternate config. Likewise, fastapi logs to `fastapi` if that needs filtering. For logging to files, set `use_colors = False` in the relevant handlers or shell colour settings will appear as garbage in the logs. --ssl-keyfile: A PEM format key for the server certificate --ssl-certfile: A PEM format certificate for signing HTTPS communications @@ -77,11 +77,11 @@ pip install -e .[test] pytest ``` -Individual tests are run in the form `pytest server/tests/init_test.py` +Individual tests are run in the form `pytest tests/init_test.py` ### Fixtures -Fixtures reside under `server/tests/fixtures` and are registered in `server/tests/conftest.py` +Fixtures reside under `tests/fixtures` and are registered in `tests/conftest.py` They can also be listed by invoking `pytest --fixtures` Any fixtures that are not imported in `conftest.py` will not be detected. @@ -106,7 +106,7 @@ The SET command ensures that the new schema is visible _for one session only_ in DB=npg_porch export DB_URL=postgresql+psycopg2://npg_admin:$PASS@npg_porch_db:$PORT/$DB # note that the script requires a regular PG driver, not the async version showed above -server/deploy_schema.py +src/deploy_schema.py psql --host=npg_porch_db --port=$PORT --username=npg_admin --password -d $DB ``` diff --git a/setup.cfg b/setup.cfg index 9eb1408..e4b61c5 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,8 +4,8 @@ version = 1.0.0 [options] package_dir = - =server + =src packages = find: [options.packages.find] -where=server \ No newline at end of file +where=src diff --git a/server/__init__.py b/src/__init__.py similarity index 100% rename from server/__init__.py rename to src/__init__.py diff --git a/server/logging.json b/src/logging.json similarity index 100% rename from server/logging.json rename to src/logging.json diff --git a/server/npg/__init__.py b/src/npg/__init__.py similarity index 100% rename from server/npg/__init__.py rename to src/npg/__init__.py diff --git a/server/npg/main.py b/src/npg/main.py similarity index 100% rename from server/npg/main.py rename to src/npg/main.py diff --git a/server/npg/porch/__init__.py b/src/npg/porch/__init__.py similarity index 100% rename from server/npg/porch/__init__.py rename to src/npg/porch/__init__.py diff --git a/server/npg/porch/auth/__init__.py b/src/npg/porch/auth/__init__.py similarity index 100% rename from server/npg/porch/auth/__init__.py rename to src/npg/porch/auth/__init__.py diff --git a/server/npg/porch/auth/token.py b/src/npg/porch/auth/token.py similarity index 100% rename from server/npg/porch/auth/token.py rename to src/npg/porch/auth/token.py diff --git a/server/npg/porch/endpoints/__init__.py b/src/npg/porch/endpoints/__init__.py similarity index 100% rename from server/npg/porch/endpoints/__init__.py rename to src/npg/porch/endpoints/__init__.py diff --git a/server/npg/porch/endpoints/pipelines.py b/src/npg/porch/endpoints/pipelines.py similarity index 100% rename from server/npg/porch/endpoints/pipelines.py rename to src/npg/porch/endpoints/pipelines.py diff --git a/server/npg/porch/endpoints/tasks.py b/src/npg/porch/endpoints/tasks.py similarity index 100% rename from server/npg/porch/endpoints/tasks.py rename to src/npg/porch/endpoints/tasks.py diff --git a/server/npg/porch/models/__init__.py b/src/npg/porch/models/__init__.py similarity index 100% rename from server/npg/porch/models/__init__.py rename to src/npg/porch/models/__init__.py diff --git a/server/npg/porch/models/permission.py b/src/npg/porch/models/permission.py similarity index 100% rename from server/npg/porch/models/permission.py rename to src/npg/porch/models/permission.py diff --git a/server/npg/porch/models/pipeline.py b/src/npg/porch/models/pipeline.py similarity index 100% rename from server/npg/porch/models/pipeline.py rename to src/npg/porch/models/pipeline.py diff --git a/server/npg/porch/models/task.py b/src/npg/porch/models/task.py similarity index 100% rename from server/npg/porch/models/task.py rename to src/npg/porch/models/task.py diff --git a/server/npg/porchdb/__init__.py b/src/npg/porchdb/__init__.py similarity index 100% rename from server/npg/porchdb/__init__.py rename to src/npg/porchdb/__init__.py diff --git a/server/npg/porchdb/auth.py b/src/npg/porchdb/auth.py similarity index 100% rename from server/npg/porchdb/auth.py rename to src/npg/porchdb/auth.py diff --git a/server/npg/porchdb/connection.py b/src/npg/porchdb/connection.py similarity index 100% rename from server/npg/porchdb/connection.py rename to src/npg/porchdb/connection.py diff --git a/server/npg/porchdb/data_access.py b/src/npg/porchdb/data_access.py similarity index 100% rename from server/npg/porchdb/data_access.py rename to src/npg/porchdb/data_access.py diff --git a/server/npg/porchdb/models/__init__.py b/src/npg/porchdb/models/__init__.py similarity index 100% rename from server/npg/porchdb/models/__init__.py rename to src/npg/porchdb/models/__init__.py diff --git a/server/npg/porchdb/models/base.py b/src/npg/porchdb/models/base.py similarity index 100% rename from server/npg/porchdb/models/base.py rename to src/npg/porchdb/models/base.py diff --git a/server/npg/porchdb/models/event.py b/src/npg/porchdb/models/event.py similarity index 100% rename from server/npg/porchdb/models/event.py rename to src/npg/porchdb/models/event.py diff --git a/server/npg/porchdb/models/pipeline.py b/src/npg/porchdb/models/pipeline.py similarity index 100% rename from server/npg/porchdb/models/pipeline.py rename to src/npg/porchdb/models/pipeline.py diff --git a/server/npg/porchdb/models/task.py b/src/npg/porchdb/models/task.py similarity index 100% rename from server/npg/porchdb/models/task.py rename to src/npg/porchdb/models/task.py diff --git a/server/npg/porchdb/models/token.py b/src/npg/porchdb/models/token.py similarity index 100% rename from server/npg/porchdb/models/token.py rename to src/npg/porchdb/models/token.py From 7b6e4e0ff7227257650aabcc55b851daa786bbf8 Mon Sep 17 00:00:00 2001 From: mgcam Date: Thu, 18 Apr 2024 11:21:41 +0100 Subject: [PATCH 03/25] Renamed the entry-point module --- README.md | 4 ++-- src/npg/{main.py => porch_server.py} | 0 tests/fixtures/deploy_db.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename src/npg/{main.py => porch_server.py} (100%) diff --git a/README.md b/README.md index 6ee5cdf..5df1a5b 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ cd src mkdir -p logs export DB_URL=postgresql+asyncpg://npg_rw:$PASS@npg_porch_db:$PORT/$DATABASE export DB_SCHEMA='non_default' -uvicorn npg.main:app --host 0.0.0.0 --port 8080 --reload --log-config logging.json +uvicorn npg.porch_server:app --host 0.0.0.0 --port 8080 --reload --log-config logging.json ``` and open your browser at `http://localhost:8080` to see links to the docs. @@ -52,7 +52,7 @@ The server will not start without `DB_URL` in the environment When you want HTTPS, logging and all that jazz: ```bash -uvicorn main:app --workers 2 --host 0.0.0.0 --port 8080 --log-config ~/logging.json --ssl-keyfile ~/.ssh/key.pem --ssl-certfile ~/.ssh/cert.pem --ssl-ca-certs /usr/local/share/ca-certificates/institute_ca.crt +uvicorn porch_server:app --workers 2 --host 0.0.0.0 --port 8080 --log-config ~/logging.json --ssl-keyfile ~/.ssh/key.pem --ssl-certfile ~/.ssh/cert.pem --ssl-ca-certs /usr/local/share/ca-certificates/institute_ca.crt ``` Consider running with nohup or similar. diff --git a/src/npg/main.py b/src/npg/porch_server.py similarity index 100% rename from src/npg/main.py rename to src/npg/porch_server.py diff --git a/tests/fixtures/deploy_db.py b/tests/fixtures/deploy_db.py index 1a0d966..78c9cee 100644 --- a/tests/fixtures/deploy_db.py +++ b/tests/fixtures/deploy_db.py @@ -8,7 +8,7 @@ ) from npg.porchdb.data_access import AsyncDbAccessor from npg.porch.models import Task as ModelledTask, TaskStateEnum -from npg.main import app +from npg.porch_server import app @pytest.fixture def minimum_data(): From 22455e537f9dff2345d121517a5c346b0793aad9 Mon Sep 17 00:00:00 2001 From: mgcam Date: Thu, 18 Apr 2024 12:14:20 +0100 Subject: [PATCH 04/25] Renamed the top-level npg namespace to npg_porch. npg.porch is collapsed into npg_porch npg.porchdb is reorganised into npg_porch.db The entry point of the application is relocated and renamed npg/porch_server.py -> npg_porch/server.py Import statements in teh code and tests and docs are updated. --- README.md | 4 ++-- docs/user_guide.md | 8 ++++---- scripts/deploy_schema.py | 6 +++--- scripts/issue_token.py | 2 +- src/npg/porchdb/__init__.py | 0 src/{npg => npg_porch}/__init__.py | 0 src/{npg/porch => npg_porch/auth}/__init__.py | 0 src/{npg/porch => npg_porch}/auth/token.py | 4 ++-- src/{npg/porch/auth => npg_porch/db}/__init__.py | 0 src/{npg/porchdb => npg_porch/db}/auth.py | 4 ++-- src/{npg/porchdb => npg_porch/db}/connection.py | 10 +++------- src/{npg/porchdb => npg_porch/db}/data_access.py | 4 ++-- .../porchdb => npg_porch/db}/models/__init__.py | 0 src/{npg/porchdb => npg_porch/db}/models/base.py | 0 .../porchdb => npg_porch/db}/models/event.py | 0 .../porchdb => npg_porch/db}/models/pipeline.py | 2 +- src/{npg/porchdb => npg_porch/db}/models/task.py | 2 +- .../porchdb => npg_porch/db}/models/token.py | 0 .../porch => npg_porch}/endpoints/__init__.py | 0 .../porch => npg_porch}/endpoints/pipelines.py | 8 ++++---- src/{npg/porch => npg_porch}/endpoints/tasks.py | 10 +++++----- src/{npg/porch => npg_porch}/models/__init__.py | 0 .../porch => npg_porch}/models/permission.py | 2 +- src/{npg/porch => npg_porch}/models/pipeline.py | 0 src/{npg/porch => npg_porch}/models/task.py | 2 +- src/{npg/porch_server.py => npg_porch/server.py} | 2 +- tests/data_access_test.py | 4 ++-- tests/db_auth_test.py | 16 ++++++++-------- tests/db_task_test.py | 2 +- tests/db_token_test.py | 2 +- tests/fixtures/deploy_db.py | 8 ++++---- tests/fixtures/orm_session.py | 4 ++-- tests/init_test.py | 2 +- tests/model_permission_test.py | 4 ++-- tests/pipeline_route_test.py | 2 +- tests/task_route_test.py | 2 +- 36 files changed, 56 insertions(+), 60 deletions(-) delete mode 100644 src/npg/porchdb/__init__.py rename src/{npg => npg_porch}/__init__.py (100%) rename src/{npg/porch => npg_porch/auth}/__init__.py (100%) rename src/{npg/porch => npg_porch}/auth/token.py (91%) rename src/{npg/porch/auth => npg_porch/db}/__init__.py (100%) rename src/{npg/porchdb => npg_porch/db}/auth.py (96%) rename src/{npg/porchdb => npg_porch/db}/connection.py (90%) rename src/{npg/porchdb => npg_porch/db}/data_access.py (98%) rename src/{npg/porchdb => npg_porch/db}/models/__init__.py (100%) rename src/{npg/porchdb => npg_porch/db}/models/base.py (100%) rename src/{npg/porchdb => npg_porch/db}/models/event.py (100%) rename src/{npg/porchdb => npg_porch/db}/models/pipeline.py (96%) rename src/{npg/porchdb => npg_porch/db}/models/task.py (98%) rename src/{npg/porchdb => npg_porch/db}/models/token.py (100%) rename src/{npg/porch => npg_porch}/endpoints/__init__.py (100%) rename src/{npg/porch => npg_porch}/endpoints/pipelines.py (95%) rename src/{npg/porch => npg_porch}/endpoints/tasks.py (95%) rename src/{npg/porch => npg_porch}/models/__init__.py (100%) rename src/{npg/porch => npg_porch}/models/permission.py (98%) rename src/{npg/porch => npg_porch}/models/pipeline.py (100%) rename src/{npg/porch => npg_porch}/models/task.py (98%) rename src/{npg/porch_server.py => npg_porch/server.py} (97%) diff --git a/README.md b/README.md index 5df1a5b..89f395c 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ cd src mkdir -p logs export DB_URL=postgresql+asyncpg://npg_rw:$PASS@npg_porch_db:$PORT/$DATABASE export DB_SCHEMA='non_default' -uvicorn npg.porch_server:app --host 0.0.0.0 --port 8080 --reload --log-config logging.json +uvicorn npg_porch.server:app --host 0.0.0.0 --port 8080 --reload --log-config logging.json ``` and open your browser at `http://localhost:8080` to see links to the docs. @@ -52,7 +52,7 @@ The server will not start without `DB_URL` in the environment When you want HTTPS, logging and all that jazz: ```bash -uvicorn porch_server:app --workers 2 --host 0.0.0.0 --port 8080 --log-config ~/logging.json --ssl-keyfile ~/.ssh/key.pem --ssl-certfile ~/.ssh/cert.pem --ssl-ca-certs /usr/local/share/ca-certificates/institute_ca.crt +uvicorn server:app --workers 2 --host 0.0.0.0 --port 8080 --log-config ~/logging.json --ssl-keyfile ~/.ssh/key.pem --ssl-certfile ~/.ssh/cert.pem --ssl-ca-certs /usr/local/share/ca-certificates/institute_ca.crt ``` Consider running with nohup or similar. diff --git a/docs/user_guide.md b/docs/user_guide.md index 73e50ba..e677a3d 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -30,7 +30,7 @@ Access to the service is loosely controlled with authorisation tokens. You will ### Step 1 - register your pipeline with npg_porch -*Schema: npg.porch.model.pipeline* +*Schema: npg_porch.model.pipeline* Nothing in npg_porch can happen until there's a pipeline defined. For our purposes "pipeline" means "a thing you can run", and it may refer to specific code, or a wrapper that can run the pipeline in this particular way with some standard arguments. @@ -50,7 +50,7 @@ You can name your pipeline however you like, but the name must be unique, and be Keep this pipeline definition with your data, as you will need it to tell npg_porch which pipeline you are acting on. -When communicating with npg_porch (as with any HTTP server) you must inspect the response code and message after each communication. See `-w " %{http_code}" above. The API documentation lists the response codes you can expect to have to handle. In this case, the server may respond with 400 - BAD REQUEST if you leave out a name, or 409 - CONFLICT if you chose a name that is already created. +As with any HTTP server, when communicating with npg_porch you must inspect the response code and message after each communication. See `-w " %{http_code}" above. The API documentation lists the response codes you can expect to have to handle. In this case, the server may respond with 400 - BAD REQUEST if you leave out a name, or 409 - CONFLICT if you chose a name that is already created. ### Step 2 - decide on the unique criteria for running the pipeline @@ -58,7 +58,7 @@ e.g. Once per 24 hours, poll iRODS metadata for data relating to a study. We might create a cronjob that runs a script. It invokes `imeta` and retrieves a list of results. Now we turn each of those results into a JSON document to our own specification: -*Schema: npg.porch.model.task* +*Schema: npg_porch.model.task* **study-100-id-run-45925.json** @@ -113,7 +113,7 @@ Note that it is possible to run the same `task_input` with a different `pipeline ### Step 3 - register the documents with npg_porch -*Schema: npg.porch.model.task* +*Schema: npg_porch.model.task* Now you want the pipeline to run once per specification, and so register the documents with npg_porch. diff --git a/scripts/deploy_schema.py b/scripts/deploy_schema.py index 9f56d71..ea7adf4 100644 --- a/scripts/deploy_schema.py +++ b/scripts/deploy_schema.py @@ -3,7 +3,7 @@ import os import sqlalchemy -import npg.porchdb.models +import npg_porch.db.models db_url = os.environ.get('DB_URL') schema_name = os.environ.get('DB_SCHEMA') @@ -17,5 +17,5 @@ connect_args={'options': f'-csearch_path={schema_name}'} ) -npg.porchdb.models.Base.metadata.schema = schema_name -npg.porchdb.models.Base.metadata.create_all(engine) +npg_porch.db.models.Base.metadata.schema = schema_name +npg_porch.db.models.Base.metadata.create_all(engine) diff --git a/scripts/issue_token.py b/scripts/issue_token.py index b31e4db..6300f1a 100755 --- a/scripts/issue_token.py +++ b/scripts/issue_token.py @@ -5,7 +5,7 @@ from sqlalchemy.orm import sessionmaker from sqlalchemy.orm.exc import NoResultFound -from npg.porchdb.models import Token, Pipeline +from npg_porch.db.models import Token, Pipeline parser = argparse.ArgumentParser( description='Creates a token in the backend DB and returns it' diff --git a/src/npg/porchdb/__init__.py b/src/npg/porchdb/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/src/npg/__init__.py b/src/npg_porch/__init__.py similarity index 100% rename from src/npg/__init__.py rename to src/npg_porch/__init__.py diff --git a/src/npg/porch/__init__.py b/src/npg_porch/auth/__init__.py similarity index 100% rename from src/npg/porch/__init__.py rename to src/npg_porch/auth/__init__.py diff --git a/src/npg/porch/auth/token.py b/src/npg_porch/auth/token.py similarity index 91% rename from src/npg/porch/auth/token.py rename to src/npg_porch/auth/token.py index 49cf89c..0fa4ed9 100644 --- a/src/npg/porch/auth/token.py +++ b/src/npg_porch/auth/token.py @@ -23,8 +23,8 @@ from fastapi.security import HTTPBearer from fastapi import HTTPException -from npg.porchdb.connection import get_CredentialsValidator -from npg.porchdb.auth import CredentialsValidationException +from npg_porch.db.connection import get_CredentialsValidator +from npg_porch.db.auth import CredentialsValidationException auth_scheme = HTTPBearer() diff --git a/src/npg/porch/auth/__init__.py b/src/npg_porch/db/__init__.py similarity index 100% rename from src/npg/porch/auth/__init__.py rename to src/npg_porch/db/__init__.py diff --git a/src/npg/porchdb/auth.py b/src/npg_porch/db/auth.py similarity index 96% rename from src/npg/porchdb/auth.py rename to src/npg_porch/db/auth.py index 3d30ef5..364ee21 100644 --- a/src/npg/porchdb/auth.py +++ b/src/npg_porch/db/auth.py @@ -23,8 +23,8 @@ from sqlalchemy.orm import contains_eager from sqlalchemy.orm.exc import NoResultFound -from npg.porchdb.models import Token -from npg.porch.models.permission import Permission, RolesEnum +from npg_porch.db.models import Token +from npg_porch.models.permission import Permission, RolesEnum __AUTH_TOKEN_LENGTH__ = 32 __AUTH_TOKEN_REGEXP__ = re.compile( diff --git a/src/npg/porchdb/connection.py b/src/npg_porch/db/connection.py similarity index 90% rename from src/npg/porchdb/connection.py rename to src/npg_porch/db/connection.py index 1374db3..fea8c3e 100644 --- a/src/npg/porchdb/connection.py +++ b/src/npg_porch/db/connection.py @@ -22,9 +22,9 @@ from sqlalchemy.orm import sessionmaker from sqlalchemy.ext.asyncio import AsyncSession, create_async_engine -from npg.porchdb.models import Base -from npg.porchdb.data_access import AsyncDbAccessor -from npg.porchdb.auth import Validator +from npg_porch.db.models import Base +from npg_porch.db.data_access import AsyncDbAccessor +from npg_porch.db.auth import Validator config = { 'DB_URL': os.environ.get('DB_URL'), @@ -34,7 +34,6 @@ if config['TEST']: config['DB_URL'] = 'sqlite+aiosqlite:///:memory:' - # config['DB_URL'] = 'sqlite+aiosqlite:///test.db' if config['DB_URL'] is None or config['DB_URL'] == '': raise Exception( @@ -92,6 +91,3 @@ async def deploy_schema(): async def close_engine(): 'Currently only needed when testing to force fixtures to refresh' await engine.dispose() - # Delete the data here for stateless testing if not in-memory - # if config['TEST']: - # os.remove('test.db') diff --git a/src/npg/porchdb/data_access.py b/src/npg_porch/db/data_access.py similarity index 98% rename from src/npg/porchdb/data_access.py rename to src/npg_porch/db/data_access.py index effd0e4..67ad9c3 100644 --- a/src/npg/porchdb/data_access.py +++ b/src/npg_porch/db/data_access.py @@ -24,8 +24,8 @@ from sqlalchemy.orm import contains_eager, joinedload from sqlalchemy.orm.exc import NoResultFound -from npg.porchdb.models import Pipeline as DbPipeline, Task as DbTask, Event -from npg.porch.models import Task, Pipeline, TaskStateEnum +from npg_porch.db.models import Pipeline as DbPipeline, Task as DbTask, Event +from npg_porch.models import Task, Pipeline, TaskStateEnum class AsyncDbAccessor: diff --git a/src/npg/porchdb/models/__init__.py b/src/npg_porch/db/models/__init__.py similarity index 100% rename from src/npg/porchdb/models/__init__.py rename to src/npg_porch/db/models/__init__.py diff --git a/src/npg/porchdb/models/base.py b/src/npg_porch/db/models/base.py similarity index 100% rename from src/npg/porchdb/models/base.py rename to src/npg_porch/db/models/base.py diff --git a/src/npg/porchdb/models/event.py b/src/npg_porch/db/models/event.py similarity index 100% rename from src/npg/porchdb/models/event.py rename to src/npg_porch/db/models/event.py diff --git a/src/npg/porchdb/models/pipeline.py b/src/npg_porch/db/models/pipeline.py similarity index 96% rename from src/npg/porchdb/models/pipeline.py rename to src/npg_porch/db/models/pipeline.py index 1f03d5d..b4803a6 100644 --- a/src/npg/porchdb/models/pipeline.py +++ b/src/npg_porch/db/models/pipeline.py @@ -25,7 +25,7 @@ from .base import Base -from npg.porch.models import Pipeline as ModeledPipeline +from npg_porch.models import Pipeline as ModeledPipeline class Pipeline(Base): ''' diff --git a/src/npg/porchdb/models/task.py b/src/npg_porch/db/models/task.py similarity index 98% rename from src/npg/porchdb/models/task.py rename to src/npg_porch/db/models/task.py index dd146eb..91a7550 100644 --- a/src/npg/porchdb/models/task.py +++ b/src/npg_porch/db/models/task.py @@ -26,7 +26,7 @@ from sqlalchemy.sql.sqltypes import DateTime from .base import Base -from npg.porch.models import Task as ModelledTask +from npg_porch.models import Task as ModelledTask class Task(Base): diff --git a/src/npg/porchdb/models/token.py b/src/npg_porch/db/models/token.py similarity index 100% rename from src/npg/porchdb/models/token.py rename to src/npg_porch/db/models/token.py diff --git a/src/npg/porch/endpoints/__init__.py b/src/npg_porch/endpoints/__init__.py similarity index 100% rename from src/npg/porch/endpoints/__init__.py rename to src/npg_porch/endpoints/__init__.py diff --git a/src/npg/porch/endpoints/pipelines.py b/src/npg_porch/endpoints/pipelines.py similarity index 95% rename from src/npg/porch/endpoints/pipelines.py rename to src/npg_porch/endpoints/pipelines.py index d1afcd7..9a99604 100644 --- a/src/npg/porch/endpoints/pipelines.py +++ b/src/npg_porch/endpoints/pipelines.py @@ -25,10 +25,10 @@ from sqlalchemy.orm.exc import NoResultFound from starlette import status -from npg.porch.models.pipeline import Pipeline -from npg.porch.models.permission import RolesEnum -from npg.porchdb.connection import get_DbAccessor -from npg.porch.auth.token import validate +from npg_porch.models.pipeline import Pipeline +from npg_porch.models.permission import RolesEnum +from npg_porch.db.connection import get_DbAccessor +from npg_porch.auth.token import validate router = APIRouter( diff --git a/src/npg/porch/endpoints/tasks.py b/src/npg_porch/endpoints/tasks.py similarity index 95% rename from src/npg/porch/endpoints/tasks.py rename to src/npg_porch/endpoints/tasks.py index 3a57d2a..b4aecd1 100644 --- a/src/npg/porch/endpoints/tasks.py +++ b/src/npg_porch/endpoints/tasks.py @@ -22,11 +22,11 @@ from typing import Annotated from fastapi import APIRouter, Depends, HTTPException, Query -from npg.porch.auth.token import validate -from npg.porch.models.permission import PermissionValidationException -from npg.porch.models.pipeline import Pipeline -from npg.porch.models.task import Task, TaskStateEnum -from npg.porchdb.connection import get_DbAccessor +from npg_porch.auth.token import validate +from npg_porch.models.permission import PermissionValidationException +from npg_porch.models.pipeline import Pipeline +from npg_porch.models.task import Task, TaskStateEnum +from npg_porch.db.connection import get_DbAccessor from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound from starlette import status diff --git a/src/npg/porch/models/__init__.py b/src/npg_porch/models/__init__.py similarity index 100% rename from src/npg/porch/models/__init__.py rename to src/npg_porch/models/__init__.py diff --git a/src/npg/porch/models/permission.py b/src/npg_porch/models/permission.py similarity index 98% rename from src/npg/porch/models/permission.py rename to src/npg_porch/models/permission.py index df5b62c..783235f 100644 --- a/src/npg/porch/models/permission.py +++ b/src/npg_porch/models/permission.py @@ -22,7 +22,7 @@ from pydantic import BaseModel, Field, field_validator, FieldValidationInfo from typing import Optional -from npg.porch.models.pipeline import Pipeline +from npg_porch.models.pipeline import Pipeline class PermissionValidationException(Exception): diff --git a/src/npg/porch/models/pipeline.py b/src/npg_porch/models/pipeline.py similarity index 100% rename from src/npg/porch/models/pipeline.py rename to src/npg_porch/models/pipeline.py diff --git a/src/npg/porch/models/task.py b/src/npg_porch/models/task.py similarity index 98% rename from src/npg/porch/models/task.py rename to src/npg_porch/models/task.py index 788518d..df3f01e 100644 --- a/src/npg/porch/models/task.py +++ b/src/npg_porch/models/task.py @@ -23,7 +23,7 @@ import ujson from pydantic import BaseModel, Field -from npg.porch.models.pipeline import Pipeline +from npg_porch.models.pipeline import Pipeline class TaskStateEnum(str, Enum): PENDING = 'PENDING' diff --git a/src/npg/porch_server.py b/src/npg_porch/server.py similarity index 97% rename from src/npg/porch_server.py rename to src/npg_porch/server.py index 836928b..c7a9f8a 100644 --- a/src/npg/porch_server.py +++ b/src/npg_porch/server.py @@ -21,7 +21,7 @@ from fastapi import FastAPI from fastapi.responses import HTMLResponse -from npg.porch.endpoints import pipelines, tasks +from npg_porch.endpoints import pipelines, tasks #https://fastapi.tiangolo.com/tutorial/bigger-applications/ #https://fastapi.tiangolo.com/tutorial/metadata diff --git a/tests/data_access_test.py b/tests/data_access_test.py index c463b0c..bf1bccc 100644 --- a/tests/data_access_test.py +++ b/tests/data_access_test.py @@ -4,8 +4,8 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound -from npg.porchdb.data_access import AsyncDbAccessor -from npg.porch.models import Pipeline as ModelledPipeline, Task, TaskStateEnum +from npg_porch.db.data_access import AsyncDbAccessor +from npg_porch.models import Pipeline as ModelledPipeline, Task, TaskStateEnum def give_me_a_pipeline(number: int = 1): diff --git a/tests/db_auth_test.py b/tests/db_auth_test.py index e155452..625d493 100644 --- a/tests/db_auth_test.py +++ b/tests/db_auth_test.py @@ -2,16 +2,16 @@ import datetime from sqlalchemy import select -from npg.porchdb.models import Token, Pipeline -from npg.porchdb.auth import Validator, CredentialsValidationException -import npg.porch.models.permission -import npg.porch.models.pipeline +from npg_porch.db.models import Token, Pipeline +from npg_porch.db.auth import Validator, CredentialsValidationException +import npg_porch.models.permission +import npg_porch.models.pipeline @pytest.mark.asyncio async def test_token_string_is_valid(async_minimum): v = Validator(session = async_minimum) - assert isinstance(v, (npg.porchdb.auth.Validator)) + assert isinstance(v, (npg_porch.db.auth.Validator)) with pytest.raises(CredentialsValidationException, match=r'The token should be 32 chars long'): @@ -76,15 +76,15 @@ async def test_permission_object_is_returned(async_minimum): for t in token_rows: if t.description == 'Seqfarm host, job runner': p = await v.token2permission(t.token) - assert isinstance(p, (npg.porch.models.permission.Permission)) + assert isinstance(p, (npg_porch.models.permission.Permission)) assert p.pipeline is not None - assert isinstance(p.pipeline, (npg.porch.models.pipeline.Pipeline)) + assert isinstance(p.pipeline, (npg_porch.models.pipeline.Pipeline)) assert p.pipeline.name == 'ptest one' assert p.requestor_id == t.token_id assert p.role == 'regular_user' elif t.description == 'Seqfarm host, admin': p = await v.token2permission(t.token) - assert isinstance(p, (npg.porch.models.permission.Permission)) + assert isinstance(p, (npg_porch.models.permission.Permission)) assert p.pipeline is None assert p.requestor_id == t.token_id assert p.role == 'power_user' diff --git a/tests/db_task_test.py b/tests/db_task_test.py index 6672ea8..e1c9ec5 100644 --- a/tests/db_task_test.py +++ b/tests/db_task_test.py @@ -1,7 +1,7 @@ import pytest from sqlalchemy import select -from npg.porchdb.models import Task +from npg_porch.db.models import Task @pytest.mark.asyncio async def test_task_creation(async_minimum): diff --git a/tests/db_token_test.py b/tests/db_token_test.py index a7edbc0..0197b04 100644 --- a/tests/db_token_test.py +++ b/tests/db_token_test.py @@ -1,7 +1,7 @@ import pytest from sqlalchemy import select -from npg.porchdb.models import Token +from npg_porch.db.models import Token @pytest.mark.asyncio async def test_token_creation(async_minimum): diff --git a/tests/fixtures/deploy_db.py b/tests/fixtures/deploy_db.py index 78c9cee..ffecb99 100644 --- a/tests/fixtures/deploy_db.py +++ b/tests/fixtures/deploy_db.py @@ -3,12 +3,12 @@ import pytest_asyncio from starlette.testclient import TestClient -from npg.porchdb.models import ( +from npg_porch.db.models import ( Pipeline, Task, Event, Token ) -from npg.porchdb.data_access import AsyncDbAccessor -from npg.porch.models import Task as ModelledTask, TaskStateEnum -from npg.porch_server import app +from npg_porch.db.data_access import AsyncDbAccessor +from npg_porch.models import Task as ModelledTask, TaskStateEnum +from npg_porch.server import app @pytest.fixture def minimum_data(): diff --git a/tests/fixtures/orm_session.py b/tests/fixtures/orm_session.py index 7cdbb09..3d5215f 100644 --- a/tests/fixtures/orm_session.py +++ b/tests/fixtures/orm_session.py @@ -4,8 +4,8 @@ import sqlalchemy import sqlalchemy.orm -from npg.porchdb.models import Base -from npg.porchdb.connection import session_factory, deploy_schema, close_engine +from npg_porch.db.models import Base +from npg_porch.db.connection import session_factory, deploy_schema, close_engine @pytest.fixture diff --git a/tests/init_test.py b/tests/init_test.py index 0f67135..9853441 100644 --- a/tests/init_test.py +++ b/tests/init_test.py @@ -1,7 +1,7 @@ import pytest from sqlalchemy import select -from npg.porchdb.models import Pipeline +from npg_porch.db.models import Pipeline def test_fixture(sync_minimum): diff --git a/tests/model_permission_test.py b/tests/model_permission_test.py index 23d3716..80c42b4 100644 --- a/tests/model_permission_test.py +++ b/tests/model_permission_test.py @@ -1,7 +1,7 @@ import pytest -from npg.porch.models.pipeline import Pipeline -from npg.porch.models.permission import Permission, PermissionValidationException +from npg_porch.models.pipeline import Pipeline +from npg_porch.models.permission import Permission, PermissionValidationException from pydantic import ValidationError diff --git a/tests/pipeline_route_test.py b/tests/pipeline_route_test.py index bf0f5d8..fc455db 100644 --- a/tests/pipeline_route_test.py +++ b/tests/pipeline_route_test.py @@ -1,6 +1,6 @@ from starlette import status -from npg.porch.models import Pipeline +from npg_porch.models import Pipeline headers = { diff --git a/tests/task_route_test.py b/tests/task_route_test.py index c24bd6a..043b40d 100644 --- a/tests/task_route_test.py +++ b/tests/task_route_test.py @@ -1,6 +1,6 @@ from starlette import status -from npg.porch.models import Task, TaskStateEnum, Pipeline +from npg_porch.models import Task, TaskStateEnum, Pipeline # Not testing get-all-tasks as this method will ultimately go From 3dc42393482627e37de8fd5e872ab4dab61763cd Mon Sep 17 00:00:00 2001 From: mgcam Date: Thu, 18 Apr 2024 20:48:48 +0100 Subject: [PATCH 05/25] Standard project layout does not need setup.cfg --- setup.cfg | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 setup.cfg diff --git a/setup.cfg b/setup.cfg deleted file mode 100644 index e4b61c5..0000000 --- a/setup.cfg +++ /dev/null @@ -1,11 +0,0 @@ -[metadata] -name = npg_porch -version = 1.0.0 - -[options] -package_dir = - =src -packages = find: - -[options.packages.find] -where=src From f374479ab3f25f57c6486f710b94c8a54efa20d8 Mon Sep 17 00:00:00 2001 From: mgcam Date: Thu, 18 Apr 2024 21:05:25 +0100 Subject: [PATCH 06/25] Moved tests config file to the tests directory --- pytest.ini => tests/pytest.ini | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename pytest.ini => tests/pytest.ini (100%) diff --git a/pytest.ini b/tests/pytest.ini similarity index 100% rename from pytest.ini rename to tests/pytest.ini From 7dfcdf7c5373a1c9b833e435594963180508ad91 Mon Sep 17 00:00:00 2001 From: mgcam Date: Thu, 18 Apr 2024 21:13:00 +0100 Subject: [PATCH 07/25] Dropped superfluous file --- tests/__init__.py | 0 tests/conftest.py | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 tests/__init__.py diff --git a/tests/__init__.py b/tests/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/conftest.py b/tests/conftest.py index 0a4ef75..337f633 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,5 @@ -from .fixtures.orm_session import sync_session, async_session -from .fixtures.deploy_db import ( +from fixtures.orm_session import sync_session, async_session +from fixtures.deploy_db import ( sync_minimum, async_minimum, minimum_data, From 5e6875320ae6487efc4bc02a21ad8669c53baae9 Mon Sep 17 00:00:00 2001 From: mgcam Date: Fri, 19 Apr 2024 09:52:53 +0100 Subject: [PATCH 08/25] Removed init file from the src directory --- src/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 src/__init__.py diff --git a/src/__init__.py b/src/__init__.py deleted file mode 100644 index e69de29..0000000 From efb667f0de79b8a6d6dfc7c5671db91af24f6142 Mon Sep 17 00:00:00 2001 From: Keith James Date: Thu, 13 Jun 2024 16:12:08 +0100 Subject: [PATCH 09/25] Add a note about installing on macOS --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index 89f395c..26b7f64 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,16 @@ uvicorn npg_porch.server:app --host 0.0.0.0 --port 8080 --reload --log-config lo and open your browser at `http://localhost:8080` to see links to the docs. +On macOS you will need to ensure that a version of the `sqlite3` library that supports SQLite extensions +is used when installing the `pysqlite3` package. The system library on macOS does not, so an alternative +such as the one provided by MacPorts or Homebrew should be used. For example, when using MacPorts this +can be done by setting the `CPPFLAGS` environment variable before running the `pip install` command: + +``` +export CPPFLAGS="-I/opt/local/include" +``` + + The server will not start without `DB_URL` in the environment ## Running in production From f5bc68b47714205fa20d201f2be6fc137adc7dec Mon Sep 17 00:00:00 2001 From: Keith James Date: Fri, 14 Jun 2024 11:50:10 +0100 Subject: [PATCH 10/25] Add an endpoint for creating pipeline tokens Add a POST endpoint to be used with the admin token to create tokens for a pipeline. There is no limit on the number of tokens that can be created for a particular pipeline. --- docs/user_guide.md | 8 +++++-- src/npg_porch/db/data_access.py | 15 +++++++++++- src/npg_porch/endpoints/pipelines.py | 30 ++++++++++++++++++++++-- tests/pipeline_route_test.py | 34 ++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 5 deletions(-) diff --git a/docs/user_guide.md b/docs/user_guide.md index e677a3d..4d261a8 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -22,19 +22,23 @@ Security is necessary in order to prevent accidental misuse of npg_porch. An aut A note on HTTPS: Client libraries like `requests`, certain GUIs and Firefox will try to verify the server certificate authority. System-administered software are already configured correctly, but other packages installed by conda or pip may need to be told how the client may verify with a client certificate e.g. contained in `/usr/share/ca-certificates/`. It may also be useful to unset `https_proxy` and `HTTPS_PROXY` in your environment. -### Step 0 - get issued security tokens +### Step 0 - get issued authorisation tokens Access to the service is loosely controlled with authorisation tokens. You will be issued with an admin token that enables you to register pipelines, and further tokens for pipeline-specific communication. Please do not share the tokens around and use them for purposes besides the specific pipeline. This will help us to monitor pipeline reliability and quality of service. Authorisation is achieved by HTTP Bearer Token: `curl -L -H "Authorization: Bearer $TOKEN" https://$SERVER:$PORT` +Authorisation tokens are specific to a pipeline and more than one token can be issued for a pipeline. New tokens for a pipeline can be obtained using the admin token, from the pipeline's token endpoint: + +`curl -L -X POST -H "Authorization: Bearer $ADMIN_TOKEN" https://$SERVER:$PORT/pipelines/$PIPELINE_NAME/token/$TOKEN_DESCRIPTION` + ### Step 1 - register your pipeline with npg_porch *Schema: npg_porch.model.pipeline* Nothing in npg_porch can happen until there's a pipeline defined. For our purposes "pipeline" means "a thing you can run", and it may refer to specific code, or a wrapper that can run the pipeline in this particular way with some standard arguments. -You can name your pipeline however you like, but the name must be unique, and be as informative to you as possible. Version and a URI can be useful for undestanding what code is being run. +You can name your pipeline however you like, but the name must be unique, and be as informative to you as possible. Version and a URI can be useful for understanding what code is being run. **pipeline-def.json** diff --git a/src/npg_porch/db/data_access.py b/src/npg_porch/db/data_access.py index 67ad9c3..0bb2931 100644 --- a/src/npg_porch/db/data_access.py +++ b/src/npg_porch/db/data_access.py @@ -1,4 +1,4 @@ -# Copyright (C) 2021, 2022 Genome Research Ltd. +# Copyright (C) 2021, 2022, 2024 Genome Research Ltd. # # Author: Kieron Taylor kt19@sanger.ac.uk # Author: Marina Gourtovaia mg8@sanger.ac.uk @@ -27,6 +27,8 @@ from npg_porch.db.models import Pipeline as DbPipeline, Task as DbTask, Event from npg_porch.models import Task, Pipeline, TaskStateEnum +from npg_porch.db.models import Token + class AsyncDbAccessor: ''' @@ -92,6 +94,17 @@ async def create_pipeline(self, pipeline: Pipeline) -> Pipeline: await session.commit() return pipe.convert_to_model() + + async def create_pipeline_token(self, name: str, desc: str) -> str: + session = self.session + db_pipeline = await self._get_pipeline_db_object(name) + + token = Token(pipeline=db_pipeline, description=desc) + session.add(token) + await session.commit() + return token.token + + async def create_task(self, token_id: int, task: Task) -> Task: self.logger.debug('CREATE TASK: ' + str(task)) session = self.session diff --git a/src/npg_porch/endpoints/pipelines.py b/src/npg_porch/endpoints/pipelines.py index 9a99604..efe4d2f 100644 --- a/src/npg_porch/endpoints/pipelines.py +++ b/src/npg_porch/endpoints/pipelines.py @@ -1,4 +1,4 @@ -# Copyright (C) 2021, 2022 Genome Research Ltd. +# Copyright (C) 2021, 2022, 2024 Genome Research Ltd. # # Author: Kieron Taylor kt19@sanger.ac.uk # Author: Marina Gourtovaia mg8@sanger.ac.uk @@ -18,7 +18,7 @@ # You should have received a copy of the GNU General Public License along with # this program. If not, see . -from fastapi import APIRouter, HTTPException, Depends +from fastapi import APIRouter, HTTPException, Depends, Response import logging import re from sqlalchemy.exc import IntegrityError @@ -84,6 +84,32 @@ async def get_pipeline( return pipeline +@router.post( + "/{pipeline_name}/token/{token_desc}", + response_model=str, + responses={ + status.HTTP_201_CREATED: {"description": "Token was created"}, + status.HTTP_404_NOT_FOUND: {"description": "Pipeline not found"}, + }, + summary="Create a new token for a pipeline and return it.", + description=""" + Returns a token for the pipeline with the given name. + A valid token issued for any pipeline is required for authorisation.""" +) +async def create_pipeline_token( + pipeline_name: str, + token_desc: str, + db_accessor=Depends(get_DbAccessor), + permissions=Depends(validate) +) -> Response: + try: + token = await db_accessor.create_pipeline_token(name=pipeline_name, desc=token_desc) + except NoResultFound: + raise HTTPException(status_code=404, + detail=f"Pipeline '{pipeline_name}' not found") + return Response(status_code=status.HTTP_201_CREATED, content=token, media_type="text/plain") + + @router.post( "/", response_model=Pipeline, diff --git a/tests/pipeline_route_test.py b/tests/pipeline_route_test.py index fc455db..7eed67e 100644 --- a/tests/pipeline_route_test.py +++ b/tests/pipeline_route_test.py @@ -152,3 +152,37 @@ def test_create_pipeline(async_minimum, fastapi_testclient): headers=headers4power_user ) assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_create_pipeline_token(async_minimum, fastapi_testclient): + pipeline_name = "ptest four" + token_desc = "this is a token" + + # Create a pipeline + desired_pipeline = Pipeline( + name=pipeline_name, + uri='http://test.com', + version='1' + ) + + http_create_pipeline(fastapi_testclient, desired_pipeline) + + # Create new tokens for the pipeline. There is no limit on the number of tokens + # that can be created and no restriction on token description. + for _ in range(3): + response = fastapi_testclient.post( + f"/pipelines/{pipeline_name}/token/{token_desc}", + follow_redirects=True, + headers=headers4power_user + ) + + assert response.status_code == status.HTTP_201_CREATED + assert len(response.content.decode(encoding="utf-8")) == 32 + + # Create a new token for a non-existent pipeline + response = fastapi_testclient.post( + "/pipelines/not here/token/{token_desc}", + follow_redirects=True, + headers=headers4power_user + ) + assert response.status_code == status.HTTP_404_NOT_FOUND From 1c4b9988e080f39c8364609212378278a8b5b5b1 Mon Sep 17 00:00:00 2001 From: Keith James Date: Fri, 14 Jun 2024 12:16:38 +0100 Subject: [PATCH 11/25] Fix pipelines endpoint to return HTTP 422 on bad input rather than 409 Add model validation to the Pydantic class to forbid extra fields. This prevents the endpoint returning that a pipeline already exists (409) when it fails to create a row in the database due to constraint violation (NULL because of a misspelled field). --- src/npg_porch/models/pipeline.py | 6 ++++-- tests/pipeline_route_test.py | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/npg_porch/models/pipeline.py b/src/npg_porch/models/pipeline.py index ce212b8..ffdea9b 100644 --- a/src/npg_porch/models/pipeline.py +++ b/src/npg_porch/models/pipeline.py @@ -1,4 +1,4 @@ -# Copyright (C) 2021, 2022 Genome Research Ltd. +# Copyright (C) 2021, 2022, 2024 Genome Research Ltd. # # Author: Kieron Taylor kt19@sanger.ac.uk # Author: Marina Gourtovaia mg8@sanger.ac.uk @@ -18,9 +18,11 @@ # You should have received a copy of the GNU General Public License along with # this program. If not, see . -from pydantic import BaseModel, Field +from pydantic import BaseModel, ConfigDict, Field class Pipeline(BaseModel): + model_config = ConfigDict(extra='forbid') + name: str = Field( default = None, title='Pipeline Name', diff --git a/tests/pipeline_route_test.py b/tests/pipeline_route_test.py index fc455db..eb559a2 100644 --- a/tests/pipeline_route_test.py +++ b/tests/pipeline_route_test.py @@ -1,3 +1,5 @@ +import json + from starlette import status from npg_porch.models import Pipeline @@ -100,6 +102,16 @@ def test_get_known_pipeline(async_minimum, fastapi_testclient): def test_create_pipeline(async_minimum, fastapi_testclient): + invalid_pipeline = { + "name": "ptest one", + "url": "http://test.com", # URL, not URI + "version": "1" + } + response = fastapi_testclient.post( + "/pipelines", json=json.dumps(invalid_pipeline), follow_redirects=True, + headers=headers4power_user + ) + assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY # Create a pipeline desired_pipeline = Pipeline( From ffbb100d6c69e49523c5747b978b39bb9dd8ccfd Mon Sep 17 00:00:00 2001 From: Keith James Date: Fri, 14 Jun 2024 15:23:26 +0100 Subject: [PATCH 12/25] Add a development Dockerfile --- docker/Dockerfile.dev | 99 ++++++++++++++++++++ docker/README.md | 19 ++++ docker/logging.json | 51 ++++++++++ docker/scripts/configure_database_service.sh | 12 +++ docker/scripts/create_database.sh | 30 ++++++ docker/scripts/entrypoint.sh | 16 ++++ docker/scripts/insert_admin_token.sh | 18 ++++ scripts/deploy_schema.py | 2 + 8 files changed, 247 insertions(+) create mode 100644 docker/Dockerfile.dev create mode 100644 docker/README.md create mode 100644 docker/logging.json create mode 100755 docker/scripts/configure_database_service.sh create mode 100755 docker/scripts/create_database.sh create mode 100755 docker/scripts/entrypoint.sh create mode 100755 docker/scripts/insert_admin_token.sh mode change 100644 => 100755 scripts/deploy_schema.py diff --git a/docker/Dockerfile.dev b/docker/Dockerfile.dev new file mode 100644 index 0000000..cf4f678 --- /dev/null +++ b/docker/Dockerfile.dev @@ -0,0 +1,99 @@ + +ARG BASE_IMAGE=python:3.10-slim +FROM $BASE_IMAGE as builder + +ARG DEBIAN_FRONTEND="noninteractive" + +RUN apt-get update && \ + apt-get install -q -y --no-install-recommends \ + build-essential \ + gcc \ + libsqlite3-dev \ + unattended-upgrades && \ + unattended-upgrade -v + +WORKDIR /app + +COPY .. . + +RUN python -m venv /app && \ + . ./bin/activate && \ + pip install --no-cache-dir --upgrade pip && \ + pip install --no-cache-dir . + + +FROM $BASE_IMAGE + +ARG DEBIAN_FRONTEND + +RUN apt-get update && \ + apt-get upgrade -y && \ + apt-get install -q -y --no-install-recommends \ + libsqlite3-0 \ + postgresql \ + sudo \ + tini \ + locales && \ + locale-gen en_GB en_GB.UTF-8 && \ + localedef -i en_GB -c -f UTF-8 -A /usr/share/locale/locale.alias en_GB.UTF-8 + +RUN apt-get install -q -y --no-install-recommends \ + unattended-upgrades && \ + unattended-upgrade -v && \ + apt-get remove -q -y unattended-upgrades && \ + apt-get autoremove -q -y && \ + apt-get clean -q -y && \ + rm -rf /var/lib/apt/lists/* + +ENV LANG=en_GB.UTF-8 \ + LANGUAGE=en_GB \ + LC_ALL=en_GB.UTF-8 \ + TZ="Etc/UTC" + +ARG APP_USER=appuser +ARG APP_UID=1000 +ARG APP_GID=$APP_UID + +WORKDIR /app + +RUN groupadd --gid $APP_GID $APP_USER && \ + useradd --uid $APP_UID --gid $APP_GID --shell /bin/bash --create-home $APP_USER + +COPY --from=builder --chown=$APP_USER:$APP_GID /app /app + +ARG DB_HOST=localhost +ARG DB_PORT=5432 +ARG DB_SCHEMA=porch_dev +ARG DB_NAME=porch_dev_db +ARG DB_USER=porch_admin +ARG DB_PASS=porch +ARG URL_SLUG="$DB_USER:$DB_PASS@$DB_HOST:$DB_PORT/$DB_NAME" + +ENV DB_HOST=$DB_HOST \ + DB_PORT=$DB_PORT \ + DB_SCHEMA=$DB_SCHEMA \ + DB_NAME=$DB_NAME \ + DB_USER=$DB_USER \ + DB_PASS=$DB_PASS \ + DB_URL="postgresql+psycopg2://$URL_SLUG" + +RUN service postgresql start && \ + /app/docker/scripts/create_database.sh && \ + /app/docker/scripts/configure_database_service.sh && \ + . /app/bin/activate && \ + /app/scripts/deploy_schema.py && \ + /app/docker/scripts/insert_admin_token.sh && \ + service postgresql stop + +USER $APP_USER + +ARG PORT=8081 + +ENV DB_URL="postgresql+asyncpg://$URL_SLUG" \ + PORT=${PORT} + +EXPOSE ${PORT} + +ENTRYPOINT ["/usr/bin/tini", "--"] + +CMD ["/app/docker/scripts/entrypoint.sh"] diff --git a/docker/README.md b/docker/README.md new file mode 100644 index 0000000..73b0fe2 --- /dev/null +++ b/docker/README.md @@ -0,0 +1,19 @@ +# Development Dockerfile + +The Dockerfile and scripts in this directory may be use to create a development image +that hosts both the PostgreSQL database and the npg_porch server. + +The application is populated with a hard-coded administrator user, password and +administration token and is configured log to STDERR and STDOUT. + +To create an image using the Dockerfile, run the following command from the root of the +repository: + +```bash +docker build --rm -f docker/Dockerfile.dev -t npg_porch_dev . +``` + +The Dockerfile supports a number of arguments that can be passed to the `docker build` +command to configure most aspects of the application, including user names, passwords, +database names and ports. However, the default values should be suitable for most +needs. diff --git a/docker/logging.json b/docker/logging.json new file mode 100644 index 0000000..53723a0 --- /dev/null +++ b/docker/logging.json @@ -0,0 +1,51 @@ +{ + "version": 1, + "formatters": { + "default": { + "()": "uvicorn.logging.DefaultFormatter", + "fmt": "%(levelprefix)s %(message)s", + "use_colors": null + }, + "access": { + "()": "uvicorn.logging.AccessFormatter", + "fmt": "%(levelprefix)s %(client_addr)s - \"%(request_line)s\" %(status_code)s" + } + }, + "handlers": { + "stderr": { + "formatter": "default", + "class": "logging.StreamHandler", + "stream": "ext://sys.stderr" + }, + "stdout": { + "formatter": "default", + "class": "logging.StreamHandler", + "stream": "ext://sys.stdout" + }, + "access": { + "formatter": "access", + "class": "logging.StreamHandler", + "stream": "ext://sys.stdout" + } + }, + "loggers": { + "uvicorn": { + "handlers": ["access"], + "level": "INFO", + "propagate": false + }, + "uvicorn.error": { + "handlers": ["stderr"], + "level": "DEBUG", + "propagate": false + }, + "fastapi": { + "handlers": ["stderr"], + "level": "INFO" + } + }, + "root": { + "handlers": ["stdout"], + "level": "DEBUG" + } +} diff --git a/docker/scripts/configure_database_service.sh b/docker/scripts/configure_database_service.sh new file mode 100755 index 0000000..7df868c --- /dev/null +++ b/docker/scripts/configure_database_service.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +set -eo pipefail +set -x + +APP_USER=${APP_USER:? The APP_USER environment variable must be set} + +cat > "/etc/sudoers.d/$APP_USER" << EOF +$APP_USER ALL= NOPASSWD: /usr/sbin/service postgresql start +$APP_USER ALL= NOPASSWD: /usr/sbin/service postgresql restart +$APP_USER ALL= NOPASSWD: /usr/sbin/service postgresql stop +EOF diff --git a/docker/scripts/create_database.sh b/docker/scripts/create_database.sh new file mode 100755 index 0000000..52d094b --- /dev/null +++ b/docker/scripts/create_database.sh @@ -0,0 +1,30 @@ +#!/bin/bash + +set -eo pipefail +set -x + +pg_isready --quiet || { + echo "PostgreSQL is not ready" >&2 + exit 1 +} + +DB_SCHEMA=${DB_SCHEMA:? The DB_SCHEMA environment variable must be set} +DB_NAME=${DB_NAME:? The DB_NAME environment variable must be set} +DB_USER=${DB_USER:? The DB_USER environment variable must be set} +DB_PASS=${DB_PASS:? The DB_PASS environment variable must be set} + +sudo -u postgres createuser -D -R -S ${DB_USER} +sudo -u postgres createdb -O ${DB_USER} ${DB_NAME} + +sudo -u postgres psql -d ${DB_NAME} << EOF +ALTER USER ${DB_USER} WITH PASSWORD '${DB_PASS}'; + +CREATE SCHEMA ${DB_SCHEMA}; + +SET search_path TO ${DB_SCHEMA}, public; + +GRANT ALL PRIVILEGES ON SCHEMA ${DB_SCHEMA} TO ${DB_USER}; +GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA ${DB_SCHEMA} TO ${DB_USER}; +GRANT USAGE ON ALL SEQUENCES IN SCHEMA ${DB_SCHEMA} TO ${DB_USER}; +GRANT SELECT, INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA ${DB_SCHEMA} TO ${DB_USER}; +EOF diff --git a/docker/scripts/entrypoint.sh b/docker/scripts/entrypoint.sh new file mode 100755 index 0000000..777f43a --- /dev/null +++ b/docker/scripts/entrypoint.sh @@ -0,0 +1,16 @@ +#!/bin/bash + +set -eo pipefail + +sudo service postgresql start + +pg_isready --quiet --timeout=30 || { + echo "PostgreSQL is not ready" >&2 + exit 1 +} + +PORT=${PORT:? The PORT environment variable must be set} + +source /app/bin/activate + +uvicorn npg_porch.server:app --host 0.0.0.0 --port ${PORT} --reload --log-config /app/docker/logging.json diff --git a/docker/scripts/insert_admin_token.sh b/docker/scripts/insert_admin_token.sh new file mode 100755 index 0000000..34ef944 --- /dev/null +++ b/docker/scripts/insert_admin_token.sh @@ -0,0 +1,18 @@ +#!/bin/bash + +set -eo pipefail +set -x + +pg_isready --quiet || { + echo "PostgreSQL is not ready" >&2 + exit 1 +} + +DB_SCHEMA=${DB_SCHEMA:? The DB_SCHEMA environment variable must be set} +DB_NAME=${DB_NAME:? The DB_NAME environment variable must be set} + +ADMIN_TOKEN=${ADMIN_TOKEN:="00000000000000000000000000000000"} + +sudo -u postgres psql -d ${DB_NAME} << EOF +INSERT INTO ${DB_SCHEMA}."token" (token, description, date_issued) VALUES ('${ADMIN_TOKEN}', 'Admin token', NOW()); +EOF diff --git a/scripts/deploy_schema.py b/scripts/deploy_schema.py old mode 100644 new mode 100755 index ea7adf4..2d264ed --- a/scripts/deploy_schema.py +++ b/scripts/deploy_schema.py @@ -1,3 +1,5 @@ +#!/usr/bin/env python + # Replace with Alembic in due course import os From 18d5ea61a41e9a42b942064752943f6f22b90f72 Mon Sep 17 00:00:00 2001 From: Keith James Date: Mon, 17 Jun 2024 11:49:17 +0100 Subject: [PATCH 13/25] Add a Token model and return this instead of a bare string This makes the token creation endpoint better aligned with the others in behaviour (all now return JSON with correct Content-Type for both success and HTTP errors). The token JSON includes the relevant pipeline name and token description, an improvment over anonymous strings. --- src/npg_porch/db/data_access.py | 15 +++++------ src/npg_porch/db/models/token.py | 4 +-- src/npg_porch/endpoints/pipelines.py | 9 ++++--- src/npg_porch/models/token.py | 38 ++++++++++++++++++++++++++++ tests/pipeline_route_test.py | 8 +++--- 5 files changed, 57 insertions(+), 17 deletions(-) create mode 100644 src/npg_porch/models/token.py diff --git a/src/npg_porch/db/data_access.py b/src/npg_porch/db/data_access.py index 0bb2931..a55cf45 100644 --- a/src/npg_porch/db/data_access.py +++ b/src/npg_porch/db/data_access.py @@ -24,10 +24,9 @@ from sqlalchemy.orm import contains_eager, joinedload from sqlalchemy.orm.exc import NoResultFound -from npg_porch.db.models import Pipeline as DbPipeline, Task as DbTask, Event +from npg_porch.db.models import Pipeline as DbPipeline, Task as DbTask, Token as DbToken, Event from npg_porch.models import Task, Pipeline, TaskStateEnum - -from npg_porch.db.models import Token +from npg_porch.models.token import Token class AsyncDbAccessor: @@ -94,15 +93,15 @@ async def create_pipeline(self, pipeline: Pipeline) -> Pipeline: await session.commit() return pipe.convert_to_model() - - async def create_pipeline_token(self, name: str, desc: str) -> str: + async def create_pipeline_token(self, name: str, desc: str) -> Token: session = self.session db_pipeline = await self._get_pipeline_db_object(name) - token = Token(pipeline=db_pipeline, description=desc) - session.add(token) + db_token = DbToken(pipeline=db_pipeline, description=desc) + session.add(db_token) await session.commit() - return token.token + + return Token(name=db_pipeline.name, token=db_token.token, description=desc) async def create_task(self, token_id: int, task: Task) -> Task: diff --git a/src/npg_porch/db/models/token.py b/src/npg_porch/db/models/token.py index db9bdd2..aa3d725 100644 --- a/src/npg_porch/db/models/token.py +++ b/src/npg_porch/db/models/token.py @@ -29,11 +29,11 @@ class Token(Base): ''' - A string token ussued to client applications for the purpose of + A string token issued to client applications for the purpose of authorizing them to perform certain actions. ''' - def random_token(): + def random_token(self): ''' Returns a 32 characters long random string. The chance of a collision is small. diff --git a/src/npg_porch/endpoints/pipelines.py b/src/npg_porch/endpoints/pipelines.py index efe4d2f..9a9588f 100644 --- a/src/npg_porch/endpoints/pipelines.py +++ b/src/npg_porch/endpoints/pipelines.py @@ -29,7 +29,7 @@ from npg_porch.models.permission import RolesEnum from npg_porch.db.connection import get_DbAccessor from npg_porch.auth.token import validate - +from npg_porch.models.token import Token router = APIRouter( prefix="/pipelines", @@ -86,7 +86,8 @@ async def get_pipeline( @router.post( "/{pipeline_name}/token/{token_desc}", - response_model=str, + response_model=Token, + status_code=status.HTTP_201_CREATED, responses={ status.HTTP_201_CREATED: {"description": "Token was created"}, status.HTTP_404_NOT_FOUND: {"description": "Pipeline not found"}, @@ -101,13 +102,13 @@ async def create_pipeline_token( token_desc: str, db_accessor=Depends(get_DbAccessor), permissions=Depends(validate) -) -> Response: +) -> Token: try: token = await db_accessor.create_pipeline_token(name=pipeline_name, desc=token_desc) except NoResultFound: raise HTTPException(status_code=404, detail=f"Pipeline '{pipeline_name}' not found") - return Response(status_code=status.HTTP_201_CREATED, content=token, media_type="text/plain") + return token @router.post( diff --git a/src/npg_porch/models/token.py b/src/npg_porch/models/token.py new file mode 100644 index 0000000..c84e0dd --- /dev/null +++ b/src/npg_porch/models/token.py @@ -0,0 +1,38 @@ +# Copyright (C) 2024 Genome Research Ltd. +# +# This file is part of npg_porch +# +# npg_porch is free software: you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the Free +# Software Foundation; either version 3 of the License, or (at your option) any +# later version. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program. If not, see . + +from pydantic import BaseModel, ConfigDict, Field + + +class Token(BaseModel): + model_config = ConfigDict(extra='forbid') + + name: str = Field( + default=None, + title='Pipeline Name', + description='A user-controlled name of an existing pipeline' + ) + description: str | None = Field( + default=None, + title='Description', + description='A user-controlled description of the token' + ) + token: str | None = Field( + default=None, + title='Token', + description='An authorisation token' + ) diff --git a/tests/pipeline_route_test.py b/tests/pipeline_route_test.py index 606eb2f..19feab4 100644 --- a/tests/pipeline_route_test.py +++ b/tests/pipeline_route_test.py @@ -173,8 +173,8 @@ def test_create_pipeline_token(async_minimum, fastapi_testclient): # Create a pipeline desired_pipeline = Pipeline( name=pipeline_name, - uri='http://test.com', - version='1' + uri="http://test.com", + version="1" ) http_create_pipeline(fastapi_testclient, desired_pipeline) @@ -189,7 +189,9 @@ def test_create_pipeline_token(async_minimum, fastapi_testclient): ) assert response.status_code == status.HTTP_201_CREATED - assert len(response.content.decode(encoding="utf-8")) == 32 + assert response.json()["name"] == pipeline_name + assert response.json()["description"] == token_desc + assert len(response.json()["token"]) == 32 # Create a new token for a non-existent pipeline response = fastapi_testclient.post( From 7b3d42a21d6c36b86bba4eb2a30fa38b803a12ec Mon Sep 17 00:00:00 2001 From: Keith James Date: Mon, 17 Jun 2024 12:19:22 +0100 Subject: [PATCH 14/25] Remove variable assignments that are never used --- src/npg_porch/auth/token.py | 2 +- src/npg_porch/db/auth.py | 2 -- src/npg_porch/db/data_access.py | 1 - src/npg_porch/endpoints/pipelines.py | 1 - src/npg_porch/endpoints/tasks.py | 4 ++-- tests/data_access_test.py | 6 +++--- 6 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/npg_porch/auth/token.py b/src/npg_porch/auth/token.py index 0fa4ed9..e495d19 100644 --- a/src/npg_porch/auth/token.py +++ b/src/npg_porch/auth/token.py @@ -34,7 +34,7 @@ async def validate( ): token = creds.credentials - p = None + try: p = await validator.token2permission(token) except CredentialsValidationException as e: diff --git a/src/npg_porch/db/auth.py b/src/npg_porch/db/auth.py index 364ee21..9bf9029 100644 --- a/src/npg_porch/db/auth.py +++ b/src/npg_porch/db/auth.py @@ -55,7 +55,6 @@ async def token2permission(self, token: str): 'Token failed character validation' ) - valid_token_row = None try: # Using 'outerjoin' to get the left join for token, pipeline. # We need to retrieve all token rows, regardless of whether @@ -74,7 +73,6 @@ async def token2permission(self, token: str): if (valid_token_row is not None) and (valid_token_row.date_revoked is not None): raise CredentialsValidationException('A revoked token is used') - permission = None pipeline = valid_token_row.pipeline token_id = valid_token_row.token_id if pipeline is None: diff --git a/src/npg_porch/db/data_access.py b/src/npg_porch/db/data_access.py index 0bb2931..ce1c783 100644 --- a/src/npg_porch/db/data_access.py +++ b/src/npg_porch/db/data_access.py @@ -76,7 +76,6 @@ async def get_all_pipelines( uri: str | None = None, version: str | None = None ) -> list[Pipeline]: - pipelines = [] pipelines = await self._get_pipeline_db_objects(uri=uri, version=version) return [pipe.convert_to_model() for pipe in pipelines] diff --git a/src/npg_porch/endpoints/pipelines.py b/src/npg_porch/endpoints/pipelines.py index efe4d2f..f37311a 100644 --- a/src/npg_porch/endpoints/pipelines.py +++ b/src/npg_porch/endpoints/pipelines.py @@ -134,7 +134,6 @@ async def create_pipeline( logging.error(f"Role {RolesEnum.POWER_USER} is required") raise HTTPException(status_code=403) - new_pipeline = None try: new_pipeline = await db_accessor.create_pipeline(pipeline) except IntegrityError as e: diff --git a/src/npg_porch/endpoints/tasks.py b/src/npg_porch/endpoints/tasks.py index b4aecd1..90072df 100644 --- a/src/npg_porch/endpoints/tasks.py +++ b/src/npg_porch/endpoints/tasks.py @@ -100,7 +100,7 @@ async def create_task( ) -> Task: _validate_request(permission, task.pipeline) - created_task = None + try: created_task = await db_accessor.create_task( token_id=permission.requestor_id, @@ -137,7 +137,7 @@ async def update_task( ) -> Task: _validate_request(permission, task.pipeline) - changed_task = None + try: changed_task = await db_accessor.update_task( token_id=permission.requestor_id, diff --git a/tests/data_access_test.py b/tests/data_access_test.py index bf1bccc..d8a6cc1 100644 --- a/tests/data_access_test.py +++ b/tests/data_access_test.py @@ -32,7 +32,7 @@ async def test_get_pipeline(db_accessor): db_accessor.get_pipeline_by_name() with pytest.raises(NoResultFound): - pipeline = await db_accessor.get_pipeline_by_name('not here') + await db_accessor.get_pipeline_by_name('not here') pipeline = await db_accessor.get_pipeline_by_name('ptest one') assert pipeline @@ -76,7 +76,7 @@ async def test_create_pipeline(db_accessor): assert saved_pipeline.uri == pipeline.uri with pytest.raises(AssertionError): - saved_pipeline = await db_accessor.create_pipeline({}) + await db_accessor.create_pipeline({}) with pytest.raises(IntegrityError) as exception: # Making duplicate provides a useful error await db_accessor.create_pipeline(pipeline) @@ -125,7 +125,7 @@ async def test_claim_tasks(db_accessor): pipeline = give_me_a_pipeline() with pytest.raises(NoResultFound) as exception: - tasks = await db_accessor.claim_tasks(1, pipeline) + await db_accessor.claim_tasks(1, pipeline) assert exception.value == 'Pipeline not found' From 15e2adca8458024a13f9ec88704a1e49b1f18432 Mon Sep 17 00:00:00 2001 From: Keith James Date: Mon, 17 Jun 2024 14:34:11 +0100 Subject: [PATCH 15/25] Update user guide for tokens --- docs/user_guide.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/user_guide.md b/docs/user_guide.md index 4d261a8..169eb55 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -32,6 +32,16 @@ Authorisation tokens are specific to a pipeline and more than one token can be i `curl -L -X POST -H "Authorization: Bearer $ADMIN_TOKEN" https://$SERVER:$PORT/pipelines/$PIPELINE_NAME/token/$TOKEN_DESCRIPTION` +The server will respond with a JSON document containing the new bearer token which you may use for subsequent pipeline-specific communication: + +```javascript +{ + "name": "$PIPELINE_NAME", + "description": "$TOKEN_DESCRIPTION", + "token": "$TOKEN" +} +``` + ### Step 1 - register your pipeline with npg_porch *Schema: npg_porch.model.pipeline* From eede4681bd03ee6826cde2fd89fa261316c52355 Mon Sep 17 00:00:00 2001 From: Keith James Date: Mon, 17 Jun 2024 14:44:42 +0100 Subject: [PATCH 16/25] Add Dependabot configuration for pip and GitHub Actions --- .github/dependabot.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..1a9554d --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,17 @@ +version: 2 +updates: +- package-ecosystem: "pip" + directory: "/" + schedule: + interval: "daily" + time: "00:00" + timezone: "Europe/London" + open-pull-requests-limit: 5 + +- package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "daily" + time: "00:00" + timezone: "Europe/London" + open-pull-requests-limit: 5 From a155a99d4f416863f07efb3359cdf1b52b372dd4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 26 Jun 2024 14:17:03 +0000 Subject: [PATCH 17/25] Bump actions/checkout from 3 to 4 Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/python-app.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 041de38..f87151b 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -15,7 +15,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Python 3.10 uses: actions/setup-python@v2 with: From 84006d3ec2000ba2e1ab28a1baa7e78bdbb41bb7 Mon Sep 17 00:00:00 2001 From: mgcam Date: Wed, 26 Jun 2024 16:50:19 +0100 Subject: [PATCH 18/25] Stopped errors for requests to create an existing tasks. Status 200 is returned instead of previously returned 409. --- src/npg_porch/db/data_access.py | 66 +++++++++++++++++++++------- src/npg_porch/endpoints/pipelines.py | 9 ++-- src/npg_porch/endpoints/tasks.py | 30 +++++++------ tests/data_access_test.py | 25 ++++++----- tests/task_route_test.py | 12 ++--- 5 files changed, 93 insertions(+), 49 deletions(-) diff --git a/src/npg_porch/db/data_access.py b/src/npg_porch/db/data_access.py index 9cd2e22..8281d18 100644 --- a/src/npg_porch/db/data_access.py +++ b/src/npg_porch/db/data_access.py @@ -19,13 +19,17 @@ # this program. If not, see . import logging + from sqlalchemy import select from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import contains_eager, joinedload from sqlalchemy.orm.exc import NoResultFound -from npg_porch.db.models import Pipeline as DbPipeline, Task as DbTask, Token as DbToken, Event -from npg_porch.models import Task, Pipeline, TaskStateEnum +from npg_porch.db.models import Event +from npg_porch.db.models import Pipeline as DbPipeline +from npg_porch.db.models import Task as DbTask +from npg_porch.db.models import Token as DbToken +from npg_porch.models import Pipeline, Task, TaskStateEnum from npg_porch.models.token import Token @@ -102,29 +106,44 @@ async def create_pipeline_token(self, name: str, desc: str) -> Token: return Token(name=db_pipeline.name, token=db_token.token, description=desc) + async def create_task(self, token_id: int, task: Task) -> tuple[Task, bool]: + '''Given a task definition creates a task. + + If the task does not exist, a tuple consisting of Task object for a + newly created database record and a boolean True object is returned. - async def create_task(self, token_id: int, task: Task) -> Task: + If the task already exists, a tuple consisting of Task object for an + existing database record and a boolean True object is returned. + ''' self.logger.debug('CREATE TASK: ' + str(task)) session = self.session db_pipeline = await self._get_pipeline_db_object( task.pipeline.name ) - # Check they exist and so on - task.status = TaskStateEnum.PENDING + task.status = TaskStateEnum.PENDING t = self.convert_task_to_db(task, db_pipeline) - session.add(t) - - event = Event( - task=t, - token_id = token_id, - change='Created' - ) - t.events.append(event) + nested = await session.begin_nested() + created = True + try: + session.add(t) + event = Event( + task=t, + token_id = token_id, + change='Created' + ) + t.events.append(event) + await session.commit() + except IntegrityError: + await nested.rollback() + # Task already exists, query the database to get the up-to-date + # representation of the task. + t = await self.get_db_task( + pipeline_name=task.pipeline.name, job_descriptor=t.job_descriptor + ) + created = False - await session.commit() - # Error handling to follow - return t.convert_to_model() + return (t.convert_to_model(), created) async def claim_tasks( self, token_id: int, pipeline: Pipeline, claim_limit: int | None = 1 @@ -228,6 +247,21 @@ async def get_tasks( tasks = task_result.scalars().all() return [t.convert_to_model() for t in tasks] + async def get_db_task( + self, + pipeline_name: str, + job_descriptor: str, + ) -> DbTask: + '''Get the task.''' + query = select(DbTask)\ + .join(DbTask.pipeline)\ + .options(joinedload(DbTask.pipeline))\ + .where(DbPipeline.name == pipeline_name)\ + .where(DbTask.job_descriptor == job_descriptor) + task_result = await self.session.execute(query) + + return task_result.scalars().one() + @staticmethod def convert_task_to_db(task: Task, pipeline: DbPipeline) -> DbTask: assert task.status in TaskStateEnum diff --git a/src/npg_porch/endpoints/pipelines.py b/src/npg_porch/endpoints/pipelines.py index ded6dbd..8ab140e 100644 --- a/src/npg_porch/endpoints/pipelines.py +++ b/src/npg_porch/endpoints/pipelines.py @@ -18,17 +18,18 @@ # You should have received a copy of the GNU General Public License along with # this program. If not, see . -from fastapi import APIRouter, HTTPException, Depends, Response import logging import re + +from fastapi import APIRouter, Depends, HTTPException from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound from starlette import status -from npg_porch.models.pipeline import Pipeline -from npg_porch.models.permission import RolesEnum -from npg_porch.db.connection import get_DbAccessor from npg_porch.auth.token import validate +from npg_porch.db.connection import get_DbAccessor +from npg_porch.models.permission import RolesEnum +from npg_porch.models.pipeline import Pipeline from npg_porch.models.token import Token router = APIRouter( diff --git a/src/npg_porch/endpoints/tasks.py b/src/npg_porch/endpoints/tasks.py index 90072df..e601517 100644 --- a/src/npg_porch/endpoints/tasks.py +++ b/src/npg_porch/endpoints/tasks.py @@ -22,14 +22,15 @@ from typing import Annotated from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi.responses import JSONResponse +from sqlalchemy.orm.exc import NoResultFound +from starlette import status + from npg_porch.auth.token import validate +from npg_porch.db.connection import get_DbAccessor from npg_porch.models.permission import PermissionValidationException from npg_porch.models.pipeline import Pipeline from npg_porch.models.task import Task, TaskStateEnum -from npg_porch.db.connection import get_DbAccessor -from sqlalchemy.exc import IntegrityError -from sqlalchemy.orm.exc import NoResultFound -from starlette import status def _validate_request(permission, pipeline): @@ -82,14 +83,20 @@ async def get_tasks( status_code=status.HTTP_201_CREATED, responses={ status.HTTP_201_CREATED: {"description": "Task creation was successful"}, - status.HTTP_409_CONFLICT: {"description": "A task with the same signature already exists"} + status.HTTP_200_OK: {"description": "A task with the same signature already exists"}, + status.HTTP_404_NOT_FOUND: {"description": "Pipeline does not exist."}, }, summary="Creates one task record.", description=''' Given a Task object, creates a database record for it and returns - the same object, the response HTTP status is 201 'Created'. The + a new Task object. The response HTTP status is 201 'Created'. The new task is assigned pending status, ie becomes available for claiming. + A request to create a task for which the database record already exists + is accepted. The return status code 200 is set in this case. The returned + Task object has its status set to the value currently available in the + database. + The pipeline specified by the `pipeline` attribute of the Task object should exist. If it does not exist, return status 404 'Not found'.''' ) @@ -102,19 +109,16 @@ async def create_task( _validate_request(permission, task.pipeline) try: - created_task = await db_accessor.create_task( + (task, created) = await db_accessor.create_task( token_id=permission.requestor_id, task=task ) - except IntegrityError: - raise HTTPException( - status_code=409, - detail='Unable to create task, as another like it already exists' - ) except NoResultFound: raise HTTPException(status_code=404, detail='Failed to find pipeline for this task') - return created_task + if created is True: + return task + return JSONResponse(status_code=status.HTTP_200_OK, content=task.model_dump()) @router.put( diff --git a/tests/data_access_test.py b/tests/data_access_test.py index d8a6cc1..bcc666f 100644 --- a/tests/data_access_test.py +++ b/tests/data_access_test.py @@ -1,12 +1,13 @@ +import re + import pytest +from npg_porch.db.data_access import AsyncDbAccessor +from npg_porch.models import Pipeline as ModelledPipeline +from npg_porch.models import Task, TaskStateEnum from pydantic import ValidationError -import re from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound -from npg_porch.db.data_access import AsyncDbAccessor -from npg_porch.models import Pipeline as ModelledPipeline, Task, TaskStateEnum - def give_me_a_pipeline(number: int = 1): return ModelledPipeline( @@ -101,11 +102,11 @@ async def test_create_task(db_accessor): task_input={'test': True} ) - saved_task = await db_accessor.create_task( + (saved_task, created) = await db_accessor.create_task( token_id=1, task=task ) - + assert created is True assert saved_task.status == TaskStateEnum.PENDING, 'State automatically set to PENDING' assert saved_task.pipeline.name == 'ptest 1' assert saved_task.task_input_id, 'Input ID is created automatically' @@ -114,10 +115,12 @@ async def test_create_task(db_accessor): assert len(events) == 1, 'An event was created with a successful task creation' assert events[0].change == 'Created', 'Message set' - with pytest.raises(IntegrityError) as exception: - await db_accessor.create_task(1, task) - - assert re.match('UNIQUE constraint failed', exception.value) + (existing_task, created) = await db_accessor.create_task(1, task) + assert created is False + assert existing_task.status == TaskStateEnum.PENDING, 'State automatically set to PENDING' + assert existing_task.pipeline.name == 'ptest 1' + events = await db_accessor.get_events_for_task(existing_task) + assert len(events) == 1, 'No additional events' @pytest.mark.asyncio async def test_claim_tasks(db_accessor): @@ -201,7 +204,7 @@ async def test_multi_claim_tasks(db_accessor): @pytest.mark.asyncio async def test_update_tasks(db_accessor): saved_pipeline = await store_me_a_pipeline(db_accessor) - saved_task = await db_accessor.create_task( + (saved_task, created) = await db_accessor.create_task( token_id=1, task=Task( task_input={'number': 1}, diff --git a/tests/task_route_test.py b/tests/task_route_test.py index 043b40d..ba62fd5 100644 --- a/tests/task_route_test.py +++ b/tests/task_route_test.py @@ -1,7 +1,6 @@ +from npg_porch.models import Pipeline, Task, TaskStateEnum from starlette import status -from npg_porch.models import Task, TaskStateEnum, Pipeline - # Not testing get-all-tasks as this method will ultimately go headers4ptest_one = { @@ -32,16 +31,19 @@ def test_task_creation(async_minimum, fastapi_testclient): headers=headers4ptest_one ) assert response.status_code == status.HTTP_201_CREATED - assert task_one == response.json() + response_obj = response.json() + assert task_one == response_obj - # Try again and expect to fail + # Try again and expect to succeed with a different status code and the + # same task returned. response = fastapi_testclient.post( 'tasks', json=task_one.model_dump(), follow_redirects=True, headers=headers4ptest_one ) - assert response.status_code == status.HTTP_409_CONFLICT + assert response.status_code == status.HTTP_200_OK + assert response.json() == response_obj task_two = Task( pipeline = { From 4543b67f9f4e899d478cf523de1a898a0cbc8616 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 27 Jun 2024 10:21:46 +0000 Subject: [PATCH 19/25] Bump actions/setup-python from 2 to 5 Bumps [actions/setup-python](https://github.com/actions/setup-python) from 2 to 5. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](https://github.com/actions/setup-python/compare/v2...v5) --- updated-dependencies: - dependency-name: actions/setup-python dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/python-app.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index f87151b..f82987e 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -17,7 +17,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: Set up Python 3.10 - uses: actions/setup-python@v2 + uses: actions/setup-python@v5 with: python-version: "3.10" - name: Install dependencies From 04f1a0c0a8cbe61065aae37fae4081f8f5b8c887 Mon Sep 17 00:00:00 2001 From: mgcam Date: Thu, 27 Jun 2024 11:42:06 +0100 Subject: [PATCH 20/25] Updated the user guide. --- docs/user_guide.md | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/docs/user_guide.md b/docs/user_guide.md index 169eb55..e4f762c 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -158,8 +158,6 @@ $request->content($DOC); my $response = $ua->request($request); if ($response->is_success) { print "Submitted successfully\n"; -} elsif ($response->code == 409 ){ - print "Already exists, that's fine.\n"; } else { die q(It's all gone wrong!) } @@ -183,18 +181,10 @@ if ($response->is_success) { } ``` -If you get a 409 response, it is highly likely that this particular task is already registered. In this way it is possible to tell whether something has already been submitted. Note that if there are many many tasks to register some of which were submitted previously, further work is required to make the process efficient - such as to ask the npg_porch server for a list of previously registered tasks for this pipeline. - -**Example 409 failure response** - -```javascript -{ - "detail": "Unable to create task, as another like it already exists" -} -``` - Once a task has been submitted, and a 201 CREATED response has been received, the npg_porch server assigns a timestamp to the task, gives it a status of `PENDING` and assigns a unique ID to it. The response from the server contains this extra information. +A 200 OK response means that this particular task for this pipeline has already been registered. The current representation of the task is returned, the status of the task might be differ from `PENDING`. Note that if there are many tasks to register, some of which were submitted previously, further work is required to make the process efficient - such as to ask the npg_porch server for a list of previously registered tasks for this pipeline. + ### Step 4 - write a script or program that can launch the pipeline Supposing there are new tasks created every 24 hours, we then also need a client that checks every 24 hours for new work it can run on a compute farm. From 57cfb5e632890930bbca6a6582e95ac69abd4b27 Mon Sep 17 00:00:00 2001 From: mgcam Date: Thu, 27 Jun 2024 17:06:34 +0100 Subject: [PATCH 21/25] Localised the nested transaction --- src/npg_porch/db/data_access.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/npg_porch/db/data_access.py b/src/npg_porch/db/data_access.py index 8281d18..2e71804 100644 --- a/src/npg_porch/db/data_access.py +++ b/src/npg_porch/db/data_access.py @@ -123,9 +123,9 @@ async def create_task(self, token_id: int, task: Task) -> tuple[Task, bool]: task.status = TaskStateEnum.PENDING t = self.convert_task_to_db(task, db_pipeline) - nested = await session.begin_nested() created = True try: + nested = await session.begin_nested() session.add(t) event = Event( task=t, From 43b6fae0d1bd37e145bf9d9b4369057f6ca53f85 Mon Sep 17 00:00:00 2001 From: mgcam Date: Wed, 10 Jul 2024 15:07:47 +0100 Subject: [PATCH 22/25] Moved pysqlite3 to test dependencies --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 933ec91..e14255e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,7 +14,6 @@ dependencies = [ "asyncpg", "fastapi", "pydantic > 2.0.0", - "pysqlite3", "psycopg2-binary", "sqlalchemy >2", "ujson", @@ -25,6 +24,7 @@ dynamic = ["version"] [project.optional-dependencies] test = [ + "pysqlite3", "pytest", "pytest-asyncio", "requests", From f5241208f37ded34edf5c844bd9baa41ba54e7ce Mon Sep 17 00:00:00 2001 From: Keith James Date: Thu, 11 Jul 2024 14:18:13 +0100 Subject: [PATCH 23/25] Update Task model validation to prevent status being NULL Change the eq method of Task so that it doesn't raise ValidationError when the "other" object is a dict which isn't a valid task. It should simply return that they are not equal. Simplified the eq method a bit by removing redundant "else"s. Some expected test results needed changing to accommodate the new Task status. --- src/npg_porch/models/task.py | 19 +++++++++++-------- tests/fixtures/deploy_db.py | 6 ++++-- tests/task_route_test.py | 15 ++++++++++++--- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/npg_porch/models/task.py b/src/npg_porch/models/task.py index df3f01e..7947473 100644 --- a/src/npg_porch/models/task.py +++ b/src/npg_porch/models/task.py @@ -21,7 +21,7 @@ from enum import Enum import hashlib import ujson -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, ValidationError from npg_porch.models.pipeline import Pipeline @@ -45,7 +45,7 @@ class Task(BaseModel): title='Task Input', description='A structured parameter set that uniquely identifies a piece of work, and enables an iteration of a pipeline' # noqa: E501 ) - status: TaskStateEnum | None = None + status: TaskStateEnum = TaskStateEnum.PENDING def generate_task_id(self): return hashlib.sha256(ujson.dumps(self.task_input, sort_keys=True).encode()).hexdigest() @@ -56,18 +56,21 @@ def __eq__(self, other): The pipeline and task_input_ids can partially differ and it still be a valid comparison. Clients do not get to create task_input_ids and may - not fully specify a pipeline. Status is also optional + not fully specify a pipeline. Automatically attempts to cast a dict into a Task, and therefore ignores any properties not valid for a Task ''' - if not isinstance(other, Task): - if isinstance(other, dict): + if isinstance(other, dict): + try: other = Task.model_validate(other) - else: + except ValidationError: return False + if not isinstance(other, Task): + return False + truths = [] for k, v in self.model_dump().items(): other_d = other.model_dump() @@ -81,5 +84,5 @@ def __eq__(self, other): truths.append(v == other_d[k]) if all(truths): return True - else: - return False + + return False diff --git a/tests/fixtures/deploy_db.py b/tests/fixtures/deploy_db.py index ffecb99..654c467 100644 --- a/tests/fixtures/deploy_db.py +++ b/tests/fixtures/deploy_db.py @@ -46,7 +46,8 @@ def minimum_data(): definition={ 'to_do': 'stuff', 'why': 'reasons' - } + }, + state=TaskStateEnum.PENDING ), Task( pipeline=pipeline, @@ -56,7 +57,8 @@ def minimum_data(): definition={ 'to_do': 'more stuff', 'why': 'reasons' - } + }, + state=TaskStateEnum.PENDING ) ] diff --git a/tests/task_route_test.py b/tests/task_route_test.py index ba62fd5..90ebaa4 100644 --- a/tests/task_route_test.py +++ b/tests/task_route_test.py @@ -67,9 +67,9 @@ def test_task_creation(async_minimum, fastapi_testclient): def test_task_update(async_minimum, fastapi_testclient): task = fastapi_testclient.get('/tasks', headers=headers4ptest_one).json()[0] - assert task['status'] is None + assert task['status'] == TaskStateEnum.PENDING.value - task['status'] = TaskStateEnum.PENDING + task['status'] = TaskStateEnum.RUNNING response = fastapi_testclient.put( '/tasks', json=task, @@ -206,7 +206,16 @@ def test_get_tasks(async_minimum, async_tasks, fastapi_testclient): ) assert response.status_code == status.HTTP_200_OK, 'Other optional argument works' tasks = response.json() - assert len(tasks) == 10, 'Ten pending tasks selected' + # async_minimum provides 2 tasks, async_tasks provides 10 + assert len(tasks) == 12, 'Twelve pending tasks selected' + + response = fastapi_testclient.get( + '/tasks?status=RUNNING', + headers=headers4ptest_one + ) + assert response.status_code == status.HTTP_200_OK, 'Other optional argument works' + tasks = response.json() + assert len(tasks) == 0, 'No running tasks selected' response = fastapi_testclient.get( '/tasks?pipeline_name="ptest one"&status=PENDING', From c14737e2aecc1232b956d96600eaa90a2ba3f00d Mon Sep 17 00:00:00 2001 From: Keith James Date: Thu, 11 Jul 2024 16:07:40 +0100 Subject: [PATCH 24/25] Remove default value for status from Task Avoid setting a default value for status to ensure that it must be set explicitly. The create_task method in the database layer now has exclusive control over whether a default (of PENDING) is set, which it does only when a new task record is created, not when an exisiting task record is updated. --- src/npg_porch/models/task.py | 2 +- tests/data_access_test.py | 24 ++++++++++++++++-------- tests/task_route_test.py | 14 ++++++++------ 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/npg_porch/models/task.py b/src/npg_porch/models/task.py index 7947473..64c4147 100644 --- a/src/npg_porch/models/task.py +++ b/src/npg_porch/models/task.py @@ -45,7 +45,7 @@ class Task(BaseModel): title='Task Input', description='A structured parameter set that uniquely identifies a piece of work, and enables an iteration of a pipeline' # noqa: E501 ) - status: TaskStateEnum = TaskStateEnum.PENDING + status: TaskStateEnum def generate_task_id(self): return hashlib.sha256(ujson.dumps(self.task_input, sort_keys=True).encode()).hexdigest() diff --git a/tests/data_access_test.py b/tests/data_access_test.py index bcc666f..b2c2e5f 100644 --- a/tests/data_access_test.py +++ b/tests/data_access_test.py @@ -99,7 +99,8 @@ async def test_create_task(db_accessor): task = Task( pipeline=saved_pipeline, - task_input={'test': True} + task_input={'test': True}, + status=TaskStateEnum.PENDING ) (saved_task, created) = await db_accessor.create_task( @@ -144,7 +145,8 @@ async def test_claim_tasks(db_accessor): token_id=1, task=Task( task_input={'number': i + 1}, - pipeline=pipeline + pipeline=pipeline, + status=TaskStateEnum.PENDING ) ) @@ -179,14 +181,16 @@ async def test_multi_claim_tasks(db_accessor): token_id=1, task=Task( task_input={'number': i + 1}, - pipeline=pipeline + pipeline=pipeline, + status=TaskStateEnum.PENDING ) ) await db_accessor.create_task( token_id=2, task=Task( task_input={'number': i + 1}, - pipeline=other_pipeline + pipeline=other_pipeline, + status=TaskStateEnum.PENDING ) ) @@ -208,7 +212,8 @@ async def test_update_tasks(db_accessor): token_id=1, task=Task( task_input={'number': 1}, - pipeline=saved_pipeline + pipeline=saved_pipeline, + status=TaskStateEnum.PENDING ) ) @@ -219,11 +224,13 @@ async def test_update_tasks(db_accessor): events = await db_accessor.get_events_for_task(modified_task) assert len(events) == 2, 'Task was created, and then updated' - events[1].change == 'Task changed, new status DONE' + assert events[1].change == 'Task changed, new status DONE' # Try to change a task that doesn't exist with pytest.raises(NoResultFound): - await db_accessor.update_task(1, Task(task_input={'number': None}, pipeline=saved_pipeline)) + await db_accessor.update_task(1, Task(task_input={'number': None}, + pipeline=saved_pipeline, + status=TaskStateEnum.PENDING)) # Try modifying something we're not allowed to saved_task.task_input_id = None @@ -254,7 +261,8 @@ async def test_get_tasks(db_accessor): token_id=1, task=Task( task_input={'number': i + 1}, - pipeline=pipeline + pipeline=pipeline, + status=TaskStateEnum.PENDING ) ) diff --git a/tests/task_route_test.py b/tests/task_route_test.py index 90ebaa4..8c5af1b 100644 --- a/tests/task_route_test.py +++ b/tests/task_route_test.py @@ -16,12 +16,13 @@ def test_task_creation(async_minimum, fastapi_testclient): # Create a task with a sparse pipeline definition task_one = Task( - pipeline = { + pipeline={ 'name': 'ptest one' }, - task_input = { + task_input={ 'number': 1 - } + }, + status=TaskStateEnum.PENDING, ) response = fastapi_testclient.post( @@ -46,12 +47,13 @@ def test_task_creation(async_minimum, fastapi_testclient): assert response.json() == response_obj task_two = Task( - pipeline = { + pipeline={ 'name': 'ptest none' }, - task_input = { + task_input={ 'number': 1 - } + }, + status=TaskStateEnum.PENDING, ) # The token is valid, but for a different pipeline. It is impossible # to have a valid token for a pipeline that does not exist. From 26cfa79f3abb9fbfa08065d3b7884ab5279eda29 Mon Sep 17 00:00:00 2001 From: mgcam Date: Tue, 30 Jul 2024 13:45:14 +0100 Subject: [PATCH 25/25] Logged changes for release 1.1.0 --- CHANGELOG.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bfaa4d1..6991894 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,4 +5,34 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +## [2.0.0] - 2024-07-31 + +### Added + +* A note about installing on macOS +* An endpoint for creating pipeline tokens +* A development Docker file +* A Token model + +### Changed + +* Moved the project to the standard Python project layout. +* Reorganised and renamed the modules: + 1. `npg.porch` namespace is collapsed into `npg_porch`, + 2. `npg.porchdb` is reorganised into `npg_porch.db`, + 3. the entry point of the application is relocated and + renamed `npg/porch_server.py` -> `npg_porch/server.py` +* Moved the test configuration file to the `tests` directory. +* Changed the HTTP code for the `pipeline` endpoint from 409 to 422 in case of + bad input. +* A Token model is returned rather than a bare string by the endpoint which +creates tokens for a pipeline. +* A request to create an already existing task is no longer invalid, does not +raise an error. +* Updated the Task model so that it is impossible to create or update the task + without explicitly specifying the new task status. +* pysqlite3 is moved from the production to test dependencies. + ## [1.0.0] - 2023-10-16 + +Initial release