From 4bc72ee446014e75f88f80a42f895cf8ace9894d Mon Sep 17 00:00:00 2001 From: Emilio Righi Date: Fri, 25 Sep 2026 08:06:28 +0200 Subject: [PATCH] falback to last modified date when release date is empty --- server/db/models.py | 43 +++++- server/jobs/services/annotation.py | 32 ++-- server/jobs/services/classes.py | 25 ++- .../unit/jobs/test_annotation_job_helpers.py | 143 ++++++++++++++++++ 4 files changed, 225 insertions(+), 18 deletions(-) diff --git a/server/db/models.py b/server/db/models.py index 54f6ee7..1b7f3e7 100644 --- a/server/db/models.py +++ b/server/db/models.py @@ -281,11 +281,50 @@ class GenomeAnnotation(DynamicDocument): "busco.missing", ] } + @staticmethod def parse_iso_date(iso_date: str) -> datetime: """ - Parse an ISO date string to a datetime object + Parse an ISO date string to a datetime object. + Raises ValueError if iso_date is missing or not a valid ISO date. """ - return datetime.fromisoformat(iso_date) + if iso_date is None or (isinstance(iso_date, str) and not iso_date.strip()): + raise ValueError("Missing or empty ISO date string") + return datetime.fromisoformat(iso_date.strip()) + + @staticmethod + def try_parse_iso_date(iso_date: str | None) -> datetime | None: + """Parse an ISO date string, returning None if missing or invalid.""" + try: + return GenomeAnnotation.parse_iso_date(iso_date) + except (TypeError, ValueError): + return None + + @staticmethod + def resolve_source_dates( + release_date: str | None, + last_modified: str | None, + ) -> tuple[datetime, datetime]: + """ + Resolve release_date and last_modified for SourceFileInfo. + + Empty/missing release_date falls back to last_modified (same rule as the + community annotation tracker). Raises ValueError if last_modified is also + missing/invalid, or if release_date is present but unparseable and there + is no valid last_modified fallback for a missing release only. + """ + last_mod_dt = GenomeAnnotation.try_parse_iso_date(last_modified) + release_raw = release_date.strip() if isinstance(release_date, str) else release_date + if release_raw is None or release_raw == "": + if last_mod_dt is None: + raise ValueError("Missing release_date and last_modified") + return last_mod_dt, last_mod_dt + + release_dt = GenomeAnnotation.try_parse_iso_date(release_date) + if release_dt is None: + raise ValueError(f"Invalid release_date: {release_date!r}") + if last_mod_dt is None: + raise ValueError(f"Missing or invalid last_modified: {last_modified!r}") + return release_dt, last_mod_dt class TaxonNode(Document): children = ListField(StringField()) diff --git a/server/jobs/services/annotation.py b/server/jobs/services/annotation.py index 7aae690..478a8be 100644 --- a/server/jobs/services/annotation.py +++ b/server/jobs/services/annotation.py @@ -51,15 +51,23 @@ def __init__(self, md5_checksum: str): ) def handle_annotation_error(annotation_to_process: AnnotationToProcess, error: str): - """Handle annotation processing errors.""" + """Handle annotation processing errors. + + Never raises: a failure to persist the error document must not abort the + import batch (e.g. incomplete dates on AnnotationError). + """ url_path = annotation_to_process.access_url source_md5 = annotation_to_process.md5_checksum # Check by url_path first (unique constraint), then by source_md5 as fallback - annotation_error = AnnotationError.objects(url_path=url_path).first() - if not annotation_error: - annotation_error = AnnotationError.objects(source_md5=source_md5).first() + try: + annotation_error = AnnotationError.objects(url_path=url_path).first() + if not annotation_error: + annotation_error = AnnotationError.objects(source_md5=source_md5).first() + except Exception as e: + print(f"Failed to look up AnnotationError for {url_path}: {e}") + return if isinstance(error, Exception): error = str(error) @@ -67,13 +75,15 @@ def handle_annotation_error(annotation_to_process: AnnotationToProcess, error: s if isinstance(error, str): error = error.replace('\n', ';') - if annotation_error: - annotation_error.error_message = error - annotation_error.save() - - else: - annotation_error = annotation_to_process.to_annotation_error(error) - annotation_error.save() + try: + if annotation_error: + annotation_error.error_message = error + annotation_error.save() + else: + annotation_error = annotation_to_process.to_annotation_error(error) + annotation_error.save() + except Exception as e: + print(f"Failed to save AnnotationError for {url_path}: {e}") def update_annotation_source_metadata(existing: GenomeAnnotation, annotation_to_process: AnnotationToProcess) -> bool: diff --git a/server/jobs/services/classes.py b/server/jobs/services/classes.py index a6fd2a2..55c7bd5 100644 --- a/server/jobs/services/classes.py +++ b/server/jobs/services/classes.py @@ -1,4 +1,5 @@ from array import array +from datetime import datetime, timezone from db.models import GenomeAnnotation, AnnotationError, Organism, TaxonNode, GenomicSequence, GenomeAssembly from db.embedded_documents import SourceFileInfo, PipelineInfo, AssemblyStats, BuscoScore import re @@ -29,12 +30,15 @@ def to_genome_annotation(self, **kwargs) -> GenomeAnnotation: """ Convert the AnnotationToProcess object to a GenomeAnnotation document """ + release_dt, last_mod_dt = GenomeAnnotation.resolve_source_dates( + self.release_date, self.last_modified + ) source_info = SourceFileInfo( database=self.source_database, provider=self.annotation_provider, - release_date=GenomeAnnotation.parse_iso_date(self.release_date), + release_date=release_dt, url_path=self.access_url, - last_modified=GenomeAnnotation.parse_iso_date(self.last_modified), + last_modified=last_mod_dt, uncompressed_md5=self.md5_checksum, ) if self.pipeline_name: @@ -54,8 +58,19 @@ def to_genome_annotation(self, **kwargs) -> GenomeAnnotation: def to_annotation_error(self, error_message: str) -> AnnotationError: """ - Convert the AnnotationToProcess object to an AnnotationError document + Convert the AnnotationToProcess object to an AnnotationError document. + + Dates always resolve to valid datetimes so error recording cannot fail + on incomplete tracker metadata (fall back to last_modified, then utcnow). """ + try: + release_dt, last_mod_dt = GenomeAnnotation.resolve_source_dates( + self.release_date, self.last_modified + ) + except ValueError: + now = datetime.now(timezone.utc) + last_mod_dt = GenomeAnnotation.try_parse_iso_date(self.last_modified) or now + release_dt = GenomeAnnotation.try_parse_iso_date(self.release_date) or last_mod_dt return AnnotationError( assembly_accession=self.assembly_accession, taxid=self.taxon_id, @@ -63,8 +78,8 @@ def to_annotation_error(self, error_message: str) -> AnnotationError: error_message=error_message, url_path=self.access_url, source_md5=self.md5_checksum, - release_date=self.release_date, - last_modified=self.last_modified, + release_date=release_dt, + last_modified=last_mod_dt, source_database=self.source_database, ) diff --git a/server/tests/unit/jobs/test_annotation_job_helpers.py b/server/tests/unit/jobs/test_annotation_job_helpers.py index 8df39a3..c6accc7 100644 --- a/server/tests/unit/jobs/test_annotation_job_helpers.py +++ b/server/tests/unit/jobs/test_annotation_job_helpers.py @@ -311,3 +311,146 @@ def test_returns_md5_and_size_when_not_a_duplicate(self, tmp_path): assert md5_result == content_md5 assert file_size > 0 + + +class TestResolveSourceDates: + def test_empty_release_date_falls_back_to_last_modified(self): + from datetime import datetime + from db.models import GenomeAnnotation + + release_dt, last_mod_dt = GenomeAnnotation.resolve_source_dates("", "2026-08-27") + assert release_dt == datetime.fromisoformat("2026-08-27") + assert last_mod_dt == datetime.fromisoformat("2026-08-27") + + def test_whitespace_release_date_falls_back(self): + from datetime import datetime + from db.models import GenomeAnnotation + + release_dt, last_mod_dt = GenomeAnnotation.resolve_source_dates(" ", "2026-08-27") + assert release_dt == last_mod_dt == datetime.fromisoformat("2026-08-27") + + def test_both_present(self): + from datetime import datetime + from db.models import GenomeAnnotation + + release_dt, last_mod_dt = GenomeAnnotation.resolve_source_dates( + "2024-01-02", "2024-01-01" + ) + assert release_dt == datetime.fromisoformat("2024-01-02") + assert last_mod_dt == datetime.fromisoformat("2024-01-01") + + def test_both_missing_raises(self): + from db.models import GenomeAnnotation + + with pytest.raises(ValueError, match="Missing release_date and last_modified"): + GenomeAnnotation.resolve_source_dates("", "") + + def test_parse_iso_date_rejects_empty(self): + from db.models import GenomeAnnotation + + with pytest.raises(ValueError): + GenomeAnnotation.parse_iso_date("") + assert GenomeAnnotation.try_parse_iso_date("") is None + assert GenomeAnnotation.try_parse_iso_date(None) is None + + +class TestAnnotationToProcessDates: + def _base_kwargs(self, **overrides): + data = dict( + source_database="GenBank", + annotation_provider="", + release_date="2024-01-02", + last_modified_date="2024-01-01", + md5_checksum="abc123def456abc123def456abc123de", + access_url="https://example.com/a.gff.gz", + taxon_id="9606", + organism_name="Homo sapiens", + assembly_accession="GCA_000001405.29", + assembly_name="GRCh38", + ) + data.update(overrides) + return data + + def test_to_genome_annotation_backfills_empty_release_date(self): + from datetime import datetime + from db.embedded_documents import IndexedFileInfo, FeatureOverview, GFFStats, PipelineInfo + + ann = AnnotationToProcess(**self._base_kwargs(release_date="")) + ga = ann.to_genome_annotation( + annotation_id="contentmd5contentmd5contentmd5co", + taxon_lineage=["9606"], + indexed_file_info=IndexedFileInfo( + uncompressed_md5="contentmd5contentmd5contentmd5co", + bgzipped_path="/x.gff.gz", + csi_path="/x.gff.gz.csi", + file_size=1, + pipeline=PipelineInfo(name="t", version="1", method="m"), + ), + features_summary=FeatureOverview(sources=["s"], types=["gene"]), + features_statistics=GFFStats(), + ) + expected = datetime.fromisoformat("2024-01-01") + assert ga.source_file_info.release_date == expected + assert ga.source_file_info.last_modified == expected + + def test_to_genome_annotation_raises_when_both_dates_missing(self): + ann = AnnotationToProcess( + **self._base_kwargs(release_date="", last_modified_date="") + ) + with pytest.raises(ValueError, match="Missing release_date and last_modified"): + ann.to_genome_annotation(annotation_id="x", taxon_lineage=["9606"]) + + def test_to_annotation_error_uses_utcnow_when_both_dates_missing(self): + from datetime import datetime + + ann = AnnotationToProcess( + **self._base_kwargs(release_date="", last_modified_date="") + ) + err = ann.to_annotation_error("tabix failed") + assert isinstance(err.release_date, datetime) + assert isinstance(err.last_modified, datetime) + assert err.error_message == "tabix failed" + + +class TestHandleAnnotationError: + def test_does_not_raise_when_save_fails(self): + real = AnnotationToProcess( + source_database="GenBank", + release_date="", + last_modified_date="", + md5_checksum="md5x", + access_url="https://example.com/a.gff.gz", + taxon_id="1", + organism_name="x", + assembly_accession="GCA_1", + ) + err_doc = MagicMock() + err_doc.save.side_effect = Exception("cannot parse date") + + with ( + patch.object(ann_svc, "AnnotationError") as AE, + patch.object(real, "to_annotation_error", return_value=err_doc), + ): + AE.objects.return_value.first.side_effect = [None, None] + ann_svc.handle_annotation_error(real, "boom") + + err_doc.save.assert_called_once() + + def test_updates_existing_error_without_raising(self): + existing = MagicMock() + real = AnnotationToProcess( + source_database="GenBank", + release_date="2024-01-01", + last_modified_date="2024-01-01", + md5_checksum="md5x", + access_url="https://example.com/a.gff.gz", + taxon_id="1", + organism_name="x", + assembly_accession="GCA_1", + ) + with patch.object(ann_svc, "AnnotationError") as AE: + AE.objects.return_value.first.return_value = existing + ann_svc.handle_annotation_error(real, "new error") + + assert existing.error_message == "new error" + existing.save.assert_called_once()