From d493f9ec620e86bf7b18fdb4b2bd72db4efe00f5 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Thu, 17 Sep 2026 15:35:10 +0200 Subject: [PATCH 01/21] ENAVisibilityChecker for prepro --- .../check_external_visibility.py | 8 +++- kubernetes/loculus/values.yaml | 8 ++++ .../external_services.py | 42 +++++++++++++++++++ .../src/loculus_preprocessing/prepro.py | 1 + .../processing_functions.py | 16 +++++++ .../nextclade/tests/factory_methods.py | 2 + 6 files changed, 76 insertions(+), 1 deletion(-) diff --git a/ena-submission/src/ena_deposition/check_external_visibility.py b/ena-submission/src/ena_deposition/check_external_visibility.py index f058448fc5..918c28296c 100644 --- a/ena-submission/src/ena_deposition/check_external_visibility.py +++ b/ena-submission/src/ena_deposition/check_external_visibility.py @@ -68,7 +68,13 @@ def check_visibility(self, config: Config, accession: str) -> datetime | None: class ENAVisibilityChecker(VisibilityChecker): - """Checker for ENA visibility""" + """Checker for ENA visibility + + This class is adapted and re-used in + preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py + If you change anything here, check if the update needs to be applied there + as well. + """ def check_visibility(self, config: Config, accession: str) -> datetime | None: file_type = ( diff --git a/kubernetes/loculus/values.yaml b/kubernetes/loculus/values.yaml index ba2f9148b3..696ab37e3b 100644 --- a/kubernetes/loculus/values.yaml +++ b/kubernetes/loculus/values.yaml @@ -721,6 +721,10 @@ defaultOrganismConfig: &defaultOrganismConfig url: "https://www.ebi.ac.uk/ena/browser/view/__value__" - name: View in DDBJ url: "https://ddbj.nig.ac.jp/search/entry/bioproject/__value__" + preprocessing: + function: check_ena_accession + inputs: + accession: bioprojectAccession header: "INSDC" orderOnDetailsPage: 1060 ingest: bioprojects @@ -748,6 +752,10 @@ defaultOrganismConfig: &defaultOrganismConfig url: "https://www.ebi.ac.uk/ena/browser/view/__value__" - name: View in DDBJ url: "https://ddbj.nig.ac.jp/search/entry/biosample/__value__" + preprocessing: + function: check_ena_accession + inputs: + accession: biosampleAccession header: "INSDC" orderOnDetailsPage: 1080 guidance: "Only add this field if you have already submitted the raw reads to the INSDC, read more at https://pathoplexus.org/docs/how-to/upload-sequences. If you add this field we will not create a new biosample for you and assume you have submitted all required metadata in the linked biosample." diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index b6b63187c2..292888121e 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -2,6 +2,7 @@ import urllib.parse from collections import OrderedDict from dataclasses import dataclass +from http import HTTPStatus import requests from pydantic import BaseModel, Field, ValidationError @@ -17,6 +18,7 @@ ProcessingAnnotation, RawProcessingResult, _internal_error_message, + processing_error, raw_internal_error, ) @@ -316,3 +318,43 @@ def _annotation( if not internal_error: return ProcessingAnnotation([source], [source], message) return ProcessingAnnotation([source], [source], _internal_error_message(message)) + + +ena_cache = RequestCache(1024) + + +class ENAVisibilityChecker: + """Used to check ENA visibility + + Adapted from the ENAVisibilityChecker in + ena-submission/src/ena_deposition/check_external_visibility.py. + If anything changes in that class, check whether the update needs to + be applied here as well. + """ + + def __init__( + self, + timeout_seconds: int = 30, + ): + self.timeout_seconds = timeout_seconds + + def check_visibility(self, accession: str) -> RawProcessingResult: + file_type = ( + "xml" + if accession.startswith(("PRJ", "SAM", "GCA", "ERR", "ERX", "SRR", "SRX", "DRR", "DRX")) + else "embl" + ) + url = f"https://www.ebi.ac.uk/ena/browser/api/{file_type}/{accession}" + ena_error_message = ( + f"unable to validate accession '{accession}': could not reach ENA, " + "please try resubmitting later" + ) + try: + response = ena_cache.get_or_fetch(url, timeout=self.timeout_seconds) + except requests.RequestException: + return processing_error(ena_error_message) + if response.status_code == HTTPStatus.OK: + return RawProcessingResult(datum=accession) + if response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR: + return processing_error(ena_error_message) + return processing_error(f"accession '{accession}' does not exist on ENA") diff --git a/preprocessing/nextclade/src/loculus_preprocessing/prepro.py b/preprocessing/nextclade/src/loculus_preprocessing/prepro.py index 67b9542be8..5b65d43556 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/prepro.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/prepro.py @@ -6,6 +6,7 @@ from typing import Any from pydantic import ValidationError +from loculus_preprocessing.external_services import ENAVisibilityChecker from .backend import ( download_diamond_db, diff --git a/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py b/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py index 79f83c5914..78aa60fb60 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py @@ -18,6 +18,7 @@ import pytz from loculus_preprocessing.external_services import ExternalServices +from loculus_preprocessing.external_services import ENAVisibilityChecker, TaxonomyService from .datatypes import ( AnnotationSourceType, @@ -1297,6 +1298,21 @@ def common_name_from_id( # noqa: PLR0913, PLR0917 return external_services.taxonomy_service.get_common_name(tax_id) + @staticmethod + def check_ena_accession( + input_data: InputMetadata, + output_field: str, + input_fields: list[str], + args: FunctionArgs, + context: ProcessingContext, + ): + accession: str | None = input_data.get("accession") + if not accession: + return RawProcessingResult() + if context.is_insdc_ingest_group: + return RawProcessingResult(datum=accession) + return context.ena_checker.check_visibility(accession) + def single_metadata_annotation( source_name: str, diff --git a/preprocessing/nextclade/tests/factory_methods.py b/preprocessing/nextclade/tests/factory_methods.py index e05b9b5acf..0c93ba1ed2 100644 --- a/preprocessing/nextclade/tests/factory_methods.py +++ b/preprocessing/nextclade/tests/factory_methods.py @@ -24,6 +24,8 @@ ) from loculus_preprocessing.external_services import ExternalServices, TaxonomyService from loculus_preprocessing.nextclade_annotation import GffAttributes, NextcladeAnnotation +from loculus_preprocessing.external_services import ENAVisibilityChecker, TaxonomyService +from loculus_preprocessing.processing_functions import ProcessingContext # Default ProcessingContext for tests that don't care about its contents (no INSDC # ingest group, no submittedAt). Tests that do care about specific field(s) should use From 77d16b54f862c73a67ae9ff89b7c73c516ac50f8 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Thu, 17 Sep 2026 15:46:59 +0200 Subject: [PATCH 02/21] Add tests --- .../test_metadata_processing_functions.py | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index 0732959cb4..27e88c755e 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -3,6 +3,7 @@ from unittest import mock import pytest +import requests from factory_methods import ( DEFAULT_EXTERNAL_SERVICES, DEFAULT_TEST_CONTEXT, @@ -16,6 +17,7 @@ verify_processed_entry, ) +from loculus_preprocessing import external_services from loculus_preprocessing.config import Config, ProcessingSpec, get_config, get_processing_order from loculus_preprocessing.datatypes import ( AnnotationSource, @@ -1805,5 +1807,101 @@ def test_call_function_converts_raw_errors_to_annotations() -> None: ) +@dataclass +class EnaAccessionCase: + name: str + accession: str + # How ENA responds: an HTTP status code, or an exception raised by the session. + # None means ENA must not be contacted at all. + ena_response: int | Exception | None = None + is_insdc_ingest_group: bool = False + expected_datum: str | None = None + expected_error: str | None = None + expected_url: str | None = None + + +ena_accession_cases = [ + EnaAccessionCase( + name="public_bioproject_is_accepted", + accession="PRJEB12345", + ena_response=200, + expected_datum="PRJEB12345", + expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB12345", + ), + EnaAccessionCase( + name="nucleotide_accession_uses_embl_endpoint", + accession="OZ123456", + ena_response=200, + expected_datum="OZ123456", + expected_url="https://www.ebi.ac.uk/ena/browser/api/embl/OZ123456", + ), + EnaAccessionCase( + name="unknown_accession_is_rejected", + accession="PRJEB99999", + ena_response=404, + expected_error="does not exist on ENA", + expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB99999", + ), + EnaAccessionCase( + name="server_error_asks_submitter_to_retry", + # 501 is not in the session's status_forcelist, so it arrives as a response + accession="PRJEB12345", + ena_response=501, + expected_error="could not reach ENA", + expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB12345", + ), + EnaAccessionCase( + name="exhausted_retries_ask_submitter_to_retry", + # what urllib3 raises once the retries for 429/5xx in status_forcelist run out + accession="PRJEB12345", + ena_response=requests.exceptions.RetryError(), + expected_error="could not reach ENA", + expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB12345", + ), + EnaAccessionCase( + name="missing_accession_is_not_checked", + accession="", + ), + EnaAccessionCase( + name="insdc_ingested_accession_is_not_checked", + # ingest joins multiple bioprojects with commas, which ENA cannot resolve + accession="PRJNA123,PRJNA456", + is_insdc_ingest_group=True, + expected_datum="PRJNA123,PRJNA456", + ), +] + + +@pytest.mark.parametrize("case", ena_accession_cases, ids=lambda c: c.name) +def test_check_ena_accession(case: EnaAccessionCase) -> None: + external_services.ena_cache.clear() + with mock.patch.object(external_services.ena_cache, "session") as mock_session: + if isinstance(case.ena_response, Exception): + mock_session.get.side_effect = case.ena_response + else: + mock_session.get.return_value = mock.MagicMock(status_code=case.ena_response) + + result = ProcessingFunctions.check_ena_accession( + input_data={"accession": case.accession}, + output_field="bioprojectAccession", + input_fields=["bioprojectAccession"], + args={}, + context=replace(DEFAULT_TEST_CONTEXT, is_insdc_ingest_group=case.is_insdc_ingest_group), + ) + + assert result.datum == case.expected_datum + if case.expected_error is None: + assert result.errors == [] + else: + assert len(result.errors) == 1 + assert case.expected_error in result.errors[0] + + if case.expected_url is None: + assert mock_session.get.call_count == 0 + else: + mock_session.get.assert_called_once() + assert mock_session.get.call_args.args[0] == case.expected_url + + if __name__ == "__main__": pytest.main() From fae7729a20b65d4808a86b1c6917bfef332ed24b Mon Sep 17 00:00:00 2001 From: maverbiest Date: Fri, 18 Sep 2026 10:49:47 +0200 Subject: [PATCH 03/21] Fixup --- .../nextclade/src/loculus_preprocessing/processing_functions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py b/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py index 78aa60fb60..478b9c8f0b 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py @@ -1305,7 +1305,7 @@ def check_ena_accession( input_fields: list[str], args: FunctionArgs, context: ProcessingContext, - ): + ) -> RawProcessingResult: accession: str | None = input_data.get("accession") if not accession: return RawProcessingResult() From 316586a1fa81e9ac4d9b75da61c906afd727e83a Mon Sep 17 00:00:00 2001 From: maverbiest Date: Mon, 21 Sep 2026 15:24:21 +0200 Subject: [PATCH 04/21] Add cache for bioprojects --- .../external_services.py | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index 292888121e..0b7d5d463e 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -61,19 +61,24 @@ def set(self, url: str, response: requests.Response) -> None: if len(self.cache) > self.max_size: self.cache.popitem(last=False) - def get_or_fetch(self, url: str, timeout: int = 15) -> requests.Response: + def get_or_fetch( + self, url: str, timeout: int = 15, use_cache: bool = True + ) -> requests.Response: """ Check if `url` already exists in the cache and return the cached Response if it does. If `url` is not in the cache, make the actual request (with timeout and retries). Add the Response to the cache (if status code in the 200s), and return the Response. + With `use_cache=False` the request is always made, and the cache is neither read nor + written to. + The caller should wrap this in a try/except block and handle errors. """ - response = self.get(url) + response = self.get(url) if use_cache else None if response is None: response = self.session.get(url, timeout=timeout) - if 200 <= response.status_code < 300: # noqa: PLR2004 + if use_cache and 200 <= response.status_code < 300: # noqa: PLR2004 self.set(url, response) return response @@ -320,7 +325,22 @@ def _annotation( return ProcessingAnnotation([source], [source], _internal_error_message(message)) -ena_cache = RequestCache(1024) +# Rarely, a bioproject XML can reach ~10 MB (e.g., PRJNA591860) +# so keeping the cache small. Should still have high hit rate when +# all submissions for a batch have the same project +ena_cache = RequestCache(max_size=16) +PROJECT_PREFIX = "PRJ" +XML_PREFIXES = ( + PROJECT_PREFIX, + "SAM", + "GCA", + "ERR", + "ERX", + "SRR", + "SRX", + "DRR", + "DRX", +) class ENAVisibilityChecker: @@ -339,20 +359,21 @@ def __init__( self.timeout_seconds = timeout_seconds def check_visibility(self, accession: str) -> RawProcessingResult: - file_type = ( - "xml" - if accession.startswith(("PRJ", "SAM", "GCA", "ERR", "ERX", "SRR", "SRX", "DRR", "DRX")) - else "embl" - ) + file_type = "xml" if accession.startswith(XML_PREFIXES) else "embl" url = f"https://www.ebi.ac.uk/ena/browser/api/{file_type}/{accession}" ena_error_message = ( f"unable to validate accession '{accession}': could not reach ENA, " "please try resubmitting later" ) try: - response = ena_cache.get_or_fetch(url, timeout=self.timeout_seconds) + # Only cache bioprojects as they're the only thing + # likely to be shared across submissions + response = ena_cache.get_or_fetch( + url, timeout=self.timeout_seconds, use_cache=accession.startswith(PROJECT_PREFIX) + ) except requests.RequestException: return processing_error(ena_error_message) + if response.status_code == HTTPStatus.OK: return RawProcessingResult(datum=accession) if response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR: From d99e6228ce9d2639b2013d08e93f336dafed432f Mon Sep 17 00:00:00 2001 From: maverbiest Date: Mon, 21 Sep 2026 21:37:52 +0200 Subject: [PATCH 05/21] Rename --- .../src/loculus_preprocessing/external_services.py | 7 +++---- .../nextclade/tests/test_metadata_processing_functions.py | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index 0b7d5d463e..4e2af5a0c8 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -328,7 +328,7 @@ def _annotation( # Rarely, a bioproject XML can reach ~10 MB (e.g., PRJNA591860) # so keeping the cache small. Should still have high hit rate when # all submissions for a batch have the same project -ena_cache = RequestCache(max_size=16) +bioproject_cache = RequestCache(max_size=16) PROJECT_PREFIX = "PRJ" XML_PREFIXES = ( PROJECT_PREFIX, @@ -366,9 +366,8 @@ def check_visibility(self, accession: str) -> RawProcessingResult: "please try resubmitting later" ) try: - # Only cache bioprojects as they're the only thing - # likely to be shared across submissions - response = ena_cache.get_or_fetch( + # Only cache bioprojects as they're what's likely to be shared across submissions + response = bioproject_cache.get_or_fetch( url, timeout=self.timeout_seconds, use_cache=accession.startswith(PROJECT_PREFIX) ) except requests.RequestException: diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index 27e88c755e..fb734b8dde 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1874,8 +1874,8 @@ class EnaAccessionCase: @pytest.mark.parametrize("case", ena_accession_cases, ids=lambda c: c.name) def test_check_ena_accession(case: EnaAccessionCase) -> None: - external_services.ena_cache.clear() - with mock.patch.object(external_services.ena_cache, "session") as mock_session: + external_services.bioproject_cache.clear() + with mock.patch.object(external_services.bioproject_cache, "session") as mock_session: if isinstance(case.ena_response, Exception): mock_session.get.side_effect = case.ena_response else: From 08be0e4b0b23e6a40f2f2e1b39c8bb217d3fa9d9 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Thu, 24 Sep 2026 18:21:12 +0200 Subject: [PATCH 06/21] Cleanup after rebase --- .../src/loculus_preprocessing/config.py | 8 +++++-- .../external_services.py | 23 ++++++++++--------- .../src/loculus_preprocessing/prepro.py | 1 - .../processing_functions.py | 6 ++--- .../nextclade/tests/factory_methods.py | 12 ++++++---- .../test_metadata_processing_functions.py | 10 ++++++-- 6 files changed, 37 insertions(+), 23 deletions(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/config.py b/preprocessing/nextclade/src/loculus_preprocessing/config.py index b2bf3901f7..9c6a1d43d8 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/config.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/config.py @@ -20,6 +20,7 @@ Topology, ) from loculus_preprocessing.external_services import ( + ENAVisibilityChecker, ExternalServices, FileProcessingService, TaxonomyService, @@ -143,7 +144,9 @@ class Config(BaseModel): # External services taxonomy_service_url: str | None = None _external_services: ExternalServices = PrivateAttr( - default=ExternalServices(taxonomy_service=TaxonomyService(None)) + default=ExternalServices( + taxonomy_service=TaxonomyService(None), ena_visibility_checker=ENAVisibilityChecker() + ) ) raw_reads_processing_service_url: str | None = None raw_reads_processing_service_timeout_seconds: int = 600 @@ -171,7 +174,8 @@ def finalize(self): validate_required_when(self) self.processing_order = get_processing_order(self) self._external_services = ExternalServices( - taxonomy_service=TaxonomyService(self.taxonomy_service_url) + taxonomy_service=TaxonomyService(self.taxonomy_service_url), + ena_visibility_checker=ENAVisibilityChecker(), ) return self diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index 4e2af5a0c8..eb450122e2 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -217,17 +217,6 @@ def get_common_name(self, tax_id: str) -> RawProcessingResult: return RawProcessingResult(datum=common_name) -@dataclass(frozen=True) -class ExternalServices: - """External services available to processing functions. - - Kept separate from `ProcessingContext` since these don't vary per accession, unlike - `ProcessingContext`'s fields. - """ - - taxonomy_service: TaxonomyService - - FileName = str @@ -378,3 +367,15 @@ def check_visibility(self, accession: str) -> RawProcessingResult: if response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR: return processing_error(ena_error_message) return processing_error(f"accession '{accession}' does not exist on ENA") + + +@dataclass(frozen=True) +class ExternalServices: + """External services available to processing functions. + + Kept separate from `ProcessingContext` since these don't vary per accession, unlike + `ProcessingContext`'s fields. + """ + + taxonomy_service: TaxonomyService + ena_visibility_checker: ENAVisibilityChecker diff --git a/preprocessing/nextclade/src/loculus_preprocessing/prepro.py b/preprocessing/nextclade/src/loculus_preprocessing/prepro.py index 5b65d43556..67b9542be8 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/prepro.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/prepro.py @@ -6,7 +6,6 @@ from typing import Any from pydantic import ValidationError -from loculus_preprocessing.external_services import ENAVisibilityChecker from .backend import ( download_diamond_db, diff --git a/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py b/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py index 478b9c8f0b..b153cc01f4 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py @@ -18,7 +18,6 @@ import pytz from loculus_preprocessing.external_services import ExternalServices -from loculus_preprocessing.external_services import ENAVisibilityChecker, TaxonomyService from .datatypes import ( AnnotationSourceType, @@ -1299,19 +1298,20 @@ def common_name_from_id( # noqa: PLR0913, PLR0917 return external_services.taxonomy_service.get_common_name(tax_id) @staticmethod - def check_ena_accession( + def check_ena_accession( # noqa: PLR0913, PLR0917 input_data: InputMetadata, output_field: str, input_fields: list[str], args: FunctionArgs, context: ProcessingContext, + external_services: ExternalServices, ) -> RawProcessingResult: accession: str | None = input_data.get("accession") if not accession: return RawProcessingResult() if context.is_insdc_ingest_group: return RawProcessingResult(datum=accession) - return context.ena_checker.check_visibility(accession) + return external_services.ena_visibility_checker.check_visibility(accession) def single_metadata_annotation( diff --git a/preprocessing/nextclade/tests/factory_methods.py b/preprocessing/nextclade/tests/factory_methods.py index 0c93ba1ed2..af9845320a 100644 --- a/preprocessing/nextclade/tests/factory_methods.py +++ b/preprocessing/nextclade/tests/factory_methods.py @@ -22,10 +22,12 @@ SegmentName, UnprocessedEntry, ) -from loculus_preprocessing.external_services import ExternalServices, TaxonomyService +from loculus_preprocessing.external_services import ( + ENAVisibilityChecker, + ExternalServices, + TaxonomyService, +) from loculus_preprocessing.nextclade_annotation import GffAttributes, NextcladeAnnotation -from loculus_preprocessing.external_services import ENAVisibilityChecker, TaxonomyService -from loculus_preprocessing.processing_functions import ProcessingContext # Default ProcessingContext for tests that don't care about its contents (no INSDC # ingest group, no submittedAt). Tests that do care about specific field(s) should use @@ -40,7 +42,9 @@ # Default ExternalServices for tests that don't care about its contents (no taxonomy # service URL configured). -DEFAULT_EXTERNAL_SERVICES = ExternalServices(taxonomy_service=TaxonomyService(None)) +DEFAULT_EXTERNAL_SERVICES = ExternalServices( + taxonomy_service=TaxonomyService(None), ena_visibility_checker=ENAVisibilityChecker() +) def ts_from_ymd(year: int, month: int, day: int) -> str: diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index fb734b8dde..570dda874c 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1873,7 +1873,7 @@ class EnaAccessionCase: @pytest.mark.parametrize("case", ena_accession_cases, ids=lambda c: c.name) -def test_check_ena_accession(case: EnaAccessionCase) -> None: +def test_check_ena_accession(case: EnaAccessionCase, config: Config) -> None: external_services.bioproject_cache.clear() with mock.patch.object(external_services.bioproject_cache, "session") as mock_session: if isinstance(case.ena_response, Exception): @@ -1886,7 +1886,13 @@ def test_check_ena_accession(case: EnaAccessionCase) -> None: output_field="bioprojectAccession", input_fields=["bioprojectAccession"], args={}, - context=replace(DEFAULT_TEST_CONTEXT, is_insdc_ingest_group=case.is_insdc_ingest_group), + context=replace( + DEFAULT_TEST_CONTEXT, + group_id=config.insdc_ingest_group_id + if case.is_insdc_ingest_group + else config.insdc_ingest_group_id + 1, + ), + external_services=DEFAULT_EXTERNAL_SERVICES, ) assert result.datum == case.expected_datum From 7fe982662cc6b6801509e352fd0b5435ee908cf2 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Fri, 25 Sep 2026 16:02:44 +0200 Subject: [PATCH 07/21] Check for umbrella projects --- .../loculus_preprocessing/external_services.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index eb450122e2..3c588d76b6 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -1,5 +1,7 @@ +import io import logging import urllib.parse +import xml.etree.ElementTree as ET from collections import OrderedDict from dataclasses import dataclass from http import HTTPStatus @@ -354,20 +356,31 @@ def check_visibility(self, accession: str) -> RawProcessingResult: f"unable to validate accession '{accession}': could not reach ENA, " "please try resubmitting later" ) + is_bioproject = accession.startswith(PROJECT_PREFIX) try: # Only cache bioprojects as they're what's likely to be shared across submissions response = bioproject_cache.get_or_fetch( - url, timeout=self.timeout_seconds, use_cache=accession.startswith(PROJECT_PREFIX) + url, timeout=self.timeout_seconds, use_cache=is_bioproject ) except requests.RequestException: return processing_error(ena_error_message) if response.status_code == HTTPStatus.OK: + if is_bioproject and self._is_umbrella_project(response.content): + return processing_error(f"bioproject '{accession}' is an umbrella project") return RawProcessingResult(datum=accession) if response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR: return processing_error(ena_error_message) return processing_error(f"accession '{accession}' does not exist on ENA") + def _is_umbrella_project(self, xml_bytes) -> bool: + for _, elem in ET.iterparse(io.BytesIO(xml_bytes), events=("start",)): # noqa: S314 + if elem.tag == "UMBRELLA_PROJECT": + return True + if elem.tag == "SUBMISSION_PROJECT": + return False + return False + @dataclass(frozen=True) class ExternalServices: From e1226d1ec03fd1e8f5191535a733127ffd57f887 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Sun, 27 Sep 2026 14:16:46 +0200 Subject: [PATCH 08/21] Update comments --- .../src/loculus_preprocessing/external_services.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index 3c588d76b6..60032522c1 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -352,10 +352,7 @@ def __init__( def check_visibility(self, accession: str) -> RawProcessingResult: file_type = "xml" if accession.startswith(XML_PREFIXES) else "embl" url = f"https://www.ebi.ac.uk/ena/browser/api/{file_type}/{accession}" - ena_error_message = ( - f"unable to validate accession '{accession}': could not reach ENA, " - "please try resubmitting later" - ) + ena_error_message = f"unable to validate accession '{accession}': could not reach ENA." is_bioproject = accession.startswith(PROJECT_PREFIX) try: # Only cache bioprojects as they're what's likely to be shared across submissions @@ -367,7 +364,10 @@ def check_visibility(self, accession: str) -> RawProcessingResult: if response.status_code == HTTPStatus.OK: if is_bioproject and self._is_umbrella_project(response.content): - return processing_error(f"bioproject '{accession}' is an umbrella project") + return processing_error( + f"bioproject '{accession}' is an umbrella project. " + "Please provide the accession of a submission project instead." + ) return RawProcessingResult(datum=accession) if response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR: return processing_error(ena_error_message) From 966febda3adfd2735d783f7f73e448e8f6a0fd86 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Sun, 27 Sep 2026 14:39:15 +0200 Subject: [PATCH 09/21] Fix comment --- ena-submission/src/ena_deposition/check_external_visibility.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ena-submission/src/ena_deposition/check_external_visibility.py b/ena-submission/src/ena_deposition/check_external_visibility.py index 918c28296c..0f2794d6fc 100644 --- a/ena-submission/src/ena_deposition/check_external_visibility.py +++ b/ena-submission/src/ena_deposition/check_external_visibility.py @@ -71,7 +71,7 @@ class ENAVisibilityChecker(VisibilityChecker): """Checker for ENA visibility This class is adapted and re-used in - preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py + preprocessing/nextclade/src/loculus_preprocessing/external_services.py If you change anything here, check if the update needs to be applied there as well. """ From 44824fb9926ef2335d8a0220f2676d5c02be29d6 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Sun, 27 Sep 2026 15:01:18 +0200 Subject: [PATCH 10/21] Review suggestions --- kubernetes/loculus/values.yaml | 4 + .../external_services.py | 44 +++++---- .../processing_functions.py | 11 ++- .../test_metadata_processing_functions.py | 91 ++++++++++++++++--- 4 files changed, 121 insertions(+), 29 deletions(-) diff --git a/kubernetes/loculus/values.yaml b/kubernetes/loculus/values.yaml index 696ab37e3b..43d9b3f0ec 100644 --- a/kubernetes/loculus/values.yaml +++ b/kubernetes/loculus/values.yaml @@ -725,6 +725,8 @@ defaultOrganismConfig: &defaultOrganismConfig function: check_ena_accession inputs: accession: bioprojectAccession + args: + accession_type: bioproject header: "INSDC" orderOnDetailsPage: 1060 ingest: bioprojects @@ -756,6 +758,8 @@ defaultOrganismConfig: &defaultOrganismConfig function: check_ena_accession inputs: accession: biosampleAccession + args: + accession_type: biosample header: "INSDC" orderOnDetailsPage: 1080 guidance: "Only add this field if you have already submitted the raw reads to the INSDC, read more at https://pathoplexus.org/docs/how-to/upload-sequences. If you add this field we will not create a new biosample for you and assume you have submitted all required metadata in the linked biosample." diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index 60032522c1..bffccd2eb1 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -1,9 +1,11 @@ import io import logging +import re import urllib.parse import xml.etree.ElementTree as ET from collections import OrderedDict from dataclasses import dataclass +from enum import StrEnum from http import HTTPStatus import requests @@ -320,18 +322,19 @@ def _annotation( # so keeping the cache small. Should still have high hit rate when # all submissions for a batch have the same project bioproject_cache = RequestCache(max_size=16) -PROJECT_PREFIX = "PRJ" -XML_PREFIXES = ( - PROJECT_PREFIX, - "SAM", - "GCA", - "ERR", - "ERX", - "SRR", - "SRX", - "DRR", - "DRX", -) + + +class EnaAccessionType(StrEnum): + BIOPROJECT = "bioproject" + BIOSAMPLE = "biosample" + + +# Matched against the upper-cased accession. Rules out surrounding whitespace and +# comma-separated lists, which the ENA browser API would otherwise resolve to multiple records. +ACCESSION_PATTERNS: dict[EnaAccessionType, re.Pattern[str]] = { + EnaAccessionType.BIOPROJECT: re.compile(r"PRJ[EDN][A-Z][0-9]+"), + EnaAccessionType.BIOSAMPLE: re.compile(r"SAM[EDN][A-Z]?[0-9]+"), +} class ENAVisibilityChecker: @@ -349,11 +352,20 @@ def __init__( ): self.timeout_seconds = timeout_seconds - def check_visibility(self, accession: str) -> RawProcessingResult: - file_type = "xml" if accession.startswith(XML_PREFIXES) else "embl" - url = f"https://www.ebi.ac.uk/ena/browser/api/{file_type}/{accession}" + def check_visibility( + self, accession: str, accession_type: EnaAccessionType + ) -> RawProcessingResult: + # Store accessions upper-cased so they're uniform in the database and on the website + accession = accession.upper() + pattern = ACCESSION_PATTERNS[accession_type] + if not pattern.fullmatch(accession): + return processing_error( + f"'{accession}' is not a valid {accession_type} accession, expected a value " + f"matching '{pattern.pattern}'." + ) + url = f"https://www.ebi.ac.uk/ena/browser/api/xml/{accession}" ena_error_message = f"unable to validate accession '{accession}': could not reach ENA." - is_bioproject = accession.startswith(PROJECT_PREFIX) + is_bioproject = accession_type == EnaAccessionType.BIOPROJECT try: # Only cache bioprojects as they're what's likely to be shared across submissions response = bioproject_cache.get_or_fetch( diff --git a/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py b/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py index b153cc01f4..5b7f57f78f 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py @@ -17,7 +17,7 @@ import dateutil.parser as dateutil import pytz -from loculus_preprocessing.external_services import ExternalServices +from loculus_preprocessing.external_services import EnaAccessionType, ExternalServices from .datatypes import ( AnnotationSourceType, @@ -1306,12 +1306,19 @@ def check_ena_accession( # noqa: PLR0913, PLR0917 context: ProcessingContext, external_services: ExternalServices, ) -> RawProcessingResult: + try: + accession_type = EnaAccessionType(args.get("accession_type")) + except ValueError: + return raw_internal_error( + f"check_ena_accession did not receive a valid accession_type, expected one of " + f"{[t.value for t in EnaAccessionType]}, got args {args}." + ) accession: str | None = input_data.get("accession") if not accession: return RawProcessingResult() if context.is_insdc_ingest_group: return RawProcessingResult(datum=accession) - return external_services.ena_visibility_checker.check_visibility(accession) + return external_services.ena_visibility_checker.check_visibility(accession, accession_type) def single_metadata_annotation( diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index 570dda874c..665aabb60c 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1807,13 +1807,35 @@ def test_call_function_converts_raw_errors_to_annotations() -> None: ) +SUBMISSION_PROJECT_XML = b""" + + A submission project + + +""" + +UMBRELLA_PROJECT_XML = b""" + + An umbrella project + + + + + +""" + +BIOSAMPLE_XML = b"""""" + + @dataclass class EnaAccessionCase: name: str accession: str + accession_type: str = "bioproject" # How ENA responds: an HTTP status code, or an exception raised by the session. # None means ENA must not be contacted at all. ena_response: int | Exception | None = None + ena_xml: bytes = b"" is_insdc_ingest_group: bool = False expected_datum: str | None = None expected_error: str | None = None @@ -1825,15 +1847,61 @@ class EnaAccessionCase: name="public_bioproject_is_accepted", accession="PRJEB12345", ena_response=200, + ena_xml=SUBMISSION_PROJECT_XML, expected_datum="PRJEB12345", expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB12345", ), EnaAccessionCase( - name="nucleotide_accession_uses_embl_endpoint", - accession="OZ123456", + name="umbrella_bioproject_is_rejected", + accession="PRJEB106529", + ena_response=200, + ena_xml=UMBRELLA_PROJECT_XML, + expected_error="is an umbrella project", + expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB106529", + ), + EnaAccessionCase( + name="public_biosample_is_accepted", + accession="SAMEA123456", + accession_type="biosample", ena_response=200, - expected_datum="OZ123456", - expected_url="https://www.ebi.ac.uk/ena/browser/api/embl/OZ123456", + ena_xml=BIOSAMPLE_XML, + expected_datum="SAMEA123456", + expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/SAMEA123456", + ), + EnaAccessionCase( + name="biosample_in_bioproject_field_is_rejected", + accession="SAMN12345678", + expected_error="not a valid bioproject accession", + ), + EnaAccessionCase( + name="bioproject_in_biosample_field_is_rejected", + accession="PRJEB12345", + accession_type="biosample", + expected_error="not a valid biosample accession", + ), + EnaAccessionCase( + name="comma_separated_bioprojects_are_rejected", + accession="PRJNA591860,PRJEB106529", + expected_error="not a valid bioproject accession", + ), + EnaAccessionCase( + name="lowercase_accession_is_uppercased", + accession="prjeb12345", + ena_response=200, + ena_xml=SUBMISSION_PROJECT_XML, + expected_datum="PRJEB12345", + expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB12345", + ), + EnaAccessionCase( + name="surrounding_whitespace_is_rejected", + accession="PRJEB12345 ", + expected_error="not a valid bioproject accession", + ), + EnaAccessionCase( + name="invalid_accession_type_is_internal_error", + accession="PRJEB12345", + accession_type="nucleotide", + expected_error="did not receive a valid accession_type", ), EnaAccessionCase( name="unknown_accession_is_rejected", @@ -1864,7 +1932,7 @@ class EnaAccessionCase: ), EnaAccessionCase( name="insdc_ingested_accession_is_not_checked", - # ingest joins multiple bioprojects with commas, which ENA cannot resolve + # ingest joins multiple bioprojects with commas; ingested values are passed through accession="PRJNA123,PRJNA456", is_insdc_ingest_group=True, expected_datum="PRJNA123,PRJNA456", @@ -1873,24 +1941,25 @@ class EnaAccessionCase: @pytest.mark.parametrize("case", ena_accession_cases, ids=lambda c: c.name) -def test_check_ena_accession(case: EnaAccessionCase, config: Config) -> None: +def test_check_ena_accession(case: EnaAccessionCase) -> None: external_services.bioproject_cache.clear() with mock.patch.object(external_services.bioproject_cache, "session") as mock_session: if isinstance(case.ena_response, Exception): mock_session.get.side_effect = case.ena_response else: - mock_session.get.return_value = mock.MagicMock(status_code=case.ena_response) + mock_session.get.return_value = mock.MagicMock( + status_code=case.ena_response, content=case.ena_xml + ) + ingest_group_id = DEFAULT_TEST_CONTEXT.insdc_ingest_group_id result = ProcessingFunctions.check_ena_accession( input_data={"accession": case.accession}, output_field="bioprojectAccession", input_fields=["bioprojectAccession"], - args={}, + args={"accession_type": case.accession_type}, context=replace( DEFAULT_TEST_CONTEXT, - group_id=config.insdc_ingest_group_id - if case.is_insdc_ingest_group - else config.insdc_ingest_group_id + 1, + group_id=ingest_group_id if case.is_insdc_ingest_group else ingest_group_id + 1, ), external_services=DEFAULT_EXTERNAL_SERVICES, ) From 0cf3a92d221ae6e0e991f441f1a12050e636d6b9 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Sun, 27 Sep 2026 15:22:57 +0200 Subject: [PATCH 11/21] Simplify --- .../external_services.py | 33 +++++++------------ .../test_metadata_processing_functions.py | 4 +-- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index bffccd2eb1..df43eb9fba 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -65,24 +65,19 @@ def set(self, url: str, response: requests.Response) -> None: if len(self.cache) > self.max_size: self.cache.popitem(last=False) - def get_or_fetch( - self, url: str, timeout: int = 15, use_cache: bool = True - ) -> requests.Response: + def get_or_fetch(self, url: str, timeout: int = 15) -> requests.Response: """ Check if `url` already exists in the cache and return the cached Response if it does. If `url` is not in the cache, make the actual request (with timeout and retries). Add the Response to the cache (if status code in the 200s), and return the Response. - With `use_cache=False` the request is always made, and the cache is neither read nor - written to. - The caller should wrap this in a try/except block and handle errors. """ - response = self.get(url) if use_cache else None + response = self.get(url) if response is None: response = self.session.get(url, timeout=timeout) - if use_cache and 200 <= response.status_code < 300: # noqa: PLR2004 + if 200 <= response.status_code < 300: # noqa: PLR2004 self.set(url, response) return response @@ -318,10 +313,9 @@ def _annotation( return ProcessingAnnotation([source], [source], _internal_error_message(message)) -# Rarely, a bioproject XML can reach ~10 MB (e.g., PRJNA591860) -# so keeping the cache small. Should still have high hit rate when -# all submissions for a batch have the same project -bioproject_cache = RequestCache(max_size=16) +# Successful ENA responses are small XML documents (a few KB). Cache hits refresh an entry's +# position; a bioproject shared across a batch stays cached while unique biosamples cycle through. +ena_cache = RequestCache(max_size=16) class EnaAccessionType(StrEnum): @@ -329,8 +323,8 @@ class EnaAccessionType(StrEnum): BIOSAMPLE = "biosample" -# Matched against the upper-cased accession. Rules out surrounding whitespace and -# comma-separated lists, which the ENA browser API would otherwise resolve to multiple records. +# Rules out surrounding whitespace and comma-separated lists, which the ENA browser API +# would otherwise resolve to multiple records. ACCESSION_PATTERNS: dict[EnaAccessionType, re.Pattern[str]] = { EnaAccessionType.BIOPROJECT: re.compile(r"PRJ[EDN][A-Z][0-9]+"), EnaAccessionType.BIOSAMPLE: re.compile(r"SAM[EDN][A-Z]?[0-9]+"), @@ -355,7 +349,6 @@ def __init__( def check_visibility( self, accession: str, accession_type: EnaAccessionType ) -> RawProcessingResult: - # Store accessions upper-cased so they're uniform in the database and on the website accession = accession.upper() pattern = ACCESSION_PATTERNS[accession_type] if not pattern.fullmatch(accession): @@ -365,17 +358,15 @@ def check_visibility( ) url = f"https://www.ebi.ac.uk/ena/browser/api/xml/{accession}" ena_error_message = f"unable to validate accession '{accession}': could not reach ENA." - is_bioproject = accession_type == EnaAccessionType.BIOPROJECT try: - # Only cache bioprojects as they're what's likely to be shared across submissions - response = bioproject_cache.get_or_fetch( - url, timeout=self.timeout_seconds, use_cache=is_bioproject - ) + response = ena_cache.get_or_fetch(url, timeout=self.timeout_seconds) except requests.RequestException: return processing_error(ena_error_message) if response.status_code == HTTPStatus.OK: - if is_bioproject and self._is_umbrella_project(response.content): + if accession_type == EnaAccessionType.BIOPROJECT and self._is_umbrella_project( + response.content + ): return processing_error( f"bioproject '{accession}' is an umbrella project. " "Please provide the accession of a submission project instead." diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index 665aabb60c..d0ab023211 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1942,8 +1942,8 @@ class EnaAccessionCase: @pytest.mark.parametrize("case", ena_accession_cases, ids=lambda c: c.name) def test_check_ena_accession(case: EnaAccessionCase) -> None: - external_services.bioproject_cache.clear() - with mock.patch.object(external_services.bioproject_cache, "session") as mock_session: + external_services.ena_cache.clear() + with mock.patch.object(external_services.ena_cache, "session") as mock_session: if isinstance(case.ena_response, Exception): mock_session.get.side_effect = case.ena_response else: From 5a53da15837ac1100c52595cc361139b422d26f6 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Sun, 27 Sep 2026 16:06:53 +0200 Subject: [PATCH 12/21] Simplify more --- .../external_services.py | 47 +++++++++++-------- .../processing_functions.py | 10 ++-- .../test_metadata_processing_functions.py | 10 +++- 3 files changed, 39 insertions(+), 28 deletions(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index df43eb9fba..2dd3cec27c 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -346,9 +346,14 @@ def __init__( ): self.timeout_seconds = timeout_seconds - def check_visibility( - self, accession: str, accession_type: EnaAccessionType - ) -> RawProcessingResult: + def check_visibility(self, accession: str, accession_type_arg: str) -> RawProcessingResult: + try: + accession_type = EnaAccessionType(accession_type_arg) + except ValueError: + return raw_internal_error( + f"invalid accession_type '{accession_type_arg}', expected one of " + f"{[t.value for t in EnaAccessionType]}." + ) accession = accession.upper() pattern = ACCESSION_PATTERNS[accession_type] if not pattern.fullmatch(accession): @@ -356,25 +361,27 @@ def check_visibility( f"'{accession}' is not a valid {accession_type} accession, expected a value " f"matching '{pattern.pattern}'." ) - url = f"https://www.ebi.ac.uk/ena/browser/api/xml/{accession}" - ena_error_message = f"unable to validate accession '{accession}': could not reach ENA." try: - response = ena_cache.get_or_fetch(url, timeout=self.timeout_seconds) + response = ena_cache.get_or_fetch( + f"https://www.ebi.ac.uk/ena/browser/api/xml/{accession}", + timeout=self.timeout_seconds, + ) except requests.RequestException: - return processing_error(ena_error_message) - - if response.status_code == HTTPStatus.OK: - if accession_type == EnaAccessionType.BIOPROJECT and self._is_umbrella_project( - response.content - ): - return processing_error( - f"bioproject '{accession}' is an umbrella project. " - "Please provide the accession of a submission project instead." - ) - return RawProcessingResult(datum=accession) - if response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR: - return processing_error(ena_error_message) - return processing_error(f"accession '{accession}' does not exist on ENA") + response = None + if response is None or response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR: + return processing_error( + f"unable to validate accession '{accession}': could not reach ENA." + ) + if response.status_code != HTTPStatus.OK: + return processing_error(f"accession '{accession}' does not exist on ENA") + if accession_type == EnaAccessionType.BIOPROJECT and self._is_umbrella_project( + response.content + ): + return processing_error( + f"bioproject '{accession}' is an umbrella project. " + "Please provide the accession of a submission project instead." + ) + return RawProcessingResult(datum=accession) def _is_umbrella_project(self, xml_bytes) -> bool: for _, elem in ET.iterparse(io.BytesIO(xml_bytes), events=("start",)): # noqa: S314 diff --git a/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py b/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py index 5b7f57f78f..8a414d9d69 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py @@ -17,7 +17,7 @@ import dateutil.parser as dateutil import pytz -from loculus_preprocessing.external_services import EnaAccessionType, ExternalServices +from loculus_preprocessing.external_services import ExternalServices from .datatypes import ( AnnotationSourceType, @@ -1306,12 +1306,10 @@ def check_ena_accession( # noqa: PLR0913, PLR0917 context: ProcessingContext, external_services: ExternalServices, ) -> RawProcessingResult: - try: - accession_type = EnaAccessionType(args.get("accession_type")) - except ValueError: + accession_type = args.get("accession_type") + if not isinstance(accession_type, str): return raw_internal_error( - f"check_ena_accession did not receive a valid accession_type, expected one of " - f"{[t.value for t in EnaAccessionType]}, got args {args}." + f"check_ena_accession requires an accession_type arg, got args {args}." ) accession: str | None = input_data.get("accession") if not accession: diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index d0ab023211..c8387fa54c 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1831,7 +1831,7 @@ def test_call_function_converts_raw_errors_to_annotations() -> None: class EnaAccessionCase: name: str accession: str - accession_type: str = "bioproject" + accession_type: str | None = "bioproject" # How ENA responds: an HTTP status code, or an exception raised by the session. # None means ENA must not be contacted at all. ena_response: int | Exception | None = None @@ -1901,7 +1901,13 @@ class EnaAccessionCase: name="invalid_accession_type_is_internal_error", accession="PRJEB12345", accession_type="nucleotide", - expected_error="did not receive a valid accession_type", + expected_error="invalid accession_type 'nucleotide'", + ), + EnaAccessionCase( + name="missing_accession_type_is_internal_error", + accession="PRJEB12345", + accession_type=None, + expected_error="requires an accession_type arg", ), EnaAccessionCase( name="unknown_accession_is_rejected", From 1584358ed355bdc702af979fe894931288aa6771 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Mon, 28 Sep 2026 14:06:22 +0200 Subject: [PATCH 13/21] Add check for raw reads --- kubernetes/loculus/values.yaml | 6 ++++++ .../external_services.py | 9 ++++++--- .../test_metadata_processing_functions.py | 20 +++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/kubernetes/loculus/values.yaml b/kubernetes/loculus/values.yaml index 43d9b3f0ec..5a306d2e8e 100644 --- a/kubernetes/loculus/values.yaml +++ b/kubernetes/loculus/values.yaml @@ -1611,6 +1611,12 @@ defaultOrganismConfig: &defaultOrganismConfig customDisplay: type: link url: "https://www.ncbi.nlm.nih.gov/sra/?term=__value__" + preprocessing: + function: check_ena_accession + inputs: + accession: insdcRawReadsAccession + args: + accession_type: raw_reads header: "INSDC" orderOnDetailsPage: 1120 ingest: ncbiSraAccessions diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index 2dd3cec27c..53159306b9 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -314,13 +314,15 @@ def _annotation( # Successful ENA responses are small XML documents (a few KB). Cache hits refresh an entry's -# position; a bioproject shared across a batch stays cached while unique biosamples cycle through. +# position; a bioproject shared across a batch stays cached while unique biosamples and runs +# cycle through. ena_cache = RequestCache(max_size=16) class EnaAccessionType(StrEnum): BIOPROJECT = "bioproject" BIOSAMPLE = "biosample" + RAW_READS = "raw_reads" # Rules out surrounding whitespace and comma-separated lists, which the ENA browser API @@ -328,6 +330,7 @@ class EnaAccessionType(StrEnum): ACCESSION_PATTERNS: dict[EnaAccessionType, re.Pattern[str]] = { EnaAccessionType.BIOPROJECT: re.compile(r"PRJ[EDN][A-Z][0-9]+"), EnaAccessionType.BIOSAMPLE: re.compile(r"SAM[EDN][A-Z]?[0-9]+"), + EnaAccessionType.RAW_READS: re.compile(r"[EDS]RR[0-9]+"), } @@ -358,8 +361,8 @@ def check_visibility(self, accession: str, accession_type_arg: str) -> RawProces pattern = ACCESSION_PATTERNS[accession_type] if not pattern.fullmatch(accession): return processing_error( - f"'{accession}' is not a valid {accession_type} accession, expected a value " - f"matching '{pattern.pattern}'." + f"'{accession}' is not a valid {accession_type.replace('_', ' ')} accession, " + f"expected a value matching '{pattern.pattern}'." ) try: response = ena_cache.get_or_fetch( diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index c8387fa54c..064bd07a6e 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1868,6 +1868,26 @@ class EnaAccessionCase: expected_datum="SAMEA123456", expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/SAMEA123456", ), + EnaAccessionCase( + name="public_sra_run_is_accepted", + accession="SRR27477368", + accession_type="raw_reads", + ena_response=200, + expected_datum="SRR27477368", + expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/SRR27477368", + ), + EnaAccessionCase( + name="sra_experiment_in_raw_reads_field_is_rejected", + accession="SRX23184375", + accession_type="raw_reads", + expected_error="not a valid raw_reads accession", + ), + EnaAccessionCase( + name="multiple_runs_are_rejected", + accession="SRR27477368,SRR27477369", + accession_type="raw_reads", + expected_error="not a valid raw_reads accession", + ), EnaAccessionCase( name="biosample_in_bioproject_field_is_rejected", accession="SAMN12345678", From 67f40207b2e2483c170a894ed59bcd3aa21702fc Mon Sep 17 00:00:00 2001 From: maverbiest Date: Mon, 28 Sep 2026 14:24:38 +0200 Subject: [PATCH 14/21] Warn instead of error --- .../external_services.py | 18 ++++++++++--- .../test_metadata_processing_functions.py | 25 +++++++++++++------ 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index 53159306b9..004ae442a7 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -372,11 +372,23 @@ def check_visibility(self, accession: str, accession_type_arg: str) -> RawProces except requests.RequestException: response = None if response is None or response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR: - return processing_error( - f"unable to validate accession '{accession}': could not reach ENA." + return RawProcessingResult( + datum=accession, + warnings=[ + f"unable to check visibility of '{accession}': could not reach ENA. Please " + f"verify yourself that '{accession}' is public before proceeding or we will " + "not be able to propagate your submission to the INSDC." + ], ) if response.status_code != HTTPStatus.OK: - return processing_error(f"accession '{accession}' does not exist on ENA") + return RawProcessingResult( + datum=accession, + warnings=[ + f"accession '{accession}' is not visible on ENA. Please make '{accession}' " + "public before proceeding or we will not be able to propagate your " + "submission to the INSDC." + ], + ) if accession_type == EnaAccessionType.BIOPROJECT and self._is_umbrella_project( response.content ): diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index 064bd07a6e..c7807964e6 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1839,6 +1839,7 @@ class EnaAccessionCase: is_insdc_ingest_group: bool = False expected_datum: str | None = None expected_error: str | None = None + expected_warning: str | None = None expected_url: str | None = None @@ -1880,13 +1881,13 @@ class EnaAccessionCase: name="sra_experiment_in_raw_reads_field_is_rejected", accession="SRX23184375", accession_type="raw_reads", - expected_error="not a valid raw_reads accession", + expected_error="not a valid raw reads accession", ), EnaAccessionCase( name="multiple_runs_are_rejected", accession="SRR27477368,SRR27477369", accession_type="raw_reads", - expected_error="not a valid raw_reads accession", + expected_error="not a valid raw reads accession", ), EnaAccessionCase( name="biosample_in_bioproject_field_is_rejected", @@ -1930,26 +1931,29 @@ class EnaAccessionCase: expected_error="requires an accession_type arg", ), EnaAccessionCase( - name="unknown_accession_is_rejected", + name="not_visible_accession_is_warned_about", accession="PRJEB99999", ena_response=404, - expected_error="does not exist on ENA", + expected_datum="PRJEB99999", + expected_warning="is not visible on ENA", expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB99999", ), EnaAccessionCase( - name="server_error_asks_submitter_to_retry", + name="server_error_is_warned_about", # 501 is not in the session's status_forcelist, so it arrives as a response accession="PRJEB12345", ena_response=501, - expected_error="could not reach ENA", + expected_datum="PRJEB12345", + expected_warning="could not reach ENA", expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB12345", ), EnaAccessionCase( - name="exhausted_retries_ask_submitter_to_retry", + name="exhausted_retries_are_warned_about", # what urllib3 raises once the retries for 429/5xx in status_forcelist run out accession="PRJEB12345", ena_response=requests.exceptions.RetryError(), - expected_error="could not reach ENA", + expected_datum="PRJEB12345", + expected_warning="could not reach ENA", expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB12345", ), EnaAccessionCase( @@ -1996,6 +2000,11 @@ def test_check_ena_accession(case: EnaAccessionCase) -> None: else: assert len(result.errors) == 1 assert case.expected_error in result.errors[0] + if case.expected_warning is None: + assert result.warnings == [] + else: + assert len(result.warnings) == 1 + assert case.expected_warning in result.warnings[0] if case.expected_url is None: assert mock_session.get.call_count == 0 From 442e77a55ed0feaf04d5e5971db6d4f739079edc Mon Sep 17 00:00:00 2001 From: maverbiest Date: Mon, 28 Sep 2026 15:10:23 +0200 Subject: [PATCH 15/21] Suggestions --- .../src/loculus_preprocessing/external_services.py | 11 ++++++----- .../tests/test_metadata_processing_functions.py | 7 +++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index 004ae442a7..cae9192241 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -316,7 +316,7 @@ def _annotation( # Successful ENA responses are small XML documents (a few KB). Cache hits refresh an entry's # position; a bioproject shared across a batch stays cached while unique biosamples and runs # cycle through. -ena_cache = RequestCache(max_size=16) +ena_cache = RequestCache(max_size=16, retries=1) class EnaAccessionType(StrEnum): @@ -325,7 +325,7 @@ class EnaAccessionType(StrEnum): RAW_READS = "raw_reads" -# Rules out surrounding whitespace and comma-separated lists, which the ENA browser API +# Also guards against comma-separated lists, which the ENA browser API # would otherwise resolve to multiple records. ACCESSION_PATTERNS: dict[EnaAccessionType, re.Pattern[str]] = { EnaAccessionType.BIOPROJECT: re.compile(r"PRJ[EDN][A-Z][0-9]+"), @@ -345,7 +345,7 @@ class ENAVisibilityChecker: def __init__( self, - timeout_seconds: int = 30, + timeout_seconds: int = 10, ): self.timeout_seconds = timeout_seconds @@ -357,7 +357,7 @@ def check_visibility(self, accession: str, accession_type_arg: str) -> RawProces f"invalid accession_type '{accession_type_arg}', expected one of " f"{[t.value for t in EnaAccessionType]}." ) - accession = accession.upper() + accession = accession.strip().upper() pattern = ACCESSION_PATTERNS[accession_type] if not pattern.fullmatch(accession): return processing_error( @@ -369,7 +369,8 @@ def check_visibility(self, accession: str, accession_type_arg: str) -> RawProces f"https://www.ebi.ac.uk/ena/browser/api/xml/{accession}", timeout=self.timeout_seconds, ) - except requests.RequestException: + except requests.RequestException as e: + logger.warning(f"Failed to check ENA visibility of '{accession}': {e}") response = None if response is None or response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR: return RawProcessingResult( diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index c7807964e6..79600f73f1 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1914,9 +1914,12 @@ class EnaAccessionCase: expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB12345", ), EnaAccessionCase( - name="surrounding_whitespace_is_rejected", + name="surrounding_whitespace_is_stripped", accession="PRJEB12345 ", - expected_error="not a valid bioproject accession", + ena_response=200, + ena_xml=SUBMISSION_PROJECT_XML, + expected_datum="PRJEB12345", + expected_url="https://www.ebi.ac.uk/ena/browser/api/xml/PRJEB12345", ), EnaAccessionCase( name="invalid_accession_type_is_internal_error", From 8a974a067e68fd2c260317e7a09b548486970c5d Mon Sep 17 00:00:00 2001 From: maverbiest Date: Mon, 28 Sep 2026 15:38:40 +0200 Subject: [PATCH 16/21] Rename --- .../nextclade/tests/test_metadata_processing_functions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index 79600f73f1..3966b9b6e0 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1987,8 +1987,8 @@ def test_check_ena_accession(case: EnaAccessionCase) -> None: ingest_group_id = DEFAULT_TEST_CONTEXT.insdc_ingest_group_id result = ProcessingFunctions.check_ena_accession( input_data={"accession": case.accession}, - output_field="bioprojectAccession", - input_fields=["bioprojectAccession"], + output_field="externalAccession", + input_fields=["externalAccession"], args={"accession_type": case.accession_type}, context=replace( DEFAULT_TEST_CONTEXT, From e5df6860b4a79adcf8a0535374916b4a89919fd7 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Mon, 28 Sep 2026 15:41:21 +0200 Subject: [PATCH 17/21] Tweak --- .../tests/test_metadata_processing_functions.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index 3966b9b6e0..28cbb5a6b4 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1883,12 +1883,6 @@ class EnaAccessionCase: accession_type="raw_reads", expected_error="not a valid raw reads accession", ), - EnaAccessionCase( - name="multiple_runs_are_rejected", - accession="SRR27477368,SRR27477369", - accession_type="raw_reads", - expected_error="not a valid raw reads accession", - ), EnaAccessionCase( name="biosample_in_bioproject_field_is_rejected", accession="SAMN12345678", @@ -1915,7 +1909,7 @@ class EnaAccessionCase: ), EnaAccessionCase( name="surrounding_whitespace_is_stripped", - accession="PRJEB12345 ", + accession=" PRJEB12345 ", ena_response=200, ena_xml=SUBMISSION_PROJECT_XML, expected_datum="PRJEB12345", @@ -1965,7 +1959,6 @@ class EnaAccessionCase: ), EnaAccessionCase( name="insdc_ingested_accession_is_not_checked", - # ingest joins multiple bioprojects with commas; ingested values are passed through accession="PRJNA123,PRJNA456", is_insdc_ingest_group=True, expected_datum="PRJNA123,PRJNA456", From c569c8ad7c0677a24c788c01cf3db43f3be91463 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Mon, 28 Sep 2026 16:30:27 +0200 Subject: [PATCH 18/21] Add new function to README --- preprocessing/nextclade/README.md | 1 + .../nextclade/tests/test_metadata_processing_functions.py | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/preprocessing/nextclade/README.md b/preprocessing/nextclade/README.md index 3e42e18be3..2297c43a1f 100644 --- a/preprocessing/nextclade/README.md +++ b/preprocessing/nextclade/README.md @@ -139,6 +139,7 @@ taxonomyService: 9. `resolve_host_taxon_id`: Validates that a host taxon ID or scientific name exists in the NCBI taxonomy, returning the taxon ID if validation is successful. Requires an input field called `host` (entries ingested from the INSDC do not error). 10. `scientific_name_from_id`: Returns a scientific name for a taxon given a taxon ID `hostTaxonId` as input (entries ingested from the INSDC do not error). 11. `common_name_from_id`: Returns a common name for a taxon given a taxon ID `hostTaxonId` as input (entries ingested from the INSDC do not error). +12. `check_ena_accession`: checks whether a submitter-provided `biosampleAccession`, `bioprojectAccession`, or `insdcRawReadsAccession` is publicly visible on ENA. Using these functions in your `values.yaml` will look like: diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index 28cbb5a6b4..801ffa304e 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1937,7 +1937,6 @@ class EnaAccessionCase: ), EnaAccessionCase( name="server_error_is_warned_about", - # 501 is not in the session's status_forcelist, so it arrives as a response accession="PRJEB12345", ena_response=501, expected_datum="PRJEB12345", @@ -1946,7 +1945,6 @@ class EnaAccessionCase: ), EnaAccessionCase( name="exhausted_retries_are_warned_about", - # what urllib3 raises once the retries for 429/5xx in status_forcelist run out accession="PRJEB12345", ena_response=requests.exceptions.RetryError(), expected_datum="PRJEB12345", From 12422c5a19432f9159d7fb05d297a73ef6544223 Mon Sep 17 00:00:00 2001 From: maverbiest <59956362+maverbiest@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:49:05 +0200 Subject: [PATCH 19/21] Apply batched suggestions from code review Co-authored-by: Anna (Anya) Parker <50943381+anna-parker@users.noreply.github.com> --- .../src/loculus_preprocessing/external_services.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index cae9192241..10c77de801 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -330,7 +330,7 @@ class EnaAccessionType(StrEnum): ACCESSION_PATTERNS: dict[EnaAccessionType, re.Pattern[str]] = { EnaAccessionType.BIOPROJECT: re.compile(r"PRJ[EDN][A-Z][0-9]+"), EnaAccessionType.BIOSAMPLE: re.compile(r"SAM[EDN][A-Z]?[0-9]+"), - EnaAccessionType.RAW_READS: re.compile(r"[EDS]RR[0-9]+"), + EnaAccessionType.RAW_READS: re.compile(r"[EDS]RR[0-9]{6,}"), } @@ -354,7 +354,7 @@ def check_visibility(self, accession: str, accession_type_arg: str) -> RawProces accession_type = EnaAccessionType(accession_type_arg) except ValueError: return raw_internal_error( - f"invalid accession_type '{accession_type_arg}', expected one of " + f"Invalid accession_type '{accession_type_arg}', expected one of " f"{[t.value for t in EnaAccessionType]}." ) accession = accession.strip().upper() @@ -376,7 +376,7 @@ def check_visibility(self, accession: str, accession_type_arg: str) -> RawProces return RawProcessingResult( datum=accession, warnings=[ - f"unable to check visibility of '{accession}': could not reach ENA. Please " + f"Unable to check visibility of '{accession}': could not reach ENA. Please " f"verify yourself that '{accession}' is public before proceeding or we will " "not be able to propagate your submission to the INSDC." ], @@ -385,7 +385,7 @@ def check_visibility(self, accession: str, accession_type_arg: str) -> RawProces return RawProcessingResult( datum=accession, warnings=[ - f"accession '{accession}' is not visible on ENA. Please make '{accession}' " + f"Accession '{accession}' is not visible on ENA. Please make '{accession}' " "public before proceeding or we will not be able to propagate your " "submission to the INSDC." ], From 9b129093142c07575eec556885299568d9fd4d00 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Tue, 20 Oct 2026 18:07:51 +0200 Subject: [PATCH 20/21] Fix test --- .../nextclade/tests/test_metadata_processing_functions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index 801ffa304e..6e449e2e1c 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1919,7 +1919,7 @@ class EnaAccessionCase: name="invalid_accession_type_is_internal_error", accession="PRJEB12345", accession_type="nucleotide", - expected_error="invalid accession_type 'nucleotide'", + expected_error="Invalid accession_type 'nucleotide'", ), EnaAccessionCase( name="missing_accession_type_is_internal_error", From 3c4a6a6abfc9554d52061883fcf9c0d4b2bcc2d7 Mon Sep 17 00:00:00 2001 From: maverbiest Date: Wed, 21 Oct 2026 09:43:40 +0200 Subject: [PATCH 21/21] Clarify comments --- .../nextclade/src/loculus_preprocessing/external_services.py | 1 + .../nextclade/tests/test_metadata_processing_functions.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py index 10c77de801..920ea1f388 100644 --- a/preprocessing/nextclade/src/loculus_preprocessing/external_services.py +++ b/preprocessing/nextclade/src/loculus_preprocessing/external_services.py @@ -327,6 +327,7 @@ class EnaAccessionType(StrEnum): # Also guards against comma-separated lists, which the ENA browser API # would otherwise resolve to multiple records. +# (see https://ena-docs.readthedocs.io/en/latest/submit/general-guide/accessions.html) ACCESSION_PATTERNS: dict[EnaAccessionType, re.Pattern[str]] = { EnaAccessionType.BIOPROJECT: re.compile(r"PRJ[EDN][A-Z][0-9]+"), EnaAccessionType.BIOSAMPLE: re.compile(r"SAM[EDN][A-Z]?[0-9]+"), diff --git a/preprocessing/nextclade/tests/test_metadata_processing_functions.py b/preprocessing/nextclade/tests/test_metadata_processing_functions.py index 6e449e2e1c..157f53c291 100644 --- a/preprocessing/nextclade/tests/test_metadata_processing_functions.py +++ b/preprocessing/nextclade/tests/test_metadata_processing_functions.py @@ -1833,13 +1833,13 @@ class EnaAccessionCase: accession: str accession_type: str | None = "bioproject" # How ENA responds: an HTTP status code, or an exception raised by the session. - # None means ENA must not be contacted at all. ena_response: int | Exception | None = None ena_xml: bytes = b"" is_insdc_ingest_group: bool = False expected_datum: str | None = None expected_error: str | None = None expected_warning: str | None = None + # None means ENA must not be contacted at all. expected_url: str | None = None