diff --git a/divref/divref/duckdb_index.py b/divref/divref/duckdb_index.py index 4a3f916..bf16c3c 100644 --- a/divref/divref/duckdb_index.py +++ b/divref/divref/duckdb_index.py @@ -62,8 +62,12 @@ def read_and_validate_pops_legends(table_pairs: list[TablePair]) -> tuple[list[s A tuple of `(gnomad_pops_legend, hgdp_pops_legend)`. Raises: - ValueError: If any contig's gnomAD or HGDP legend disagrees with the bootstrapped legend. + ValueError: If `table_pairs` is empty, or if any contig's gnomAD or HGDP legend disagrees + with the bootstrapped legend. """ + if not table_pairs: + raise ValueError("read_and_validate_pops_legends requires at least one table pair.") + first_with_hap: TablePair | None = next( (tp for tp in table_pairs if tp.haplotype_table_path is not None), None ) @@ -134,17 +138,25 @@ def write_metadata_tables( joint_pops_legend: Joint population codes stored as JSON in `joint_pops_legend`. version: Version identifier stored in the `VERSION` table. """ - conn.execute("CREATE TABLE window_size AS SELECT ? AS window_size", [window_size]) - conn.execute( - "CREATE TABLE hgdp_haplotype_pops_legend AS SELECT ? AS pops_legend", - [json.dumps(hgdp_pops_legend)], - ) - conn.execute( - "CREATE TABLE gnomad_variant_pops_legend AS SELECT ? AS pops_legend", - [json.dumps(gnomad_pops_legend)], - ) - conn.execute( - "CREATE TABLE joint_pops_legend AS SELECT ? AS pops_legend", - [json.dumps(joint_pops_legend)], - ) - conn.execute("CREATE TABLE VERSION AS SELECT ? AS version", [version]) + # Write the five tables in one transaction so an interrupted init leaves no partially + # populated index (which a later append/finalize would then read as corrupt metadata). + conn.execute("BEGIN TRANSACTION") + try: + conn.execute("CREATE TABLE window_size AS SELECT ? AS window_size", [window_size]) + conn.execute( + "CREATE TABLE hgdp_haplotype_pops_legend AS SELECT ? AS pops_legend", + [json.dumps(hgdp_pops_legend)], + ) + conn.execute( + "CREATE TABLE gnomad_variant_pops_legend AS SELECT ? AS pops_legend", + [json.dumps(gnomad_pops_legend)], + ) + conn.execute( + "CREATE TABLE joint_pops_legend AS SELECT ? AS pops_legend", + [json.dumps(joint_pops_legend)], + ) + conn.execute("CREATE TABLE VERSION AS SELECT ? AS version", [version]) + conn.execute("COMMIT") + except BaseException: + conn.execute("ROLLBACK") + raise diff --git a/divref/divref/hail.py b/divref/divref/hail.py index 009978f..e5aa026 100644 --- a/divref/divref/hail.py +++ b/divref/divref/hail.py @@ -5,6 +5,28 @@ import pyspark +def _export_gcs_credentials(gcs_credentials_path: Path | None) -> None: + """ + Export GOOGLE_APPLICATION_CREDENTIALS for the JVM subprocess (GCS mode only). + + Args: + gcs_credentials_path: Path to the ADC JSON file; must be provided and must exist. + + Raises: + ValueError: If `gcs_credentials_path` is None. + FileNotFoundError: If the file does not exist. + """ + if gcs_credentials_path is None: + raise ValueError("gcs_credentials_path is required when use_s3 is False.") + if not gcs_credentials_path.exists(): + raise FileNotFoundError( + f"GCS credentials file not found at {gcs_credentials_path}. Run " + "`gcloud auth application-default login` or pass a valid --gcs-credentials-path." + ) + if "GOOGLE_APPLICATION_CREDENTIALS" not in os.environ: + os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = str(gcs_credentials_path) + + def hail_init( gcs_credentials_path: Path | None = None, spark_driver_memory_gb: int = 1, @@ -25,9 +47,9 @@ def hail_init( Args: gcs_credentials_path: Absolute path to a GCP Application Default Credentials - JSON file. Required when `use_s3` is `False`; ignored otherwise. If the - file exists and `GOOGLE_APPLICATION_CREDENTIALS` is not already set, it - is exported to the environment before Hail starts. + JSON file. Required (and must exist) when `use_s3` is `False`; ignored + otherwise. When `GOOGLE_APPLICATION_CREDENTIALS` is not already set, it is + exported to the environment before Hail starts. spark_driver_memory_gb: Memory in GB to allocate to the Spark driver. spark_executor_memory_gb: Memory in GB to allocate to the Spark executor. use_s3: If `True`, validate and load the S3A connector JARs and configure @@ -38,9 +60,9 @@ def hail_init( ValueError: If `spark_driver_memory_gb` or `spark_executor_memory_gb` is less than 1, or if `use_s3` is `False` and `gcs_credentials_path` is `None`. - FileNotFoundError: If `use_s3` is `False` and the GCS connector JAR is - missing, or if `use_s3` is `True` and either S3A connector JAR is - missing. + FileNotFoundError: If `use_s3` is `False` and either the credentials file at + `gcs_credentials_path` or the GCS connector JAR is missing, or if `use_s3` + is `True` and either S3A connector JAR is missing. """ if spark_driver_memory_gb < 1: raise ValueError( @@ -58,10 +80,7 @@ def hail_init( ) if not use_s3: - if gcs_credentials_path is None: - raise ValueError("gcs_credentials_path is required when use_s3 is False.") - if gcs_credentials_path.exists() and "GOOGLE_APPLICATION_CREDENTIALS" not in os.environ: - os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = str(gcs_credentials_path) + _export_gcs_credentials(gcs_credentials_path) jars_dir = os.path.join(pyspark.__path__[0], "jars") cloud_jars: list[str] = [] diff --git a/divref/divref/tools/finalize_duckdb_index.py b/divref/divref/tools/finalize_duckdb_index.py index bb9483e..9ba4a78 100644 --- a/divref/divref/tools/finalize_duckdb_index.py +++ b/divref/divref/tools/finalize_duckdb_index.py @@ -37,6 +37,12 @@ def finalize_duckdb_index( f"DuckDB index {out_duckdb_file} has no 'sequences' table; " f"run append_contig_to_duckdb_index before finalizing." ) + row_count_row = conn.execute("SELECT COUNT(*) FROM sequences").fetchone() + if row_count_row is not None and row_count_row[0] == 0: + logger.warning( + f"DuckDB index {out_duckdb_file} has an empty 'sequences' table; " + f"finalizing an index with no rows." + ) # IF NOT EXISTS keeps finalize idempotent: a re-triggered Snakemake rule or a manual retry # over an already-finalized index is a no-op rather than a raw DuckDB "index already exists" # catalog error. diff --git a/divref/tests/test_hail.py b/divref/tests/test_hail.py new file mode 100644 index 0000000..879425c --- /dev/null +++ b/divref/tests/test_hail.py @@ -0,0 +1,19 @@ +"""Tests for divref.hail.hail_init input validation (no Hail context required).""" + +from pathlib import Path + +import pytest + +from divref.hail import hail_init + + +def test_hail_init_missing_credentials_file_raises(tmp_path: Path) -> None: + """GCS mode with a provided-but-missing credentials file fails loudly before starting Hail.""" + with pytest.raises(FileNotFoundError, match="credentials file not found"): + hail_init(gcs_credentials_path=tmp_path / "does_not_exist.json", use_s3=False) + + +def test_hail_init_requires_credentials_for_gcs() -> None: + """GCS mode with no credentials path raises ValueError.""" + with pytest.raises(ValueError, match="gcs_credentials_path is required"): + hail_init(gcs_credentials_path=None, use_s3=False) diff --git a/divref/tests/tools/test_finalize_duckdb_index.py b/divref/tests/tools/test_finalize_duckdb_index.py index 16e1fd4..df4e1e2 100644 --- a/divref/tests/tools/test_finalize_duckdb_index.py +++ b/divref/tests/tools/test_finalize_duckdb_index.py @@ -1,5 +1,6 @@ """Tests for the finalize_duckdb_index tool.""" +import logging from pathlib import Path import duckdb @@ -10,6 +11,21 @@ from divref.tools.init_duckdb_index import init_duckdb_index +def test_finalize_warns_on_empty_sequences( + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """Finalizing an index whose `sequences` table has no rows logs a warning (pure DuckDB).""" + output_base = tmp_path / "idx" + with duckdb.connect(str(Path(f"{output_base}.haplotypes_gnomad_merge.index.duckdb"))) as conn: + conn.execute("CREATE TABLE sequences (sequence_id VARCHAR)") + + with caplog.at_level(logging.WARNING): + finalize_duckdb_index(output_base=output_base) + + assert "empty 'sequences' table" in caplog.text + + def _write_table_pairs_tsv(path: Path, rows: list[tuple[str, str, str]]) -> Path: """Write a table-pairs TSV with a (contig, haplotype_table_path, sites_table_path) per row.""" lines = ["contig\thaplotype_table_path\tsites_table_path"] diff --git a/divref/tests/tools/test_init_duckdb_index.py b/divref/tests/tools/test_init_duckdb_index.py index 90de15d..308f7e6 100644 --- a/divref/tests/tools/test_init_duckdb_index.py +++ b/divref/tests/tools/test_init_duckdb_index.py @@ -6,11 +6,18 @@ import pytest from divref.duckdb_index import compute_joint_legend +from divref.duckdb_index import read_and_validate_pops_legends from divref.tools.append_contig_to_duckdb_index import read_legend from divref.tools.append_contig_to_duckdb_index import read_window_size from divref.tools.init_duckdb_index import init_duckdb_index +def test_read_and_validate_pops_legends_empty_raises() -> None: + """An empty table-pair list is rejected with a clear error rather than an IndexError.""" + with pytest.raises(ValueError, match="at least one table pair"): + read_and_validate_pops_legends([]) + + def _write_table_pairs_tsv(path: Path, rows: list[tuple[str, str, str]]) -> Path: """Write a table-pairs TSV with a (contig, haplotype_table_path, sites_table_path) per row.""" lines = ["contig\thaplotype_table_path\tsites_table_path"]