From 2e64c1f5289b30b07aa462bb3d3f667cb1659bef Mon Sep 17 00:00:00 2001 From: Graham Parsons Date: Wed, 17 Dec 2025 12:29:26 +0000 Subject: [PATCH 1/7] Add option to override DRS and FHIR base URLs --- cgpclient/client.py | 7 +++++++ cgpclient/drs.py | 10 +++++++++- cgpclient/drsupload.py | 2 +- cgpclient/fhir.py | 19 ++++++++++++++++--- cgpclient/scripts/list_files | 18 ++++++++++++++++++ cgpclient/scripts/upload_dragen_run | 18 ++++++++++++++++++ 6 files changed, 69 insertions(+), 5 deletions(-) diff --git a/cgpclient/client.py b/cgpclient/client.py index edfab0d..36b5c11 100644 --- a/cgpclient/client.py +++ b/cgpclient/client.py @@ -242,6 +242,7 @@ def __init__( client.headers, client.dry_run, client.override_api_base_url, + client.drs_base_url, ) self._files = [ CGPFile(document_reference=doc_ref, drs_client=drs_client, client=client) @@ -565,6 +566,8 @@ def __init__( dry_run: bool = False, output_dir: Path | None = None, fhir_config: FHIRConfig | None = None, + fhir_base_url: str | None = None, + drs_base_url: str | None = None, ): self.api_host = api_host self.api_name = api_name @@ -572,6 +575,8 @@ def __init__( self.dry_run = dry_run self.output_dir = output_dir self.fhir_config = FHIRConfig() if fhir_config is None else fhir_config + self.fhir_base_url = fhir_base_url + self.drs_base_url = drs_base_url # Use provided auth provider or create one from legacy parameters self.auth_provider = auth_provider or create_auth_provider( @@ -593,6 +598,8 @@ def __init__( config=self.fhir_config, dry_run=self.dry_run, output_dir=self.output_dir, + base_url_override=self.fhir_base_url, + drs_base_url_override=self.drs_base_url, ) # API diff --git a/cgpclient/drs.py b/cgpclient/drs.py index 273c498..dd796c0 100644 --- a/cgpclient/drs.py +++ b/cgpclient/drs.py @@ -218,14 +218,18 @@ def __init__( headers: dict, dry_run: bool = False, override_api_base_url: bool = False, + base_url_override: str | None = None, ): self.api_base_url = api_base_url self.headers = headers self.dry_run = dry_run self.override_api_base_url = override_api_base_url + self.base_url_override = base_url_override @property def base_url(self) -> str: + if self.base_url_override is not None and self.base_url_override.strip(): + return self.base_url_override.rstrip("/") return drs_base_url(self.api_base_url) def get_drs_object( @@ -297,8 +301,12 @@ def _resolve_drs_url_to_https(self, drs_url: str) -> str: if drs_url.startswith("https:"): if self.override_api_base_url: + # Preserve the path *after* /ga4gh/ but force the configured base. + # This supports deployments where the API is mounted under a prefix + # and/or where the DRS base URL is explicitly configured. _, path = drs_url.split("/ga4gh/") - drs_url = f"https://{self.api_base_url}/ga4gh/{path}" + base_prefix = self.base_url.split("/ga4gh/")[0] + drs_url = f"{base_prefix}/ga4gh/{path}" return drs_url raise CGPClientException(f"Invalid DRS URL format {drs_url}") diff --git a/cgpclient/drsupload.py b/cgpclient/drsupload.py index f1e3be3..38e17e5 100644 --- a/cgpclient/drsupload.py +++ b/cgpclient/drsupload.py @@ -244,7 +244,7 @@ def _request_upload(self, upload_request: DrsUploadRequest) -> DrsUploadResponse log.debug(upload_request.model_dump_json(exclude_defaults=True)) response = requests.post( - url=f"https://{self.drs_client.api_base_url}/upload-request", + url=f"https://{self.drs_client.base_url}/upload-request", headers=self.drs_client.headers, timeout=REQUEST_TIMEOUT_SECS, json=upload_request.model_dump(), diff --git a/cgpclient/fhir.py b/cgpclient/fhir.py index 3f67942..4a38356 100644 --- a/cgpclient/fhir.py +++ b/cgpclient/fhir.py @@ -152,15 +152,22 @@ def __init__( config: FHIRConfig, dry_run: bool, output_dir: Path | None = None, + base_url_override: str | None = None, + drs_base_url_override: str | None = None, + ): self.api_base_url = api_base_url self.headers = headers self.config = config self.dry_run = dry_run self.output_dir = output_dir + self.base_url_override = base_url_override + self.drs_base_url_override = drs_base_url_override @property def base_url(self) -> str: + if self.base_url_override is not None and self.base_url_override.strip(): + return self.base_url_override.rstrip("/") return fhir_base_url(self.api_base_url) def get_resource( @@ -460,7 +467,13 @@ def create_drs_document_references( ) -> list[DocumentReference]: """Upload the files using the DRS upload protocol and return a DocumentReference""" - drs_client = CGPDrsClient(self.api_base_url, self.headers, self.dry_run) + drs_client = CGPDrsClient( + self.api_base_url, + self.headers, + self.dry_run, + base_url_override=self.drs_base_url_override, + + ) uploader = DrsUploader(drs_client) drs_objects: list[DrsObject] = uploader.upload_files(filenames, self.output_dir) @@ -511,7 +524,7 @@ def post_fhir_resource( params: dict[str, str] | None = None, ) -> None: """Post a FHIR resource to the FHIR server""" - url: str = f"{fhir_base_url(self.api_base_url)}/{resource.resource_type}/" + url: str = f"{self.base_url}/{resource.resource_type}/" if resource.resource_type == Bundle.__name__ and resource.type in ( BundleType.BATCH, @@ -519,7 +532,7 @@ def post_fhir_resource( ): # these bundle types are posted to the root of the FHIR server log.info("Posting bundle to the root FHIR endpoint") - url = f"{fhir_base_url(self.api_base_url)}/" + url = f"{self.base_url}/" if self.config.org_reference is not None: resource = add_provenance_for_bundle( bundle=resource, org_reference=self.config.org_reference diff --git a/cgpclient/scripts/list_files b/cgpclient/scripts/list_files index f2175f1..d65c658 100755 --- a/cgpclient/scripts/list_files +++ b/cgpclient/scripts/list_files @@ -172,6 +172,22 @@ def parse_args(args: list[str]) -> argparse.Namespace: action="store_true", help="Pivot file listing", ) + parser.add_argument( + "--fhir_base_url", + type=str, + help=( + "Explicit FHIR service base URL (overrides computed default). " + "Example: https://example.org/fhir/r4" + ), + ) + parser.add_argument( + "--drs_base_url", + type=str, + help=( + "Explicit DRS service base URL (overrides computed default). " + "Example: https://example.org/ga4gh/drs/v1" + ), + ) parsed: argparse.Namespace = parser.parse_args(args) @@ -209,6 +225,8 @@ def main(cmdline_args: list[str]) -> None: apim_kid=args.apim_kid, override_api_base_url=args.override_api_base_url, fhir_config=config, + fhir_base_url=args.fhir_base_url, + drs_base_url=args.drs_base_url, ) files: CGPFiles = client.get_files() diff --git a/cgpclient/scripts/upload_dragen_run b/cgpclient/scripts/upload_dragen_run index f141666..5876703 100755 --- a/cgpclient/scripts/upload_dragen_run +++ b/cgpclient/scripts/upload_dragen_run @@ -167,6 +167,22 @@ def parse_args(args: list[str]) -> argparse.Namespace: type=str, help="FHIR server workspace ID", ) + parser.add_argument( + "--fhir_base_url", + type=str, + help=( + "Explicit FHIR service base URL (overrides computed default). " + "Example: https://example.org/fhir/r4" + ), + ) + parser.add_argument( + "--drs_base_url", + type=str, + help=( + "Explicit DRS service base URL (overrides computed default). " + "Example: https://example.org/ga4gh/drs/v1" + ), + ) parsed: argparse.Namespace = parser.parse_args(args) @@ -205,6 +221,8 @@ def main(cmdline_args: list[str]) -> None: dry_run=args.dry_run, output_dir=args.output_dir, fhir_config=config, + fhir_base_url=args.fhir_base_url, + drs_base_url=args.drs_base_url, ) client.upload_dragen_run( From 74a2cb2e3d0134fa3e301780814e2f212e5d1a25 Mon Sep 17 00:00:00 2001 From: Graham Parsons Date: Wed, 24 Dec 2025 15:12:22 +0000 Subject: [PATCH 2/7] BREAKING: Create DRS objects via register-objects The DRS api has changed since the PoC. Now we register-objects by providing candidates, rather than POSTing directly using fully-formed DRS objects. This changes the flow because we need the DRS server response to obtain a full DRS object. --- cgpclient/drs.py | 100 +++++++++++++++++++++++++++++++--------- cgpclient/drsupload.py | 48 +++++++++++++++++-- tests/test_drsupload.py | 25 ++++++++-- 3 files changed, 141 insertions(+), 32 deletions(-) diff --git a/cgpclient/drs.py b/cgpclient/drs.py index dd796c0..dfbb36b 100644 --- a/cgpclient/drs.py +++ b/cgpclient/drs.py @@ -4,6 +4,8 @@ import sys from pathlib import Path from typing import List +from uuid import uuid4 +from datetime import datetime, timezone try: from enum import StrEnum # type: ignore @@ -204,6 +206,15 @@ def _stream_data_from_https_url( log.info("File hash successfully verified") +class DrsCandidateObject(BaseModel): + name: str | None = None + size: int + mime_type: str | None = None + checksums: list[Checksum] = Field(min_length=1) + access_methods: list[AccessMethod] = Field(min_length=1) + description: str | None = None + + class Error(BaseModel): msg: str status_code: int @@ -256,38 +267,51 @@ def get_drs_object( log.debug(drs_object) return drs_object - def post_drs_object( - self, drs_object: DrsObject, output_dir: Path | None = None - ) -> None: + def post_drs_candidate_object( + self, candidate_object: DrsCandidateObject, output_dir: Path | None = None + ) -> DrsObject: """Post a DRS object to the DRS server""" - endpoint = f"{self.base_url}/objects" - log.info("Posting DRS object: %s", drs_object.id) - log.debug(drs_object.model_dump_json(exclude_defaults=True)) + endpoint = f"{self.base_url}/register-objects" + log.info("Posting DRS object: %s", candidate_object.name) + log.debug(candidate_object.model_dump_json(exclude_defaults=True)) - if output_dir is not None: - output_file = output_dir / Path("drs_objects.json") - log.info("Writing DRS object to %s", output_file) - with open(output_file, "a", encoding="utf-8") as out: - print(drs_object.model_dump_json(), file=out) + drs_object = None if self.dry_run: log.info("Dry run, so skipping posting DRS object") - return + drs_object = fake_drs_object_from(candidate_object=candidate_object) - response = requests.post( - url=endpoint, - headers=self.headers, - timeout=REQUEST_TIMEOUT_SECS, - json=drs_object.model_dump(), - ) - if response.ok: - log.info("Successfully posted DRS objects") else: - raise CGPClientException( - f"Error posting DRS object, status code: " - f"{response.status_code} response: {response.text}" + drs_request_body = { + "candidates": [ + candidate_object.model_dump(exclude_defaults=True, exclude_none=True) + ] + } + response = requests.post( + url=endpoint, + headers=self.headers, + timeout=REQUEST_TIMEOUT_SECS, + json=drs_request_body, ) + if response.ok: + drs_object = DrsObject.model_validate(response.json()["objects"][0]) + log.info("Successfully posted DRS objects") + + else: + raise CGPClientException( + f"Error posting DRS object, status code: " + f"{response.status_code} response: {response.text}" + ) + + if output_dir is not None: + output_file = output_dir / Path("drs_objects.json") + log.info("Writing DRS object to %s", output_file) + with open(output_file, "a", encoding="utf-8") as out: + print(drs_object.model_dump_json(), file=out) + + return drs_object + def _https_url_from_id(self, object_id: str) -> str: """Construct an HTTPS DRS URL from a DRS object ID""" if "/" in object_id: @@ -372,3 +396,33 @@ def map_https_to_drs_url(https_url: str) -> str: except ValueError as e: log.error("Error parsing HTTPS DRS URL: %s", https_url) raise CGPClientException(f"Unable to parse HTTPS DRS URL: {https_url}") from e + + +def fake_drs_object_from(candidate_object: "DrsCandidateObject") -> "DrsObject": + """ + Create a synthetic DrsObject from a DrsCandidateObject (used for dry-run flows). + + This keeps the user-provided metadata (name/size/mime_type/checksums/access_methods/description) + and fills required server-side fields with reasonable placeholders. + """ + object_id = f"dryrun-{uuid4().hex}" + now_iso = datetime.now(timezone.utc).isoformat() + + # Self URI is required by the DRS object model; use a clearly fake but valid-looking URI. + self_uri = f"drs://dry-run/{object_id}" + + return DrsObject( + id=object_id, + name=candidate_object.name, + self_uri=self_uri, + size=candidate_object.size, + created_time=now_iso, + updated_time=now_iso, + version=None, + mime_type=candidate_object.mime_type, + checksums=candidate_object.checksums, + access_methods=candidate_object.access_methods, + contents=[], + description=candidate_object.description, + aliases=[], + ) diff --git a/cgpclient/drsupload.py b/cgpclient/drsupload.py index 38e17e5..3ae49fe 100644 --- a/cgpclient/drsupload.py +++ b/cgpclient/drsupload.py @@ -20,7 +20,7 @@ Checksum, ChecksumType, CGPDrsClient, - DrsObject, + DrsObject, DrsCandidateObject, ) from cgpclient.htsget import htsget_base_url, mime_type_to_htsget_endpoint from cgpclient.utils import REQUEST_TIMEOUT_SECS, CGPClientException, md5sum @@ -134,6 +134,33 @@ def to_drs_object( access_methods=access_methods, ) + def to_drs_candidateobject( + self, upload_method: DrsUploadMethod, api_base_url: str + ) -> DrsCandidateObject: + access_methods: list[AccessMethod] = [] + if upload_method.type == DrsUploadMethodType.S3: + access_methods.append( + AccessMethod( + type=AccessMethodType.S3, # type: ignore + access_id="s3", + access_url=upload_method.access_url, + region=upload_method.region, + ) + ) + else: + raise CGPClientException( + f"Unsupported upload_method type: {upload_method.type}" + ) + + return DrsCandidateObject( + name=self.name, + size=self.size, + mime_type=self.mime_type, + checksums=self.checksums, + access_methods=access_methods, + description=self.description, + ) + class DrsUploadResponse(BaseModel): objects: dict[str, DrsUploadResponseObject] @@ -162,6 +189,13 @@ def upload_file(self, filename: Path, upload_method: DrsUploadMethod) -> None: return try: + # s3 = boto3.client( + # "s3", + # aws_access_key_id=upload_method.credentials["AccessKeyId"], + # aws_secret_access_key=upload_method.credentials["SecretAccessKey"], + # aws_session_token=upload_method.credentials["SessionToken"], + # region_name=upload_method.region, + # ) s3 = boto3.client( "s3", aws_access_key_id=upload_method.credentials["AccessKeyId"], @@ -204,10 +238,14 @@ def upload_files( drs_objects = [] for filename in filenames: + upload_response_object = next( + obj for obj in upload_response_objects.values() + if obj.name == filename.name + ) drs_objects.append( self._upload_file_with_response_object( filename=filename, - upload_response_object=upload_response_objects[str(filename.name)], + upload_response_object=upload_response_object, output_dir=output_dir, ) ) @@ -244,7 +282,7 @@ def _request_upload(self, upload_request: DrsUploadRequest) -> DrsUploadResponse log.debug(upload_request.model_dump_json(exclude_defaults=True)) response = requests.post( - url=f"https://{self.drs_client.base_url}/upload-request", + url=f"{self.drs_client.base_url}/upload-request", headers=self.drs_client.headers, timeout=REQUEST_TIMEOUT_SECS, json=upload_request.model_dump(), @@ -272,11 +310,11 @@ def _upload_file_with_response_object( self.s3_client.upload_file(filename=filename, upload_method=s3_upload_method) - drs_object = upload_response_object.to_drs_object( + drs_candidate_object = upload_response_object.to_drs_candidateobject( upload_method=s3_upload_method, api_base_url=self.drs_client.api_base_url ) - self.drs_client.post_drs_object(drs_object, output_dir) + drs_object = self.drs_client.post_drs_candidate_object(drs_candidate_object, output_dir) return drs_object def _guess_mime_type(self, filename: Path) -> str: diff --git a/tests/test_drsupload.py b/tests/test_drsupload.py index cb25c78..4e58fc2 100644 --- a/tests/test_drsupload.py +++ b/tests/test_drsupload.py @@ -10,6 +10,10 @@ from cgpclient.drs import CGPDrsClient, DrsObject from cgpclient.drsupload import ( AccessURL, + AccessMethodType, + AccessMethod, + Checksum, + ChecksumType, DrsUploader, DrsUploadMethod, DrsUploadMethodType, @@ -148,9 +152,9 @@ def upload_file(self, upload, Bucket, Key): @patch("cgpclient.drsupload.DrsUploader._request_upload") @patch("cgpclient.drsupload.S3Client.upload_file") -@patch("cgpclient.drs.CGPDrsClient.post_drs_object") +@patch("cgpclient.drs.CGPDrsClient.post_drs_candidate_object") def test_drs_upload_file( - mock_post_object: MagicMock, + mock_post_candidate_object: MagicMock, mock_s3_upload: MagicMock, mock_request_upload: MagicMock, tmp_path, @@ -177,7 +181,20 @@ def test_drs_upload_file( make_upload_response(upload_request) ) mock_s3_upload.return_value = "foo" - mock_post_object.return_value = None + mock_post_candidate_object.return_value = DrsObject( + id="foo-object-id", + self_uri="drs://dry-run/foo-object-id", + name=file_name, + size=len(file_data), + checksums=[Checksum(type=ChecksumType.MD5, checksum="md5placeholder")], + access_methods=[ + AccessMethod( + type=AccessMethodType.S3, # type: ignore + access_id="s3", + access_url=AccessURL(url="s3://bucket/prefix/test.fastq.gz"), + ) + ], + ) drs_client = CGPDrsClient( client.api_base_url, @@ -194,7 +211,7 @@ def test_drs_upload_file( mock_request_upload.assert_called_once() mock_s3_upload.assert_called_once() - mock_post_object.assert_called_once() + mock_post_candidate_object.assert_called_once() assert drs_object.name == file_name assert drs_object.size == len(file_data) From d1d3ac7105e60e03d8650728e13b847e4b7b4906 Mon Sep 17 00:00:00 2001 From: Graham Parsons Date: Wed, 24 Dec 2025 16:23:40 +0000 Subject: [PATCH 3/7] Allow for variations in S3 credential keys --- cgpclient/drsupload.py | 94 ++++++++++++++++++++++++++++++++++++----- tests/test_drsupload.py | 25 +++++++++++ 2 files changed, 109 insertions(+), 10 deletions(-) diff --git a/cgpclient/drsupload.py b/cgpclient/drsupload.py index 3ae49fe..d56a1ac 100644 --- a/cgpclient/drsupload.py +++ b/cgpclient/drsupload.py @@ -3,6 +3,7 @@ import logging import mimetypes from pathlib import Path +from typing import Mapping, Any try: from enum import StrEnum # type: ignore @@ -44,6 +45,84 @@ class DrsUploadMethodType(StrEnum): # type: ignore HTTPS = "https" +def _normalise_credential_key(key: str) -> str: + """ + Normalise credential keys to be comparison-friendly: + - lower-case + - remove non-alphanumeric characters (underscores, hyphens, spaces, etc.) + """ + return "".join(ch for ch in key.lower() if ch.isalnum()) + + +def coerce_aws_credentials(credentials: Mapping[str, Any]) -> dict[str, str]: + """ + Coerce AWS credentials keys from various common spellings/cases into canonical AWS keys: + - AccessKeyId + - SecretAccessKey + - SessionToken (optional) + + Accepts variants like: + access_key_id, aws_access_key_id, AWSAccessKeyId, AccessKeyID, session-token, etc. + """ + if not isinstance(credentials, Mapping): + raise CGPClientException("Invalid AWS credentials format (expected an object/map)") + + # Build a normalised lookup from the provided keys + normalised_to_value: dict[str, Any] = { + _normalise_credential_key(str(k)): v for k, v in credentials.items() + } + + aliases: dict[str, list[str]] = { + "AccessKeyId": [ + "accesskeyid", + "awsaccesskeyid", + "aws_access_key_id", + "access_key_id", + ], + "SecretAccessKey": [ + "secretaccesskey", + "awssecretaccesskey", + "aws_secret_access_key", + "secret_access_key", + ], + "SessionToken": [ + "sessiontoken", + "awssessiontoken", + "aws_session_token", + "session_token", + ], + } + + coerced: dict[str, str] = {} + missing_required: list[str] = [] + + for canonical_key, key_aliases in aliases.items(): + found = None + for alias in key_aliases: + normalised_alias = _normalise_credential_key(alias) + if normalised_alias in normalised_to_value: + found = normalised_to_value[normalised_alias] + break + + if found is None: + if canonical_key in ("AccessKeyId", "SecretAccessKey"): + missing_required.append(canonical_key) + continue + + if not isinstance(found, str): + raise CGPClientException( + f"Invalid AWS credential value type for {canonical_key} (expected string)" + ) + coerced[canonical_key] = found + + if missing_required: + raise CGPClientException( + f"Missing necessary AWS credentials: {', '.join(missing_required)}" + ) + + return coerced + + class DrsUploadMethod(BaseModel): type: DrsUploadMethodType access_url: AccessURL @@ -189,18 +268,13 @@ def upload_file(self, filename: Path, upload_method: DrsUploadMethod) -> None: return try: - # s3 = boto3.client( - # "s3", - # aws_access_key_id=upload_method.credentials["AccessKeyId"], - # aws_secret_access_key=upload_method.credentials["SecretAccessKey"], - # aws_session_token=upload_method.credentials["SessionToken"], - # region_name=upload_method.region, - # ) + creds = coerce_aws_credentials(upload_method.credentials) + s3 = boto3.client( "s3", - aws_access_key_id=upload_method.credentials["AccessKeyId"], - aws_secret_access_key=upload_method.credentials["SecretAccessKey"], - aws_session_token=upload_method.credentials["SessionToken"], + aws_access_key_id=creds["AccessKeyId"], + aws_secret_access_key=creds["SecretAccessKey"], + aws_session_token=creds.get("SessionToken"), region_name=upload_method.region, ) except KeyError as e: diff --git a/tests/test_drsupload.py b/tests/test_drsupload.py index 4e58fc2..fdf9155 100644 --- a/tests/test_drsupload.py +++ b/tests/test_drsupload.py @@ -20,6 +20,7 @@ DrsUploadRequest, DrsUploadResponse, S3Client, + coerce_aws_credentials, ) from cgpclient.utils import CGPClientException, create_uuid @@ -217,3 +218,27 @@ def test_drs_upload_file( assert drs_object.size == len(file_data) assert len(drs_object.access_methods) == 1 assert drs_object.access_methods[0].access_id == "s3" + + +def test_coerce_aws_credentials_accepts_correct_and_snake_case_keys() -> None: + # Already-correct AWS-style keys should remain usable and canonical. + creds_camel = { + "AccessKeyId": "key", + "SecretAccessKey": "secret", + "SessionToken": "token", + } + coerced = coerce_aws_credentials(creds_camel) + assert coerced == creds_camel + + # Common snake_case prefixed keys should be coerced to canonical keys. + creds_snake = { + "access_key_id": "key2", + "secret_access_key": "secret2", + "session_token": "token2", + } + coerced2 = coerce_aws_credentials(creds_snake) + assert coerced2 == { + "AccessKeyId": "key2", + "SecretAccessKey": "secret2", + "SessionToken": "token2", + } From eab97e34255a83dd0b26cafe1e67fc98a63e90d3 Mon Sep 17 00:00:00 2001 From: Graham Parsons Date: Mon, 29 Dec 2025 17:46:42 +0000 Subject: [PATCH 4/7] Update version to indicate a breaking change. Note, the version is kept below 1.0 so as not to signal stability. --- README.md | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 7f7f54e..6608741 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ This repository contains a python client library for interacting with CGP APIs h The repository also includes some scripts that perform (hopefully) useful functions using these modules, and which serve as examples on how to use the client library. -__NB__ This package, and the APIs it communicates with, are under active development and are currently at the [Alpha](https://www.gov.uk/service-manual/agile-delivery/how-the-alpha-phase-works) phase of delivery. At this stage the APIs may still be subject to breaking changes, and we do not offer any SLAs etc. This code is intended to support NHS GMS partners to start to experiment with these new services and to provide feedback - please create an Issue in github, and pull requests are welcome! +__NB__ This package, and the APIs it communicates with, are under active development and are currently at the [Alpha](https://www.gov.uk/service-manual/agile-delivery/how-the-alpha-phase-works) phase of delivery. At this stage the APIs may still be subject to breaking changes, and we do not offer any SLAs etc. This code is intended to support NHS GMS partners to start to experiment with these new services and to provide feedback - please create an Issue in github, and pull requests are welcome! As of version `0.2.0` the client targets a revised version of the DRS upload spec. We anticipate moving to a private Beta phase in the second quarter of 2025, during which we will start to experiment with realistic data rather than the test data used so far. diff --git a/pyproject.toml b/pyproject.toml index d962c3e..ef9cd1e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "cgpclient" -version = "0.1.0" +version = "0.2.0" description = "Python Client for the Clinical Genomics Platform" authors = [ {name = "Graham Ritchie",email = "gritchie@gmail.com"} From 9271b1fea83a14c8a657a87d40bc70ea71781aa5 Mon Sep 17 00:00:00 2001 From: Graham Parsons Date: Wed, 31 Dec 2025 14:30:13 +0000 Subject: [PATCH 5/7] BREAKING: Use only major version (v1 not v1.4) for DRS path --- cgpclient/drs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cgpclient/drs.py b/cgpclient/drs.py index dfbb36b..505ae09 100644 --- a/cgpclient/drs.py +++ b/cgpclient/drs.py @@ -362,7 +362,7 @@ def _get_drs_object_from_https_url(self, https_url: str) -> DrsObject: def drs_base_url(api_base_url: str) -> str: """Return the base HTTPS URL for the DRS server""" - return f"https://{api_base_url}/ga4gh/drs/v1.4" + return f"https://{api_base_url}/ga4gh/drs/v1" def map_drs_to_https_url(drs_url: str) -> str: From 52d7dc7528dde131160e82407d898e08a84e5824 Mon Sep 17 00:00:00 2001 From: Graham Parsons Date: Wed, 31 Dec 2025 14:35:21 +0000 Subject: [PATCH 6/7] Ensure map_drs_to_https_url handles DRS URLs without api-name in the path --- cgpclient/drs.py | 8 ++++++-- tests/test_drs.py | 43 ++++++++++++++++++++++++++++++++----------- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/cgpclient/drs.py b/cgpclient/drs.py index 505ae09..34783d8 100644 --- a/cgpclient/drs.py +++ b/cgpclient/drs.py @@ -372,8 +372,12 @@ def map_drs_to_https_url(drs_url: str) -> str: try: # e.g. drs://api.service.nhs.uk/genomic-data-access/1234 # maps to: https://api.service.nhs.uk/genomic-data-access/ga4gh/drs/v1.4/objects/1234 # noqa: E501 - (_, _, base_url, api_name, object_id) = drs_url.split("/") - api_base_url: str = f"{base_url}/{api_name}" + parts = drs_url.split("/") + if len(parts) not in (4,5): + raise ValueError() + object_id = parts[-1] + api_base_url = "/".join(parts[2:-1]) + https_url: str = f"{drs_base_url(api_base_url)}/objects/{object_id}" log.debug("Mapped DRS URL: %s to HTTPS URL: %s", drs_url, https_url) return https_url diff --git a/tests/test_drs.py b/tests/test_drs.py index bbcc191..fbe68c0 100644 --- a/tests/test_drs.py +++ b/tests/test_drs.py @@ -112,19 +112,40 @@ def test_get_object(mock_get_object: MagicMock, drs_object: dict, client: CGPCli mock_get_object.assert_called() -def test_map_drs_to_https_url() -> None: - object_id: str = "1234" - drs_url: str = f"drs://api.service.nhs.uk/genomic-data-access/{object_id}" - https_url: str = f"https://api.service.nhs.uk/genomic-data-access/ga4gh/drs/v1.4/objects/{object_id}" - assert map_drs_to_https_url(drs_url) == https_url - +@pytest.mark.parametrize( + ("drs_url", "expected_https_url"), + [ + ( + "drs://api.service.nhs.uk/genomic-data-access/1234", + "https://api.service.nhs.uk/genomic-data-access/ga4gh/drs/v1/objects/1234", + ), + ( + "drs://api.service.nhs.uk/1234", + "https://api.service.nhs.uk/ga4gh/drs/v1/objects/1234", + ), + ], +) +def test_map_drs_to_https_url(drs_url: str, expected_https_url: str) -> None: + assert map_drs_to_https_url(drs_url) == expected_https_url + + +@pytest.mark.parametrize( + "drs_url", + [ + pytest.param( + "drs://api.service.nhs.uk/unexpected/genomic-data-access/1234", + id="too many path segments", + ), + pytest.param( + "drs://1234", + id="missing host", + ), + ], +) +def test_map_drs_to_https_url_raises_when_urls_are_bad(drs_url: str) -> None: with pytest.raises(CGPClientException): - map_drs_to_https_url( - f"drs://api.service.nhs.uk/unexpected/genomic-data-access/{object_id}" - ) + map_drs_to_https_url(drs_url) - with pytest.raises(CGPClientException): - map_drs_to_https_url(f"drs://api.service.nhs.uk/{object_id}") with pytest.raises(CGPClientException): map_drs_to_https_url( From 56b6df0b00f0c7f760101560bf1eebe2c10e4026 Mon Sep 17 00:00:00 2001 From: Graham Parsons Date: Wed, 31 Dec 2025 14:44:56 +0000 Subject: [PATCH 7/7] Ensure map_https_to_drs_url handles HTTPS URLs without api-name in the path --- cgpclient/drs.py | 40 +++++++++++++++++++++++++++++++++++----- tests/test_drs.py | 24 ++++++++++++++++++------ 2 files changed, 53 insertions(+), 11 deletions(-) diff --git a/cgpclient/drs.py b/cgpclient/drs.py index 34783d8..1c95a5f 100644 --- a/cgpclient/drs.py +++ b/cgpclient/drs.py @@ -387,14 +387,44 @@ def map_drs_to_https_url(drs_url: str) -> str: def map_https_to_drs_url(https_url: str) -> str: - """Map an HTTPS URL to a DRS URL""" + """Map an HTTPS URL to a DRS URL. + + We assume: + - the host is always the 3rd element after splitting on "/" + - the object_id is always the final path element + - anything between host and the "ga4gh" segment is the optional api_path + + Examples: + https://host/genomic-data-access/ga4gh/.../1234 -> drs://host/genomic-data-access/1234 + https://host/ga4gh/.../1234 -> drs://host/1234 + """ if not https_url.startswith("https://"): raise CGPClientException(f"Invalid HTTPS URL: {https_url}") + try: - # e.g. https://api.service.nhs.uk/genomic-data-access/ga4gh/drs/v1.4/objects/1234 # noqa: E501 - # maps to: drs://api.service.nhs.uk/genomic-data-access/1234 - (_, _, base_url, api_name, _, _, _, _, object_id) = https_url.split("/") - drs_url: str = f"drs://{base_url}/{api_name}/{object_id}" + parts = https_url.split("/") + + if len(parts) < 4: + raise ValueError() + + host = parts[2] + object_id = parts[-1] + + if not host or not object_id: + raise ValueError() + + if "ga4gh" not in parts[3:]: + raise ValueError() + + ga4gh_index = parts.index("ga4gh") + api_path_parts = parts[3:ga4gh_index] + api_path = "/".join(p for p in api_path_parts if p) + + if api_path: + drs_url: str = f"drs://{host}/{api_path}/{object_id}" + else: + drs_url = f"drs://{host}/{object_id}" + log.debug("Mapped HTTPS URL: %s to DRS URL: %s", https_url, drs_url) return drs_url except ValueError as e: diff --git a/tests/test_drs.py b/tests/test_drs.py index fbe68c0..9238272 100644 --- a/tests/test_drs.py +++ b/tests/test_drs.py @@ -10,6 +10,7 @@ CGPDrsClient, DrsObject, map_drs_to_https_url, + map_https_to_drs_url, ) from cgpclient.utils import CGPClientException @@ -147,10 +148,21 @@ def test_map_drs_to_https_url_raises_when_urls_are_bad(drs_url: str) -> None: map_drs_to_https_url(drs_url) - with pytest.raises(CGPClientException): - map_drs_to_https_url( - f"drs://api.service.nhs.uk/ga4gh/drs/v1.4/objects/{object_id}" - ) - with pytest.raises(CGPClientException): - map_drs_to_https_url(f"drs://{object_id}") +@pytest.mark.parametrize( + ("https_url", "expected_drs_url"), + [ + pytest.param( + "https://api.service.nhs.uk/genomic-data-access/ga4gh/drs/v1.4/objects/1234", + "drs://api.service.nhs.uk/genomic-data-access/1234", + id="with api name in path", + ), + pytest.param( + "https://api.service.nhs.uk/ga4gh/drs/v1.4/objects/1234", + "drs://api.service.nhs.uk/1234", + id="without api name in path", + ), + ], +) +def test_map_https_to_drs_url(https_url: str, expected_drs_url: str) -> None: + assert map_https_to_drs_url(https_url) == expected_drs_url