From fb3192b3a76dd5f38f83829b610e13e2f82c556b Mon Sep 17 00:00:00 2001 From: ialarmedalien Date: Thu, 22 Oct 2026 09:18:05 -0700 Subject: [PATCH 1/4] Refactoring repeated code in cdm_data_loaders.readers.* --- src/cdm_data_loaders/core/fields.py | 2 + .../pipelines/all_the_bacteria.py | 8 +- src/cdm_data_loaders/readers/jsonlines.py | 58 +++---- .../jsonschema_xsv/xsv_validator/helpers.py | 8 +- .../jsonschema_xsv/xsv_validator/qsv.py | 34 ++-- .../xsv_validator/schema_utils.py | 15 +- src/cdm_data_loaders/readers/xml.py | 116 ++++++++----- src/cdm_data_loaders/readers/xml2db_doc.py | 26 +-- src/cdm_data_loaders/readers/xsd.py | 157 ++++++++++-------- src/cdm_data_loaders/utils/xml_utils.py | 12 +- .../readers/test_readers_jsonlines.py | 52 ++++++ .../test_xml_process_xml_file_and_batches.py | 77 +++++++++ .../cdm_data_loaders/utils/test_xml_utils.py | 24 ++- 13 files changed, 382 insertions(+), 207 deletions(-) diff --git a/src/cdm_data_loaders/core/fields.py b/src/cdm_data_loaders/core/fields.py index 572f9c38..25238225 100644 --- a/src/cdm_data_loaders/core/fields.py +++ b/src/cdm_data_loaders/core/fields.py @@ -69,7 +69,9 @@ class LoaderFileFormatEnum(StrEnum): } ) + NonEmptyStr = Annotated[str, StringConstraints(min_length=1)] +CharStr = Annotated[str, StringConstraints(min_length=1, max_length=1)] BatchSize = Annotated[ PositiveInt, diff --git a/src/cdm_data_loaders/pipelines/all_the_bacteria.py b/src/cdm_data_loaders/pipelines/all_the_bacteria.py index 935eaddd..4890d4be 100644 --- a/src/cdm_data_loaders/pipelines/all_the_bacteria.py +++ b/src/cdm_data_loaders/pipelines/all_the_bacteria.py @@ -16,16 +16,16 @@ from functools import cached_property from logging import Logger, getLogger from pathlib import Path -from typing import Annotated, Any, Final, Self +from typing import Any, Final, Self import dlt -import dlt.sources.helpers.requests from dlt.common.pipeline import LoadInfo from dlt.extract.items import DataItemWithMeta from dlt.sources.helpers.rest_client.client import RESTClient -from pydantic import Field, StringConstraints, computed_field, model_validator +from pydantic import Field, computed_field, model_validator from pydantic_settings import SettingsConfigDict +from cdm_data_loaders.core.fields import NonEmptyStr from cdm_data_loaders.core.settings import ( CLI_SHORTCUTS, CtsSettings, @@ -39,8 +39,6 @@ logger: Logger = getLogger(__name__) -NonEmptyStr = Annotated[str, StringConstraints(min_length=1)] - DATASET_NAME = "all_the_bacteria" ALL_FILES_TSV_FILE_ID = "R6gcp" diff --git a/src/cdm_data_loaders/readers/jsonlines.py b/src/cdm_data_loaders/readers/jsonlines.py index e261a935..4d0916c9 100644 --- a/src/cdm_data_loaders/readers/jsonlines.py +++ b/src/cdm_data_loaders/readers/jsonlines.py @@ -38,6 +38,20 @@ def parse_jsonl_line(stripped: str) -> LineRecord: return {"record": record, "raw_record": stripped, "parse_error": parse_error} +def _iter_file_jsonl_records(file_item: FileItemDict) -> Generator[LineRecord]: + """Open ``file_item`` and yield parsed ``LineRecord`` dicts for each non-blank line.""" + with file_item.open(mode="rt", compression=compression_mode(file_item["file_name"]), encoding="utf-8") as fh: + for line_no, line in enumerate(fh, start=1): + stripped = line.strip() + if not stripped: + continue + yield { + **parse_jsonl_line(stripped), + "source_file": file_item["relative_path"], + "line_no": line_no, + } + + def stream_jsonl_lines(items: Iterator[FileItemDict], buffer_size: int) -> Generator[TDataItems, Any, Any]: """Read each file in items. Yield pages of line records. @@ -56,21 +70,11 @@ def stream_jsonl_lines(items: Iterator[FileItemDict], buffer_size: int) -> Gener """ page: list[LineRecord] = [] for file_item in items: - with file_item.open(mode="rt", compression=compression_mode(file_item["file_name"]), encoding="utf-8") as fh: - for line_no, line in enumerate(fh, start=1): - stripped = line.strip() - if not stripped: - continue - page.append( - { - **parse_jsonl_line(stripped), - "source_file": file_item["relative_path"], - "line_no": line_no, - } - ) - if len(page) >= buffer_size: - yield page - page = [] + for line_record in _iter_file_jsonl_records(file_item): + page.append(line_record) + if len(page) >= buffer_size: + yield page + page = [] if page: yield page @@ -98,23 +102,9 @@ def route_jsonl_lines( for file_item in items: buf = DictBuffer(max_items=buffer_size) table_name = table_name_of(file_item) - with file_item.open(mode="rt", compression=compression_mode(file_item["file_name"]), encoding="utf-8") as fh: - for line_no, line in enumerate(fh, start=1): - stripped = line.strip() - if not stripped: - continue - line_record = parse_jsonl_line(stripped) - if line_record["parse_error"] is None: - yield from buf.add_item(table_name, line_record["record"]) - else: - yield from buf.add_item( - f"{table_name}_invalid", - { - "raw_record": stripped, - "record": None, - "source_file": file_item["relative_path"], - "line_no": line_no, - "parse_error": line_record["parse_error"], - }, - ) + for line_record in _iter_file_jsonl_records(file_item): + if line_record["parse_error"] is None: + yield from buf.add_item(table_name, line_record["record"]) + else: + yield from buf.add_item(f"{table_name}_invalid", line_record) yield from buf.flush() diff --git a/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/helpers.py b/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/helpers.py index d839a49d..33ea6f7d 100644 --- a/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/helpers.py +++ b/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/helpers.py @@ -6,23 +6,19 @@ from functools import cached_property from pathlib import Path from shutil import copy, move -from typing import Annotated, Final, Self +from typing import Final, Self from pydantic import ( BaseModel, DirectoryPath, Field, FilePath, - StringConstraints, computed_field, ) +from cdm_data_loaders.core.fields import CharStr, NonEmptyStr from cdm_data_loaders.readers.jsonschema_xsv.xsv_validator.schema_utils import ValidatedSchema -NonEmptyStr = Annotated[str, StringConstraints(min_length=1)] -CharStr = Annotated[str, StringConstraints(min_length=1, max_length=1)] - - VALIDATION_ERRORS: Final[str] = ".validation-errors.tsv" SEP_TO_EXT: Final[dict[str, str]] = {"\t": ".tsv", ",": ".csv", ";": ".ssv"} diff --git a/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/qsv.py b/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/qsv.py index 69490cea..a68cb652 100644 --- a/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/qsv.py +++ b/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/qsv.py @@ -4,6 +4,7 @@ import os import shutil import subprocess +from pathlib import Path from pydantic import validate_call @@ -127,6 +128,16 @@ def _run_qsv_step(cmd: list[str], args: "CleanerValidatorArgs") -> subprocess.Co return None +def _require_input_file(args: CleanerValidatorArgs, input_file_name: str) -> Path | None: + """Return ``args.tmp_dir_path / input_file_name`` if it exists, or record a ``FileNotFoundError`` and return ``None``.""" + input_path = args.tmp_dir_path / input_file_name + if not input_path.is_file(): + err_msg = f"Input file not found at {input_path!s}" + args.errors.append(ErrorRecord.from_exception(args.file_name, FileNotFoundError(err_msg))) + return None + return input_path + + def run_qsv_input(args: CleanerValidatorArgs, input_file_name: str) -> str | None: """Run qsv input to normalise the file input_file_name. @@ -141,9 +152,8 @@ def run_qsv_input(args: CleanerValidatorArgs, input_file_name: str) -> str | Non :return: name of the output file if all went ok; otherwise None :rtype: str | None """ - if not (args.tmp_dir_path / input_file_name).is_file(): - err_msg = f"Input file not found at {(args.tmp_dir_path / input_file_name)!s}" - args.errors.append(ErrorRecord.from_exception(args.file_name, FileNotFoundError(err_msg))) + input_path = _require_input_file(args, input_file_name) + if input_path is None: return None output_file_name = f"{args.xsv_file_base_name}{CLEANED_SUFFIX}{args.ext}" @@ -166,7 +176,7 @@ def run_qsv_input(args: CleanerValidatorArgs, input_file_name: str) -> str | Non "--output", str(output_path), # input file - str(args.tmp_dir_path / input_file_name), + str(input_path), ] result = _run_qsv_step(cmd, args) if result is None: @@ -191,9 +201,8 @@ def run_qsv_null_replacement(args: CleanerValidatorArgs, input_file_name: str) - :return: name of the output file if all went ok; otherwise None :rtype: str | None """ - if not (args.tmp_dir_path / input_file_name).is_file(): - err_msg = f"Input file not found at {(args.tmp_dir_path / input_file_name)!s}" - args.errors.append(ErrorRecord.from_exception(args.file_name, FileNotFoundError(err_msg))) + input_path = _require_input_file(args, input_file_name) + if input_path is None: return None output_file_name = f"{args.xsv_file_base_name}{NORM_SUFFIX}{args.ext}" @@ -214,7 +223,7 @@ def run_qsv_null_replacement(args: CleanerValidatorArgs, input_file_name: str) - # replacement "", # input file - str(args.tmp_dir_path / input_file_name), + str(input_path), ] result = _run_qsv_step(cmd, args) if result is None: @@ -226,7 +235,7 @@ def run_qsv_null_replacement(args: CleanerValidatorArgs, input_file_name: str) - # note: the output file may still exist, but it is ignored args.errors.append(ErrorRecord.from_qsv_result(args.file_name, result)) # copy the input file to the output directory for later perusal - copy_safely(args.tmp_dir_path / input_file_name, args.qsv_output_dir_path / input_file_name, args) + copy_safely(input_path, args.qsv_output_dir_path / input_file_name, args) return None @@ -249,9 +258,8 @@ def run_qsv_validate( :return: name of the output file if all went ok; otherwise None :rtype: str | None """ - if not (args.tmp_dir_path / input_file_name).is_file(): - err_msg = f"Input file not found at {(args.tmp_dir_path / input_file_name)!s}" - args.errors.append(ErrorRecord.from_exception(args.file_name, FileNotFoundError(err_msg))) + input_path = _require_input_file(args, input_file_name) + if input_path is None: return None # generate the full paths for the derived files @@ -278,7 +286,7 @@ def run_qsv_validate( "--valid-output", str(files.valid_output), # file to validate - str(args.tmp_dir_path / input_file_name), + str(input_path), # the schema str(files.schema), ] diff --git a/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/schema_utils.py b/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/schema_utils.py index 99f23aa9..1f7af885 100644 --- a/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/schema_utils.py +++ b/src/cdm_data_loaders/readers/jsonschema_xsv/xsv_validator/schema_utils.py @@ -3,7 +3,7 @@ import json from logging import Logger, getLogger from pathlib import Path -from typing import Annotated, Any +from typing import Any import jsonschema import jsonschema.exceptions @@ -12,22 +12,18 @@ from pydantic import ( BaseModel, DirectoryPath, - StringConstraints, computed_field, field_validator, validate_call, ) -from referencing import Registry, Resource +from cdm_data_loaders.core.fields import CharStr, NonEmptyStr from cdm_data_loaders.readers.jsonschema_xsv.xsv_validator.custom_metaschema import ( CUSTOM_META_SCHEMA, CUSTOM_META_SCHEMA_URI, X_XSV_CONFIG_KEY, ) -NonEmptyStr = Annotated[str, StringConstraints(min_length=1)] -CharStr = Annotated[str, StringConstraints(min_length=1, max_length=1)] - logger: Logger = getLogger(__name__) @@ -90,12 +86,6 @@ def validate_jsonschema(cls, schema: dict[str, Any]) -> dict[str, Any]: def register_xsv_validator() -> None: """Register the custom validator for validating JSON Schema with embedded XSV parsing metadata.""" - # register the new meta-schema and build a validator class for it - Registry().with_resource( - CUSTOM_META_SCHEMA_URI, - Resource.from_contents(CUSTOM_META_SCHEMA), - ) - # extend the Draft202012Validator metaschema to include the extra XSV parsing info # the metaschema only affects validation of schemas, not of data conforming to the schema xsv_validator = validators.extend(Draft202012Validator) @@ -116,7 +106,6 @@ def validate_jsonschema(schema_path: Path) -> ValidatedSchema: :rtype: dict[str, Any] """ schema = json.loads(schema_path.read_bytes()) - register_xsv_validator() return ValidatedSchema(jsonschema=schema) diff --git a/src/cdm_data_loaders/readers/xml.py b/src/cdm_data_loaders/readers/xml.py index 05683f62..27efee3f 100644 --- a/src/cdm_data_loaders/readers/xml.py +++ b/src/cdm_data_loaders/readers/xml.py @@ -109,6 +109,47 @@ def _log_interval_final(n_entries: int, file_path: Path, log_interval: int) -> N logger.debug("Processed %d entries from %s", n_entries + 1, file_path.name) +def _resolve_stream_tag(file_path: Path, xml_tag: str) -> str: + """Resolve an unprefixed XML tag against the file's default namespace if present.""" + if not xml_tag.startswith("{"): + default_ns = parse_head_matter(file_path).get("") + if default_ns: + return f"{{{default_ns}}}{xml_tag}" + return xml_tag + + +def _parse_element_to_dict(element: Element) -> dict[str, Any] | None: + """Parse an XML element with ``xmltodict`` and strip root-level ``_xmlns`` declarations.""" + parsed_element = xmltodict.parse(tostring(element), **DEFAULT_XMLTODICT_ARGS) + if not parsed_element: + return None + for k, v in parsed_element.items(): + # remove the xmlns declarations + parsed_element[k] = {kv: val for kv, val in v.items() if not kv.startswith("_xmlns")} + return parsed_element + + +def _stream_and_buffer_xml_elements( # noqa: PLR0913 + file_path: Path, + xml_tag: str, + log_interval: int, + buffer: ArrowListBuffer | DictBuffer | ListBuffer, + buffer_element_fn: Callable[[Element], Iterable[DataItemWithMeta]], + *, + resolve_default_ns: bool = False, +) -> Generator[DataItemWithMeta]: + """Stream matching XML elements from ``file_path``, buffer parsed items, log progress, and flush.""" + logger.info("Reading from %s", str(file_path)) + stream_tag = _resolve_stream_tag(file_path, xml_tag) if resolve_default_ns else xml_tag + n_entries = -1 + for n_entries, element in enumerate(stream_xml_file(file_path, stream_tag)): + yield from buffer_element_fn(element) + _log_interval_progress(n_entries, log_interval) + + _log_interval_final(n_entries, file_path, log_interval) + yield from buffer.flush() + + def process_xml_file_to_pyarrow(settings: XmlToPyArrowSettings, file_path: Path) -> Generator[DataItemWithMeta]: """Convert XML elements to PyArrow table batches using an XSD schema. @@ -124,32 +165,25 @@ def process_xml_file_to_pyarrow(settings: XmlToPyArrowSettings, file_path: Path) :yield: pages of PyArrow tables :rtype: Generator[DataItemWithMeta] """ - logger.info("Reading from %s", str(file_path)) - n_entries = -1 - - stream_tag = settings.xml_tag - if not stream_tag.startswith("{"): - default_ns = parse_head_matter(file_path).get("") - if default_ns: - stream_tag = f"{{{default_ns}}}{stream_tag}" - arrow_buffer = ArrowListBuffer( table_to_schema_map={settings.table_name: settings.pyarrow_schema}, max_items=settings.buffer_size, ) - for n_entries, element in enumerate(stream_xml_file(file_path, stream_tag)): - parsed_row = convert_xml_element( - element, - settings.xsd_schema, - settings.xml_tag, - max_recursion_depth=settings.max_recursion_depth, - ) - yield from arrow_buffer.add_item(parsed_row) - _log_interval_progress(n_entries, settings.log_interval) - - _log_interval_final(n_entries, file_path, settings.log_interval) - yield from arrow_buffer.flush() - return + yield from _stream_and_buffer_xml_elements( + file_path=file_path, + xml_tag=settings.xml_tag, + log_interval=settings.log_interval, + buffer=arrow_buffer, + buffer_element_fn=lambda element: arrow_buffer.add_item( + convert_xml_element( + element, + settings.xsd_schema, + settings.xml_tag, + max_recursion_depth=settings.max_recursion_depth, + ) + ), + resolve_default_ns=True, + ) def process_xml_file_to_dict(settings: XmlToDictSettings, file_path: Path) -> Generator[DataItemWithMeta]: @@ -166,21 +200,20 @@ def process_xml_file_to_dict(settings: XmlToDictSettings, file_path: Path) -> Ge :yield: pages of dictionary items :rtype: Generator[DataItemWithMeta] """ - logger.info("Reading from %s", str(file_path)) - n_entries = -1 - buffer = ListBuffer(table_name=settings.table_name, max_items=settings.buffer_size) - for n_entries, element in enumerate(stream_xml_file(file_path, settings.xml_tag)): - parsed_element = xmltodict.parse(tostring(element), **DEFAULT_XMLTODICT_ARGS) + + def _buffer_element(element: Element) -> Generator[DataItemWithMeta]: + parsed_element = _parse_element_to_dict(element) if parsed_element: - for k, v in parsed_element.items(): - # remove the xmlns declarations - parsed_element[k] = {kv: val for kv, val in v.items() if not kv.startswith("_xmlns")} yield from buffer.add_item(parsed_element) - _log_interval_progress(n_entries, settings.log_interval) - _log_interval_final(n_entries, file_path, settings.log_interval) - yield from buffer.flush() + yield from _stream_and_buffer_xml_elements( + file_path=file_path, + xml_tag=settings.xml_tag, + log_interval=settings.log_interval, + buffer=buffer, + buffer_element_fn=_buffer_element, + ) def process_xml_file( @@ -205,17 +238,14 @@ def process_xml_file( :yield: table-tagged rows :rtype: Generator[DataItemWithMeta, Any] """ - logger.info("Reading from %s", str(file_path)) - n_entries = -1 buffer = DictBuffer(max_items=settings.buffer_size) - for n_entries, element in enumerate(stream_xml_file(file_path, xml_tag)): - parsed_element = parse_fn(entry=element, file_path=file_path) - yield from buffer.add_items(parsed_element) - - _log_interval_progress(n_entries, settings.log_interval) - - _log_interval_final(n_entries, file_path, settings.log_interval) - yield from buffer.flush() + yield from _stream_and_buffer_xml_elements( + file_path=file_path, + xml_tag=xml_tag, + log_interval=settings.log_interval, + buffer=buffer, + buffer_element_fn=lambda element: buffer.add_items(parse_fn(entry=element, file_path=file_path)), + ) def process_xml_file_batches( diff --git a/src/cdm_data_loaders/readers/xml2db_doc.py b/src/cdm_data_loaders/readers/xml2db_doc.py index a7e00fa9..cfba0cf0 100644 --- a/src/cdm_data_loaders/readers/xml2db_doc.py +++ b/src/cdm_data_loaders/readers/xml2db_doc.py @@ -6,7 +6,6 @@ parsed `Document` into table rows with stable primary/foreign keys. """ -import gzip from collections.abc import Generator, Mapping from io import BytesIO from logging import Logger, getLogger @@ -19,6 +18,7 @@ from cdm_data_loaders.pipelines.xml2db.settings import Xml2DbSettings from cdm_data_loaders.utils.buffer import DictBuffer from cdm_data_loaders.utils.gz import open_maybe_gzip +from cdm_data_loaders.utils.xml_utils import local_name as _local_name logger: Logger = getLogger(__name__) @@ -272,21 +272,6 @@ def _rewrite_xml2db_junction_record( return row -def _local_name(tag: str) -> str: - """Return the local (namespace-stripped) part of an lxml expanded tag name. - - xml2db matches elements against its data model by local name only (see - `XMLConverter._parse_iterative`). So callers of `iter_xml2db_chunk_fragments` may supply - either a bare local name or a full Clark-notation expanded name. - - :param tag: an XML tag, optionally in `{namespace}local` Clark notation. - :type tag: str - :return: the local part of `tag`, with any namespace stripped. - :rtype: str - """ - return tag.rsplit("}", 1)[-1] if isinstance(tag, str) and tag.startswith("{") else tag - - def iter_xml2db_chunk_fragments( file_path: str | Path, child_tag: str, @@ -475,14 +460,9 @@ def process_xml_file_with_xml2db( return # parse it all in one go - file_path_str = str(file_path) document = Document(model) - if file_path_str.endswith(".gz"): - with gzip.open(file_path, "rb") as file_handle: - xml_source: str | BytesIO = BytesIO(file_handle.read()) - else: - xml_source = file_path_str - document.parse_xml(xml_source, skip_validation=settings.skip_xml_validation, iterparse=True) + with open_maybe_gzip(file_path) as file_handle: + document.parse_xml(BytesIO(file_handle.read()), skip_validation=settings.skip_xml_validation, iterparse=True) tables = flatten_xml2db_document(model, document, file_key=file_path.name) yield from buffer.add_items(tables) diff --git a/src/cdm_data_loaders/readers/xsd.py b/src/cdm_data_loaders/readers/xsd.py index e766fe9e..ae8311d9 100644 --- a/src/cdm_data_loaders/readers/xsd.py +++ b/src/cdm_data_loaders/readers/xsd.py @@ -6,7 +6,7 @@ from collections.abc import Mapping from dataclasses import dataclass from pathlib import Path -from typing import Any, Final +from typing import Any, Final, Literal import pyarrow as pa import xmlschema @@ -22,6 +22,8 @@ XsdType, ) +from cdm_data_loaders.utils.xml_utils import local_name as _local_name + DEFAULT_MAX_RECURSION_DEPTH: Final[int] = 2 XSD_NAMESPACE: Final[str] = "{http://www.w3.org/2001/XMLSchema}" @@ -99,13 +101,6 @@ def load_schema(schema_path: str | Path) -> xmlschema.XMLSchema: return xmlschema.XMLSchema(str(schema_path)) -def _local_name(tag: str) -> str: - """Strip namespace URI from a Clark-notation tag or prefixed attribute name.""" - if "}" in tag: - return tag.rsplit("}", maxsplit=1)[-1] - return tag - - def _extract_doc_comment(component: object, fallback_type: object = None) -> str | None: """Extract whitespace-normalized ```` text from an XSD component or its type.""" candidates = [component] @@ -230,21 +225,15 @@ def _iter_simple_builtin_names(simple_type: XsdType) -> list[str]: @dataclass(frozen=True, slots=True) -class _SchemaContext: - """Immutable traversal context for PyArrow schema generation.""" +class _TraversalContext: + """Immutable traversal context for XSD schema generation and XML-to-dict conversion.""" schema: xmlschema.XMLSchema type_counts: Mapping[int, int] - max_recursion_depth: int + max_recursion_depth: int | None -@dataclass(frozen=True, slots=True) -class _ConvertContext: - """Immutable traversal context for XML-to-dict conversion.""" - - schema: xmlschema.XMLSchema - type_counts: Mapping[int, int] - max_recursion_depth: int | None +_LeafKind = Literal["simple", "mixed", "empty_marker"] def _check_complex_type_forbidden_recursion(root_type: XsdType, current: XsdComplexType) -> list[XsdType]: @@ -430,8 +419,23 @@ def _is_empty_marker_complex_type(complex_type: XsdComplexType) -> bool: ) +def _resolve_child_cardinality( + model_group: XsdGroup, + child_decl: XsdElement, + *, + parent_nillable: bool = False, +) -> tuple[bool, bool]: + """Return ``(is_list, nullable)`` for ``child_decl`` within ``model_group``.""" + child_name = child_decl.local_name or "" + max_occurs = _group_max_occurs_by_name(model_group, child_name) + min_occurs = _group_min_occurs_by_name(model_group, child_name) + is_list = max_occurs is None or max_occurs > 1 + nullable = parent_nillable or child_decl.nillable or min_occurs < 1 + return is_list, nullable + + def _build_complex_struct_fields( - ctx: _SchemaContext, + ctx: _TraversalContext, complex_type: XsdComplexType, *, parent_nillable: bool, @@ -499,10 +503,7 @@ def _build_complex_struct_fields( ) raise ValueError(msg) - max_occurs = _group_max_occurs_by_name(model_group, child_name) - min_occurs = _group_min_occurs_by_name(model_group, child_name) - is_list = max_occurs is None or max_occurs > 1 - nullable = parent_nillable or child.nillable or min_occurs < 1 + is_list, nullable = _resolve_child_cardinality(model_group, child, parent_nillable=parent_nillable) fields.append( _build_element_field( @@ -516,27 +517,40 @@ def _build_complex_struct_fields( return fields -def _resolve_non_struct_element_type(elem_type: XsdType | None) -> tuple[pa.DataType, str | None] | None: - """Return ``(pa_type, xsd_type_or_None)`` if ``elem_type`` maps to a leaf scalar rather than a struct.""" +def _classify_leaf_element_type(elem_type: XsdType | None) -> tuple[_LeafKind, XsdType | None] | None: + """Classify ``elem_type`` as a non-struct leaf ``(kind, simple_type)`` or ``None`` if it maps to a struct.""" if not isinstance(elem_type, XsdComplexType): - return _simple_type_to_arrow(elem_type) if elem_type is not None else (pa.string(), "xs:string") + return "simple", elem_type attrs = _collect_attributes(elem_type) if elem_type.has_simple_content() and not attrs: - content_type = elem_type.content - return _simple_type_to_arrow(content_type) if isinstance(content_type, XsdType) else (pa.string(), "xs:string") + content_type = elem_type.content if isinstance(elem_type.content, XsdType) else None + return "simple", content_type if elem_type.mixed and not attrs: - return pa.string(), "xs:string" + return "mixed", None if _is_empty_marker_complex_type(elem_type): - return pa.string(), None + return "empty_marker", None return None +def _resolve_non_struct_element_type(elem_type: XsdType | None) -> tuple[pa.DataType, str | None] | None: + """Return ``(pa_type, xsd_type_or_None)`` if ``elem_type`` maps to a leaf scalar rather than a struct.""" + leaf_info = _classify_leaf_element_type(elem_type) + if leaf_info is None: + return None + kind, simple_type = leaf_info + if kind == "simple": + return _simple_type_to_arrow(simple_type) if simple_type is not None else (pa.string(), "xs:string") + if kind == "mixed": + return pa.string(), "xs:string" + return pa.string(), None + + def _build_element_field( - ctx: _SchemaContext, + ctx: _TraversalContext, element: XsdElement, *, is_list: bool, @@ -562,7 +576,7 @@ def _build_element_field( _check_no_forbidden_types_under_recursion(elem_type) next_count = ctx.type_counts.get(type_id, 0) + 1 - if next_count > ctx.max_recursion_depth: + if ctx.max_recursion_depth is not None and next_count > ctx.max_recursion_depth: return _wrap_scalar_or_list_field( element.local_name, (pa.string(), None), @@ -571,7 +585,7 @@ def _build_element_field( nullable=nullable, ) - next_ctx = _SchemaContext( + next_ctx = _TraversalContext( schema=ctx.schema, type_counts={**ctx.type_counts, type_id: next_count}, max_recursion_depth=ctx.max_recursion_depth, @@ -689,6 +703,23 @@ def _require_complex_type(elem_type: XsdType | None, xml_tag: str) -> XsdComplex return elem_type +def _prepare_target_traversal( + schema: xmlschema.XMLSchema, + xml_tag: str, + *, + max_recursion_depth: int, +) -> tuple[XsdElement, XsdComplexType, _TraversalContext]: + """Resolve ``xml_tag`` to its ``XsdElement``, validated ``XsdComplexType``, and initial ``_TraversalContext``.""" + target_elem = _resolve_target_element(schema, xml_tag) + elem_type = _require_complex_type(target_elem.type, xml_tag) + ctx = _TraversalContext( + schema=schema, + type_counts={id(elem_type): 1}, + max_recursion_depth=max_recursion_depth, + ) + return target_elem, elem_type, ctx + + def generate_pyarrow_schema( schema: xmlschema.XMLSchema, xml_tag: str, @@ -706,8 +737,11 @@ def generate_pyarrow_schema( :return: PyArrow schema whose top-level fields correspond to the target element's fields :rtype : pa.Schema """ - target_elem = _resolve_target_element(schema, xml_tag) - elem_type = _require_complex_type(target_elem.type, xml_tag) + target_elem, elem_type, ctx = _prepare_target_traversal( + schema, + xml_tag, + max_recursion_depth=max_recursion_depth, + ) if _is_empty_marker_complex_type(elem_type): msg = f"Top-level xml_tag {xml_tag!r} has an empty complex type with no attributes or child elements" @@ -718,11 +752,6 @@ def generate_pyarrow_schema( msg = f"Top-level xml_tag {xml_tag!r} must have a complex type that resolves to a struct" raise ValueError(msg) - ctx = _SchemaContext( - schema=schema, - type_counts={id(elem_type): 1}, - max_recursion_depth=max_recursion_depth, - ) fields = _build_complex_struct_fields( ctx, elem_type, @@ -833,7 +862,7 @@ def _group_child_elements_by_name(element: Element) -> dict[str, list[Element]]: def _convert_complex_struct( - ctx: _ConvertContext, + ctx: _TraversalContext, element: Element, complex_type: XsdComplexType, ) -> dict[str, Any] | None: @@ -868,10 +897,7 @@ def _convert_complex_struct( if not matching_elements: continue - max_occurs = _group_max_occurs_by_name(model_group, child_name) - min_occurs = _group_min_occurs_by_name(model_group, child_name) - is_list = max_occurs is None or max_occurs > 1 - nullable = child_decl.nillable or min_occurs < 1 + is_list, nullable = _resolve_child_cardinality(model_group, child_decl) if is_list: result[child_name] = [ @@ -902,7 +928,7 @@ def _convert_text_or_empty_leaf( def _convert_child_occurrence( - ctx: _ConvertContext, + ctx: _TraversalContext, element: Element, xsd_element: XsdElement, *, @@ -910,29 +936,27 @@ def _convert_child_occurrence( ) -> Any: # noqa: ANN401 """Convert a single XML element occurrence according to ``xsd_element``.""" elem_type = xsd_element.type + leaf_info = _classify_leaf_element_type(elem_type) + if leaf_info is not None: + kind, simple_type = leaf_info + if kind == "empty_marker": + return None if _is_xsi_nil(element) else "" + return _convert_text_or_empty_leaf( + element, + is_mixed=(kind == "mixed"), + simple_type=simple_type, + nullable=nullable, + ) - if not isinstance(elem_type, XsdComplexType): - return _convert_text_or_empty_leaf(element, is_mixed=False, simple_type=elem_type, nullable=nullable) - - attrs = _collect_attributes(elem_type) - if elem_type.has_simple_content() and not attrs: - content_type = elem_type.content if isinstance(elem_type.content, XsdType) else None - return _convert_text_or_empty_leaf(element, is_mixed=False, simple_type=content_type, nullable=nullable) - - if elem_type.mixed and not attrs: - return _convert_text_or_empty_leaf(element, is_mixed=True, simple_type=None, nullable=nullable) - - if _is_empty_marker_complex_type(elem_type): - return None if _is_xsi_nil(element) else "" - + assert isinstance(elem_type, XsdComplexType) # noqa: S101 type_id = id(elem_type) next_count = ctx.type_counts.get(type_id, 0) + 1 if ctx.max_recursion_depth is not None and next_count > ctx.max_recursion_depth: - unbounded_ctx = _ConvertContext(schema=ctx.schema, type_counts={}, max_recursion_depth=None) + unbounded_ctx = _TraversalContext(schema=ctx.schema, type_counts={}, max_recursion_depth=None) nested = _convert_complex_struct(unbounded_ctx, element, elem_type) return json.dumps(nested) if nested is not None else None - next_ctx = _ConvertContext( + next_ctx = _TraversalContext( schema=ctx.schema, type_counts={**ctx.type_counts, type_id: next_count}, max_recursion_depth=ctx.max_recursion_depth, @@ -960,12 +984,9 @@ def convert_xml_element( :return: sparse dictionary of coerced field values matching the generated PyArrow schema :rtype : dict[str, Any] """ - target_elem = _resolve_target_element(schema, xml_tag) - elem_type = _require_complex_type(target_elem.type, xml_tag) - - ctx = _ConvertContext( - schema=schema, - type_counts={id(elem_type): 1}, + _, elem_type, ctx = _prepare_target_traversal( + schema, + xml_tag, max_recursion_depth=max_recursion_depth, ) converted = _convert_complex_struct(ctx, element, elem_type) diff --git a/src/cdm_data_loaders/utils/xml_utils.py b/src/cdm_data_loaders/utils/xml_utils.py index f9ac2871..80366914 100644 --- a/src/cdm_data_loaders/utils/xml_utils.py +++ b/src/cdm_data_loaders/utils/xml_utils.py @@ -1,7 +1,8 @@ -"""Shared XML helper utilities used by UniProt and UniRef parsers. +"""Shared XML helper utilities used by XML readers and UniProt/UniRef parsers. This module centralizes common operations: - Safe text extraction +- Local (namespace-stripped) XML tag/attribute name extraction """ from lxml.etree import Element @@ -15,3 +16,12 @@ def get_text(elem: Element | None, default: str | None = None) -> str | None: return default text = elem.text.strip() return text or default + + +def local_name(tag: str) -> str: + """Return the local (namespace-stripped) part of an XML tag or attribute name. + + Handles Clark notation (``"{http://example.com}entry"`` -> ``"entry"``) and returns + already-unprefixed names unchanged. + """ + return tag.rsplit("}", 1)[-1] if isinstance(tag, str) and "}" in tag else tag diff --git a/tests/cdm_data_loaders/readers/test_readers_jsonlines.py b/tests/cdm_data_loaders/readers/test_readers_jsonlines.py index abc987ec..1254d2c0 100644 --- a/tests/cdm_data_loaders/readers/test_readers_jsonlines.py +++ b/tests/cdm_data_loaders/readers/test_readers_jsonlines.py @@ -8,6 +8,7 @@ from dlt.sources.filesystem import filesystem from cdm_data_loaders.readers.jsonlines import ( + _iter_file_jsonl_records, compression_mode, parse_jsonl_line, route_jsonl_lines, @@ -164,3 +165,54 @@ def test_route_jsonl_lines_pass_routes_valid_and_invalid_rows(tmp_path: Path) -> assert invalid[0]["line_no"] == 2 assert invalid[0]["source_file"] == "data.jsonl" assert invalid[0]["parse_error"] is not None + + +def test_route_jsonl_lines_pass_skips_blank_lines_and_flushes_at_buffer_size(tmp_path: Path) -> None: + """route_jsonl_lines skips blank lines, preserves line numbers, and flushes pages at buffer_size.""" + directory = tmp_path / "widget" + directory.mkdir() + (directory / "data.jsonl").write_text( + '{"widget_id": "a"}\n\n \n{"widget_id": "b"}\n{"widget_id": "c"}\n', + encoding="utf-8", + ) + + pages = list( + route_jsonl_lines( + iter(read_items(directory)), + buffer_size=2, + table_name_of=lambda _file_item: "widget", + ) + ) + + assert [len(page.data) for page in pages] == [2, 1] + assert [row["widget_id"] for page in pages for row in page.data] == ["a", "b", "c"] + + +def test_iter_file_jsonl_records_pass_yields_records_with_line_numbers_and_skips_blanks(tmp_path: Path) -> None: + """_iter_file_jsonl_records yields parsed and malformed records with 1-based line_no and source_file.""" + directory = tmp_path / "widget" + directory.mkdir() + (directory / "sample.jsonl").write_text('{"id": 1}\n\n{bad\n{"id": 2}\n', encoding="utf-8") + file_item = read_items(directory)[0] + + records = list(_iter_file_jsonl_records(file_item)) + + assert [r["line_no"] for r in records] == [1, 3, 4] + assert records[0] == { + "record": {"id": 1}, + "raw_record": '{"id": 1}', + "parse_error": None, + "source_file": "sample.jsonl", + "line_no": 1, + } + assert records[1]["record"] is None + assert records[1]["raw_record"] == "{bad" + assert records[1]["parse_error"] is not None + assert records[1]["source_file"] == "sample.jsonl" + assert records[2] == { + "record": {"id": 2}, + "raw_record": '{"id": 2}', + "parse_error": None, + "source_file": "sample.jsonl", + "line_no": 4, + } diff --git a/tests/cdm_data_loaders/readers/xml/test_xml_process_xml_file_and_batches.py b/tests/cdm_data_loaders/readers/xml/test_xml_process_xml_file_and_batches.py index e7caac24..fe3980c3 100644 --- a/tests/cdm_data_loaders/readers/xml/test_xml_process_xml_file_and_batches.py +++ b/tests/cdm_data_loaders/readers/xml/test_xml_process_xml_file_and_batches.py @@ -1229,3 +1229,80 @@ def _raising_process(*_: Any, **__: Any) -> Iterator[DataItemWithMeta]: # noqa: with pytest.raises(RuntimeError, match="parse failure"): list(process_xml_file_batches(fake_settings(), "tag", MagicMock())) # pyright: ignore[reportArgumentType] + + +def test_process_xml_file_to_dict_pass_skips_empty_xmltodict_result( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Verify an element whose xmltodict.parse output is empty is skipped and not buffered.""" + file_path = _write_xml(tmp_path, "two.xml", PEOPLE_XML_2) + settings = fake_xmltodict_settings(table_name="people", xml_tag="person") + original_parse = xmltodict.parse + call_count = 0 + + def _parse_skipping_first(*args: Any, **kwargs: Any) -> dict[str, Any]: # noqa: ANN401 + nonlocal call_count + call_count += 1 + if call_count == 1: + return {} + return original_parse(*args, **kwargs) + + monkeypatch.setattr(xml_module.xmltodict, "parse", _parse_skipping_first) + + items = _table_and_data(process_xml_file_to_dict(settings, file_path=file_path)) + + assert items == [("people", [PEOPLE_XML_TO_DICT_PARSED[1]])] + + +def test_process_xml_file_to_pyarrow_pass_clark_notation_tag_skips_head_matter( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Verify process_xml_file_to_pyarrow skips parse_head_matter when xml_tag is already in Clark notation.""" + namespaced_xsd = """ + + + + + + + + + + + + + + + + +""" + namespaced_xml = """ + + Anne Example + +""" + xsd_path = _write_xml(tmp_path, "people_ns.xsd", namespaced_xsd) + xml_path = _write_xml(tmp_path, "people_ns.xml", namespaced_xml) + clark_tag = "{http://example.org/ns}person" + xsd_schema = xmlschema.XMLSchema(str(xsd_path)) + pyarrow_schema = generate_pyarrow_schema(xsd_schema, clark_tag) + settings = MagicMock(spec=XmlToPyArrowSettings) + settings.table_name = "people" + settings.xml_tag = clark_tag + settings.buffer_size = DEFAULTS[BUFFER_SIZE] + settings.log_interval = DEFAULTS[LOG_INTERVAL] + settings.xsd_schema = xsd_schema + settings.pyarrow_schema = pyarrow_schema + settings.max_recursion_depth = DEFAULT_MAX_RECURSION_DEPTH + + head_matter_mock = MagicMock() + monkeypatch.setattr(xml_module, "parse_head_matter", head_matter_mock) + + items = list(process_xml_file_to_pyarrow(settings, file_path=xml_path)) + + head_matter_mock.assert_not_called() + assert len(items) == 1 + assert items[0].data.to_pylist() == [{"_id": "1", "name": "Anne Example"}] diff --git a/tests/cdm_data_loaders/utils/test_xml_utils.py b/tests/cdm_data_loaders/utils/test_xml_utils.py index c18e8c73..b8b48982 100644 --- a/tests/cdm_data_loaders/utils/test_xml_utils.py +++ b/tests/cdm_data_loaders/utils/test_xml_utils.py @@ -3,7 +3,7 @@ import pytest from lxml.etree import Element -from cdm_data_loaders.utils.xml_utils import get_text +from cdm_data_loaders.utils.xml_utils import get_text, local_name def make_element_with_text(raw_text: str | None) -> Element: @@ -36,3 +36,25 @@ def test_get_text_fail_rejects_non_element() -> None: """A non-Element input raises AttributeError.""" with pytest.raises(AttributeError): get_text(123) # type: ignore[arg-type] + + +@pytest.mark.parametrize( + ("tag", "expected"), + [ + ("{http://uniprot.org/uniprot}entry", "entry"), + ("{}entry", "entry"), + ("{http://example.org/{nested}}item", "item"), + ("entry", "entry"), + ("", ""), + ], + ids=[ + "clark_notation", + "empty_namespace_braces", + "nested_braces_in_namespace", + "unprefixed_tag", + "empty_string", + ], +) +def test_local_name_pass_extracts_local_part(tag: str, expected: str) -> None: + """local_name strips Clark-notation namespaces and preserves unprefixed names.""" + assert local_name(tag) == expected From f52536811703b865cf16648ca2995214cbe72c3c Mon Sep 17 00:00:00 2001 From: ialarmedalien Date: Thu, 22 Oct 2026 11:28:24 -0700 Subject: [PATCH 2/4] Refactor to remove reams of repeated code --- src/cdm_data_loaders/core/settings.py | 37 ++++- src/cdm_data_loaders/pipelines/core.py | 75 ++++++++++- .../pipelines/jsonlines/base.py | 36 +++-- .../extract_jsonschema_validate_pipeline.py | 8 +- .../pipelines/jsonlines/extract_pipeline.py | 20 ++- .../extract_pydantic_validate_pipeline.py | 11 +- .../pipelines/jsonlines/settings.py | 18 +-- .../pipelines/ncbi_ftp_download.py | 125 +++++------------ .../pipelines/ncbi_ftp_promote.py | 6 +- .../pipelines/pdb_manifest.py | 8 +- .../pipelines/xml2db/pipeline.py | 30 ++--- .../pipelines/xml2db/settings.py | 23 +--- .../pipelines/xmltodict/pipeline.py | 29 ++-- .../pipelines/xmltodict/settings.py | 20 +-- .../pipelines/xmltopyarrow/pipeline.py | 29 ++-- .../pipelines/xmltopyarrow/settings.py | 95 +++++-------- .../pipelines/xsv/pipeline.py | 41 +++--- .../pipelines/xsv/settings.py | 9 +- tests/cdm_data_loaders/core/test_settings.py | 82 +++++++++++- tests/cdm_data_loaders/pipelines/helpers.py | 4 +- .../integration/test_jsonschema_end_to_end.py | 25 ++-- .../integration/test_reference_end_to_end.py | 54 ++++---- .../pipelines/jsonlines/test_registry.py | 56 ++++++++ .../test_ncbi_ftp_download.py | 74 ++++++++++ .../test_ncbi_rest_api_settings.py | 30 ++++- tests/cdm_data_loaders/pipelines/test_core.py | 116 +++++++++++++++- .../pipelines/test_framework.py | 17 ++- .../uniref_cdm/test_uniref_cdm_settings.py | 20 +-- .../pipelines/xml2db/test_xml2db_settings.py | 32 +---- .../xmltodict/test_xmltodict_pipeline.py | 126 +++--------------- .../xmltodict/test_xmltodict_settings.py | 34 +---- .../test_xmltopyarrow_settings.py | 36 ++++- .../readers/xml/test_xml_parse_head_matter.py | 7 +- .../test_xml_process_xml_file_and_batches.py | 24 ++-- 34 files changed, 774 insertions(+), 583 deletions(-) diff --git a/src/cdm_data_loaders/core/settings.py b/src/cdm_data_loaders/core/settings.py index bcd8d83a..9a018a86 100644 --- a/src/cdm_data_loaders/core/settings.py +++ b/src/cdm_data_loaders/core/settings.py @@ -1,14 +1,16 @@ """Settings classes for data loading and wrangling pipelines.""" from pathlib import Path -from typing import TYPE_CHECKING, Any, Final +from typing import TYPE_CHECKING, Annotated, Any, Final from frozendict import frozendict -from pydantic import AliasChoices, AliasPath, computed_field +from pydantic import AliasChoices, AliasPath, Field, computed_field from pydantic.fields import FieldInfo from pydantic_settings import BaseSettings, SettingsConfigDict, SettingsError from cdm_data_loaders.core.fields import ( + DEFAULT_XML_FILE_GLOB, + FILE_GLOB, INPUT_DIR, LOADER_FILE_FORMAT, OUTPUT_DIR, @@ -17,6 +19,7 @@ BufferSize, DatasetName, DltDevMode, + FileGlob, LoaderFileFormat, LogConfigFile, LogInterval, @@ -47,6 +50,18 @@ USE_OUTPUT_DIR_FOR_PIPELINE_METADATA: "p", } ) +FILE_FORMAT_CLI_SHORTCUTS: frozendict[str, str] = frozendict( + { + **CLI_SHORTCUTS, + LOADER_FILE_FORMAT: "f", + } +) +XML_CLI_SHORTCUTS: frozendict[str, str] = frozendict( + { + **FILE_FORMAT_CLI_SHORTCUTS, + FILE_GLOB: "g", + } +) # argparse registers -h/--help on every parser; claiming either makes the parser fail to build RESERVED_CLI_NAMES: Final[frozenset[str]] = frozenset({"h", "help"}) @@ -210,9 +225,21 @@ def pipeline_dir(self) -> Path | None: class FileFormatIngestSettings(CtsSettings): """Configuration for DLT pipelines that import a particular file format.""" - model_config: SettingsConfigDict = default_settings_with_shortcuts( - cli_shortcuts={**CLI_SHORTCUTS, LOADER_FILE_FORMAT: "f"} - ) + model_config: SettingsConfigDict = default_settings_with_shortcuts(cli_shortcuts=FILE_FORMAT_CLI_SHORTCUTS) dataset_name: DatasetName loader_file_format: LoaderFileFormat + + +class XmlIngestSettings(FileFormatIngestSettings): + """Configuration for DLT pipelines that import XML files.""" + + model_config: SettingsConfigDict = default_settings_with_shortcuts(cli_shortcuts=XML_CLI_SHORTCUTS) + + file_glob: Annotated[ + FileGlob, + Field( + default=DEFAULT_XML_FILE_GLOB, + description="Glob pattern for XML files", + ), + ] diff --git a/src/cdm_data_loaders/pipelines/core.py b/src/cdm_data_loaders/pipelines/core.py index 6fe2344d..cb61d9f2 100644 --- a/src/cdm_data_loaders/pipelines/core.py +++ b/src/cdm_data_loaders/pipelines/core.py @@ -11,13 +11,20 @@ import dlt from dlt.common.pipeline import LoadInfo from dlt.common.runtime.slack import send_slack_message +from dlt.common.schema.typing import TSchemaContractDict from dlt.extract import DltResource from dlt.pipeline.exceptions import PipelineStepFailed +from dlt.sources.filesystem import filesystem from pydantic import ValidationError from pydantic_settings import CliApp, SettingsError -from cdm_data_loaders.core.fields import JSONL -from cdm_data_loaders.core.settings import CtsSettings, LoggerSettings +from cdm_data_loaders.core.fields import DEFAULT_XML_FILE_GLOB, JSONL +from cdm_data_loaders.core.settings import ( + CtsSettings, + FileFormatIngestSettings, + LocalInputOutputSettings, + LoggerSettings, +) from cdm_data_loaders.utils.cdm_logger import init_logger WEBHOOK_NOT_CONFIGURED: Final[str] = "Slack webhook not configured" @@ -27,6 +34,12 @@ DESTINATION_NOT_CONFIGURED: Final[str] = ( "Destination {name!r} is not configured: set destination.{name}.destination_type in the dlt config." ) +DEFAULT_SCHEMA_CONTRACT: Final[TSchemaContractDict] = { + "tables": "evolve", + "columns": "evolve", + "data_type": "discard_row", +} +DEFAULT_REJECTED_SUFFIX: Final[str] = "_rejected" logger: Logger = getLogger(__name__) @@ -300,3 +313,61 @@ def run_pipeline( send_slack_message_carefully(slack_hook, msg) return load_info + + +def build_filesystem_resource( + settings: LocalInputOutputSettings, + reader_fn: Callable[..., Any] | DltResource, + *reader_args: Any, # noqa: ANN401 + name: str, + file_glob: str | None = None, + schema_contract: TSchemaContractDict | None = None, + **transformer_kwargs: Any, # noqa: ANN401 +) -> DltResource: + """Build a dlt resource that pipes ``filesystem(settings.input_dir, file_glob)`` into a parallelized transformer. + + :param settings: pipeline settings with ``input_dir`` (and optionally ``file_glob``) + :param reader_fn: transformer function or resource accepting ``items`` followed by ``*reader_args`` + :param reader_args: positional arguments bound to the transformer after ``items`` + :param name: name of the resulting dlt transformer resource + :param file_glob: glob pattern for files; defaults to ``settings.file_glob`` if present, else ``DEFAULT_XML_FILE_GLOB`` + :param schema_contract: optional dlt schema contract hint applied to the resource + :param transformer_kwargs: additional keyword arguments passed to ``dlt.transformer`` + :return: configured ``DltResource`` + """ + resolved_glob = file_glob if file_glob is not None else getattr(settings, "file_glob", DEFAULT_XML_FILE_GLOB) + files = filesystem(bucket_url=str(settings.input_dir), file_glob=resolved_glob) + reader = ( + reader_fn + if isinstance(reader_fn, DltResource) + else dlt.transformer(reader_fn, name=name, parallelized=True, **transformer_kwargs) + ) + resource: DltResource = files | reader(*reader_args) + if schema_contract is not None: + resource.apply_hints(schema_contract=TSchemaContractDict(**schema_contract)) # pyright: ignore[reportCallIssue] + return resource + + +def build_file_ingest_kwargs( + settings: FileFormatIngestSettings, + pipeline_name: str, + *, + fallback_dataset_name: bool = False, +) -> dict[str, dict[str, Any]]: + """Build the standard ``pipeline_kwargs`` and ``pipeline_run_kwargs`` for a file-ingest pipeline. + + :param settings: file-format ingest settings with ``dataset_name`` and ``loader_file_format`` + :param pipeline_name: name of the dlt pipeline + :param fallback_dataset_name: whether to fall back to ``pipeline_name`` when ``settings.dataset_name`` is None + :return: mapping with ``pipeline_kwargs`` and ``pipeline_run_kwargs`` suitable for ``run_pipeline`` + """ + dataset_name = (settings.dataset_name or pipeline_name) if fallback_dataset_name else settings.dataset_name + return { + "pipeline_kwargs": { + "pipeline_name": pipeline_name, + "dataset_name": dataset_name, + }, + "pipeline_run_kwargs": { + "loader_file_format": str(settings.loader_file_format), + }, + } diff --git a/src/cdm_data_loaders/pipelines/jsonlines/base.py b/src/cdm_data_loaders/pipelines/jsonlines/base.py index 92888d16..0235d6e2 100644 --- a/src/cdm_data_loaders/pipelines/jsonlines/base.py +++ b/src/cdm_data_loaders/pipelines/jsonlines/base.py @@ -8,6 +8,7 @@ """ from collections.abc import Callable, Generator +from importlib import import_module from logging import Logger, getLogger from typing import TYPE_CHECKING, Any, Final @@ -16,7 +17,11 @@ from dlt.common.schema.typing import TSchemaContractDict from dlt.sources.filesystem import filesystem -from cdm_data_loaders.pipelines.core import run_pipeline +from cdm_data_loaders.pipelines.core import ( + DEFAULT_REJECTED_SUFFIX, + DEFAULT_SCHEMA_CONTRACT, + run_pipeline, +) from cdm_data_loaders.readers.jsonlines import stream_jsonl_lines from cdm_data_loaders.utils.buffer import ListBuffer @@ -25,13 +30,7 @@ logger: Logger = getLogger(__name__) -SCHEMA_CONTRACT: Final[TSchemaContractDict] = { - "tables": "evolve", - "columns": "evolve", - "data_type": "discard_row", -} - -DEFAULT_REJECTED_SUFFIX: Final[str] = "_rejected" +SCHEMA_CONTRACT: Final[TSchemaContractDict] = DEFAULT_SCHEMA_CONTRACT REJECTED_COLUMNS: Final[tuple[str, ...]] = ("source_file", "line_no", "raw_record") @@ -39,6 +38,27 @@ ResourceFactory = Callable[[str, dict[str, Any], Any], "DltResource"] +def load_registry_dict(module_path: str, attribute_name: str, type_hint: str = "") -> dict[str, Any]: + """Import ``module_path`` and return its non-empty dictionary attribute ``attribute_name``. + + :param module_path: dotted import path of the registry module + :type module_path: str + :param attribute_name: name of the dictionary attribute on the module + :type attribute_name: str + :param type_hint: optional suffix included in the ValueError message when missing or empty + :type type_hint: str + :return: the registry dictionary + :rtype: dict[str, Any] + :raises ValueError: if the attribute is missing, empty, or not a dict + """ + module = import_module(module_path) + registry = getattr(module, attribute_name, None) + if not isinstance(registry, dict) or not registry: + err_msg = f"{module_path} does not define a non-empty {attribute_name}{type_hint}" + raise ValueError(err_msg) + return registry + + def make_page_router( # noqa: PLR0913 table_name: str, buffer_size: int, diff --git a/src/cdm_data_loaders/pipelines/jsonlines/extract_jsonschema_validate_pipeline.py b/src/cdm_data_loaders/pipelines/jsonlines/extract_jsonschema_validate_pipeline.py index 6587fad6..8409a0ed 100644 --- a/src/cdm_data_loaders/pipelines/jsonlines/extract_jsonschema_validate_pipeline.py +++ b/src/cdm_data_loaders/pipelines/jsonlines/extract_jsonschema_validate_pipeline.py @@ -8,7 +8,6 @@ """ import json -from importlib import import_module from logging import Logger, getLogger from typing import Any @@ -21,6 +20,7 @@ from cdm_data_loaders.pipelines.core import PipelineCli from cdm_data_loaders.pipelines.jsonlines.base import ( ParseErrorResolver, + load_registry_dict, run_registry_ingest, ) from cdm_data_loaders.pipelines.jsonlines.base import ( @@ -44,11 +44,7 @@ def load_entity_schemas(module_path: str) -> dict[str, dict[str, Any]]: :rtype: dict[str, dict[str, Any]] :raises ValueError: if ENTITY_SCHEMAS is missing, empty, or not a dict """ - module = import_module(module_path) - registry = getattr(module, "ENTITY_SCHEMAS", None) - if not isinstance(registry, dict) or not registry: - err_msg = f"{module_path} does not define a non-empty ENTITY_SCHEMAS" - raise ValueError(err_msg) + registry = load_registry_dict(module_path, "ENTITY_SCHEMAS") errors: dict[str, str] = {} for table_name, schema in registry.items(): if not isinstance(table_name, str) or not table_name.strip(): diff --git a/src/cdm_data_loaders/pipelines/jsonlines/extract_pipeline.py b/src/cdm_data_loaders/pipelines/jsonlines/extract_pipeline.py index 0565a9f1..d8c4f886 100644 --- a/src/cdm_data_loaders/pipelines/jsonlines/extract_pipeline.py +++ b/src/cdm_data_loaders/pipelines/jsonlines/extract_pipeline.py @@ -15,9 +15,13 @@ from dlt.common.pipeline import LoadInfo from dlt.common.storages.fsspec_filesystem import FileItemDict from dlt.common.typing import TDataItems -from dlt.sources.filesystem import filesystem -from cdm_data_loaders.pipelines.core import PipelineCli, run_pipeline +from cdm_data_loaders.pipelines.core import ( + PipelineCli, + build_file_ingest_kwargs, + build_filesystem_resource, + run_pipeline, +) from cdm_data_loaders.pipelines.jsonlines.settings import ( EXTRACT_PIPELINE_NAME, JsonlExtractSettings, @@ -47,20 +51,12 @@ def run_jsonlines_pipeline(settings: JsonlExtractSettings) -> LoadInfo | None: :return: load information for the pipeline :rtype: LoadInfo | None """ - files = filesystem(bucket_url=str(settings.input_dir), file_glob=settings.file_glob) - - jsonl_resource = files | jsonl_reader(settings) + jsonl_resource = build_filesystem_resource(settings, jsonl_reader, settings, name="jsonl_reader") return run_pipeline( settings=settings, resource=jsonl_resource, - pipeline_kwargs={ - "pipeline_name": EXTRACT_PIPELINE_NAME, - "dataset_name": settings.dataset_name or EXTRACT_PIPELINE_NAME, - }, - pipeline_run_kwargs={ - "loader_file_format": str(settings.loader_file_format), - }, + **build_file_ingest_kwargs(settings, EXTRACT_PIPELINE_NAME, fallback_dataset_name=True), ) diff --git a/src/cdm_data_loaders/pipelines/jsonlines/extract_pydantic_validate_pipeline.py b/src/cdm_data_loaders/pipelines/jsonlines/extract_pydantic_validate_pipeline.py index 2b3ca0b4..5e30cb7f 100644 --- a/src/cdm_data_loaders/pipelines/jsonlines/extract_pydantic_validate_pipeline.py +++ b/src/cdm_data_loaders/pipelines/jsonlines/extract_pydantic_validate_pipeline.py @@ -31,7 +31,6 @@ class Sample(BaseModel): """ import json -from importlib import import_module from typing import Any from dlt.common.pipeline import LoadInfo @@ -40,6 +39,7 @@ class Sample(BaseModel): from cdm_data_loaders.pipelines.core import PipelineCli from cdm_data_loaders.pipelines.jsonlines.base import ( ParseErrorResolver, + load_registry_dict, run_registry_ingest, ) from cdm_data_loaders.pipelines.jsonlines.base import ( @@ -60,12 +60,9 @@ def load_entity_models(module_path: str) -> dict[str, type[BaseModel]]: :rtype: dict[str, type[BaseModel]] :raises ValueError: if ENTITY_MODELS is missing, empty, or not a dict """ - module = import_module(module_path) - registry = getattr(module, "ENTITY_MODELS", None) - if not isinstance(registry, dict) or not registry: - err_msg = f"{module_path} does not define a non-empty ENTITY_MODELS: dict[str, type[BaseModel]]" - raise ValueError(err_msg) - return registry + return load_registry_dict( + module_path, "ENTITY_MODELS", type_hint=": dict[str, type[BaseModel]]" + ) def _make_resolve_error(model: type[BaseModel]) -> ParseErrorResolver: diff --git a/src/cdm_data_loaders/pipelines/jsonlines/settings.py b/src/cdm_data_loaders/pipelines/jsonlines/settings.py index 6a82e588..59e0dde1 100644 --- a/src/cdm_data_loaders/pipelines/jsonlines/settings.py +++ b/src/cdm_data_loaders/pipelines/jsonlines/settings.py @@ -7,17 +7,14 @@ from cdm_data_loaders.core.fields import ( DEFAULT_JSONL_FILE_GLOB, - DEFAULTS, FILE_GLOB, - LOADER_FILE_FORMAT, TABLE_NAME, FileGlob, - LoaderFileFormat, NonEmptyStr, TableName, ) from cdm_data_loaders.core.settings import ( - CLI_SHORTCUTS, + FILE_FORMAT_CLI_SHORTCUTS, FileFormatIngestSettings, default_settings_with_shortcuts, ) @@ -33,7 +30,6 @@ LOCAL_CLI_SHORTCUTS: Final[dict[str, str]] = { ENTITY_MODELS_MODULE: "m", FILE_GLOB: "g", - LOADER_FILE_FORMAT: "f", SCHEMA_FILES_MODULE: "m", TABLE_NAME: "t", TABLE_NAMES: "t", @@ -50,7 +46,6 @@ class JsonlIngestSettings(FileFormatIngestSettings): description="Glob pattern for JSONL files", ), ] - loader_file_format: LoaderFileFormat = Field(default=DEFAULTS[LOADER_FILE_FORMAT]) class JsonlExtractSettings(JsonlIngestSettings): @@ -59,8 +54,8 @@ class JsonlExtractSettings(JsonlIngestSettings): model_config: SettingsConfigDict = default_settings_with_shortcuts( cli_prog_name=EXTRACT_PIPELINE_NAME, cli_shortcuts={ - **CLI_SHORTCUTS, - **{k: v for k, v in LOCAL_CLI_SHORTCUTS.items() if k in [FILE_GLOB, LOADER_FILE_FORMAT, TABLE_NAME]}, + **FILE_FORMAT_CLI_SHORTCUTS, + **{k: v for k, v in LOCAL_CLI_SHORTCUTS.items() if k in [FILE_GLOB, TABLE_NAME]}, }, ) @@ -90,11 +85,11 @@ class JsonlPydanticIngestSettings(ValidatedJsonlIngestSettings): model_config: SettingsConfigDict = default_settings_with_shortcuts( cli_prog_name=PYDANTIC_PIPELINE_NAME, cli_shortcuts={ - **CLI_SHORTCUTS, + **FILE_FORMAT_CLI_SHORTCUTS, **{ k: v for k, v in LOCAL_CLI_SHORTCUTS.items() - if k in [ENTITY_MODELS_MODULE, FILE_GLOB, LOADER_FILE_FORMAT, TABLE_NAMES] + if k in [ENTITY_MODELS_MODULE, FILE_GLOB, TABLE_NAMES] }, }, ) @@ -117,14 +112,13 @@ class JsonlJsonschemaIngestSettings(ValidatedJsonlIngestSettings): model_config: SettingsConfigDict = default_settings_with_shortcuts( cli_prog_name=JSONSCHEMA_PIPELINE_NAME, cli_shortcuts={ - **CLI_SHORTCUTS, + **FILE_FORMAT_CLI_SHORTCUTS, **{ k: v for k, v in LOCAL_CLI_SHORTCUTS.items() if k in [ FILE_GLOB, - LOADER_FILE_FORMAT, SCHEMA_FILES_MODULE, ] }, diff --git a/src/cdm_data_loaders/pipelines/ncbi_ftp_download.py b/src/cdm_data_loaders/pipelines/ncbi_ftp_download.py index c3589a7a..ad70da34 100644 --- a/src/cdm_data_loaders/pipelines/ncbi_ftp_download.py +++ b/src/cdm_data_loaders/pipelines/ncbi_ftp_download.py @@ -10,6 +10,7 @@ import shutil import tempfile import threading +from collections.abc import Callable from concurrent.futures import ThreadPoolExecutor, as_completed from datetime import UTC, datetime from ftplib import error_temp @@ -54,7 +55,6 @@ class DownloadSettings(LoggerSettings): output_dir: Path = Field( default=Path(OUTPUT_MOUNT), description="Output directory for downloaded assembly files", - validation_alias=AliasChoices("output-dir", "output_dir"), ) threads: int = Field( default=4, @@ -66,7 +66,6 @@ class DownloadSettings(LoggerSettings): ftp_host: str = Field( default=FTP_HOST, description="NCBI FTP hostname", - validation_alias=AliasChoices("ftp-host", "ftp_host"), ) limit: int | None = Field( default=None, @@ -116,12 +115,15 @@ def _upload_assembly_dir( # Batch download -def download_batch( +def download_batch( # noqa: PLR0913 manifest_path: Path, output_dir: Path, threads: int = 4, ftp_host: str = FTP_HOST, limit: int | None = None, + *, + on_assembly_downloaded: Callable[[PurePosixPath], None] | None = None, + progress_desc: str = "Downloading from NCBI FTP", ) -> dict[str, Any]: """Download all assemblies listed in the manifest. @@ -130,6 +132,8 @@ def download_batch( :param threads: number of parallel download threads :param ftp_host: FTP hostname :param limit: optional limit for testing + :param on_assembly_downloaded: optional callback invoked with each assembly's FTP path after download succeeds + :param progress_desc: description shown on the tqdm progress bar :return: report dict with overall stats """ with manifest_path.open() as f: @@ -163,16 +167,19 @@ def _attempt() -> dict[str, Any]: stats = _attempt() except Exception as e: # noqa: BLE001 return str(path), e - else: - with lock: - success_count += 1 - all_stats.append(stats) - return str(path), None + + if on_assembly_downloaded is not None: + on_assembly_downloaded(path) + + with lock: + success_count += 1 + all_stats.append(stats) + return str(path), None try: with ( tqdm.tqdm( - total=len(assembly_paths), unit="assembly", desc="Downloading from NCBI FTP", smoothing=0.01 + total=len(assembly_paths), unit="assembly", desc=progress_desc, smoothing=0.01 ) as pbar, ThreadPoolExecutor(max_workers=threads) as executor, ): @@ -235,7 +242,7 @@ def run_download(config: DownloadSettings) -> None: # Notebook / interactive entry point -def download_and_stage( # noqa: PLR0913, PLR0915 +def download_and_stage( # noqa: PLR0913 *, bucket: PurePosixPath, staging_key_prefix: PurePosixPath, @@ -285,95 +292,35 @@ def download_and_stage( # noqa: PLR0913, PLR0915 manifest_dest.write_bytes(manifest_local_path.read_bytes()) logger.info("Manifest read from local path: %s", manifest_local_path) - with manifest_dest.open() as f: - assembly_paths = [PurePosixPath(line.strip()) for line in f if line.strip() and not line.startswith("#")] - - if limit: - assembly_paths = assembly_paths[:limit] - - logger.info("Starting download & stage of %d assemblies with %d threads", len(assembly_paths), threads) - - pool = ThreadLocalFTP(ftp_host) - lock = threading.Lock() - success_count = 0 staged_objects = 0 - failed: list[dict[str, str]] = [] - all_stats: list[dict[str, Any]] = [] - - def _download_upload_one(path: PurePosixPath) -> tuple[str, Exception | None]: - nonlocal success_count, staged_objects - - @retry( - retry=retry_if_exception_type((error_temp, BrokenPipeError, ConnectionResetError, EOFError)), - stop=stop_after_attempt(5), - wait=wait_exponential(multiplier=1, min=5, max=60), - reraise=True, - before_sleep=before_sleep_log(logger, logging.WARNING), - ) - def _attempt() -> dict[str, Any]: - return download_assembly_to_local(path, tmp, ftp_host=ftp_host, ftp=pool.get()) - - try: - stats = _attempt() - except Exception as e: # noqa: BLE001 - return str(path), e - - if not dry_run: - _db, assembly_dir_name, _accession = parse_assembly_path(path) - assembly_local_dir = tmp / build_accession_path(assembly_dir_name) - count = _upload_assembly_dir(assembly_local_dir, tmp, bucket, staging_key_prefix) - with lock: - staged_objects += count - - with lock: - success_count += 1 - all_stats.append(stats) - return str(path), None - - try: - with ( - tqdm.tqdm( - total=len(assembly_paths), unit="assembly", desc="Downloading & staging", smoothing=0.01 - ) as pbar, - ThreadPoolExecutor(max_workers=threads) as executor, - ): - futures = {executor.submit(_download_upload_one, p): p for p in assembly_paths} - for future in as_completed(futures): - path, error = future.result() - if error: - logger.error("FAILED: %s: %s", path, error) - with lock: - failed.append({"path": path, "error": str(error)}) - pbar.update(1) - finally: - pool.close_all() - - report: dict[str, Any] = { - "timestamp": datetime.now(UTC).isoformat(), - "total_attempted": len(assembly_paths), - "succeeded": success_count, - "failed": len(failed), - "failures": failed, - "assembly_stats": all_stats, - } + stage_lock = threading.Lock() + + def _stage_assembly(path: PurePosixPath) -> None: + nonlocal staged_objects + _db, assembly_dir_name, _accession = parse_assembly_path(path) + assembly_local_dir = tmp / build_accession_path(assembly_dir_name) + count = _upload_assembly_dir(assembly_local_dir, tmp, bucket, staging_key_prefix) + with stage_lock: + staged_objects += count + + report = download_batch( + manifest_path=manifest_dest, + output_dir=tmp, + threads=threads, + ftp_host=ftp_host, + limit=limit, + on_assembly_downloaded=None if dry_run else _stage_assembly, + progress_desc="Downloading & staging", + ) if not dry_run: report_path = tmp / "download_report.json" - with report_path.open("w") as f: - json.dump(report, f, indent=2) if upload_file(report_path, str(bucket / staging_key_prefix), show_progress=False): staged_objects += 1 else: logger.warning("Failed to upload download report to s3://%s/%s", bucket, staging_key_prefix) logger.info("Staged %d objects to s3://%s/%s", staged_objects, bucket, staging_key_prefix) - logger.info( - "SUMMARY: %d attempted, %d succeeded, %d failed", - len(assembly_paths), - success_count, - len(failed), - ) - return { **report, "staged_objects": staged_objects, diff --git a/src/cdm_data_loaders/pipelines/ncbi_ftp_promote.py b/src/cdm_data_loaders/pipelines/ncbi_ftp_promote.py index 8d111629..be54e03d 100644 --- a/src/cdm_data_loaders/pipelines/ncbi_ftp_promote.py +++ b/src/cdm_data_loaders/pipelines/ncbi_ftp_promote.py @@ -30,21 +30,18 @@ class PromoteSettings(LoggerSettings): staging_bucket: PurePosixPath = Field( default=DEFAULT_STAGING_BUCKET, description="Bucket where staged files are located after download", - validation_alias=AliasChoices("staging-bucket", "staging_bucket"), ) destination_bucket: PurePosixPath = Field( default=DEFAULT_DESTINATION_BUCKET, description="Bucket to which staged files will be promoted.", - validation_alias=AliasChoices("destination-bucket", "destination_bucket"), ) staging_path: PurePosixPath = Field( description="Path to folder in the staging bucket where staged files are located; Should contain `raw_data/` folder", - validation_alias=AliasChoices("s", "staging-path", "staging_path"), + validation_alias=AliasChoices("s", "staging_path"), ) destination_path: PurePosixPath = Field( default=DEFAULT_DESTINATION_PREFIX, description="Path to folder in the destination bucket where files will be promoted to; Will contain `raw_data/` folder", - validation_alias=AliasChoices("destination-path", "destination_path"), ) removed_manifest_path: Path | None = Field( default=None, @@ -64,7 +61,6 @@ class PromoteSettings(LoggerSettings): dry_run: CliImplicitFlag[bool] = Field( default=False, description="Log actions without making changes", - validation_alias=AliasChoices("dry-run", "dry_run"), ) diff --git a/src/cdm_data_loaders/pipelines/pdb_manifest.py b/src/cdm_data_loaders/pipelines/pdb_manifest.py index 814c5600..b5e6acaa 100644 --- a/src/cdm_data_loaders/pipelines/pdb_manifest.py +++ b/src/cdm_data_loaders/pipelines/pdb_manifest.py @@ -29,7 +29,7 @@ from pydantic import AliasChoices, Field from pydantic_settings import CliImplicitFlag, SettingsConfigDict -from cdm_data_loaders.core.settings import DEFAULT_SETTINGS_CONFIG_DICT, CtsSettings +from cdm_data_loaders.core.settings import CtsSettings, default_settings_with_shortcuts from cdm_data_loaders.parsers.pdb_holdings_file import parse_holdings_file from cdm_data_loaders.pdb.constants import ( DEFAULT_DESTINATION_BUCKET, @@ -51,17 +51,15 @@ class PdbManfestSettings(CtsSettings): """Configuration for running the PDB manifest-generation pipeline.""" - model_config = SettingsConfigDict(**DEFAULT_SETTINGS_CONFIG_DICT, cli_prog_name="pdb_manifest") + model_config: SettingsConfigDict = default_settings_with_shortcuts(cli_prog_name="pdb_manifest") destination_bucket: PurePosixPath = Field( default=DEFAULT_DESTINATION_BUCKET, description="Bucket holding current PDB records", - validation_alias=AliasChoices("destination-bucket", "destination_bucket"), ) destination_prefix: PurePosixPath = Field( default=DEFAULT_DESTINATION_PREFIX, description="Path to folder in the destination bucket where PDB records exist. Must contain `raw_data/` folder", - validation_alias=AliasChoices("destination-prefix", "destination_prefix"), ) holdings_snapshot_path: PurePosixPath = Field( default=DEFAULT_HOLDINGS_SNAPSHOT, @@ -76,11 +74,9 @@ class PdbManfestSettings(CtsSettings): skip_diff: CliImplicitFlag[bool] = Field( default=False, description="If set, prevents reading of snapshot data. All current PDB records will be include in manifest", - validation_alias=AliasChoices("skip-diff", "skip_diff"), ) output_path: Path = Field( description="Local path to save generated manfiest files to", - validation_alias=AliasChoices("output-path", "output_path"), ) regex_filter: str | None = Field( default=None, diff --git a/src/cdm_data_loaders/pipelines/xml2db/pipeline.py b/src/cdm_data_loaders/pipelines/xml2db/pipeline.py index 1d4925ac..a31deca6 100644 --- a/src/cdm_data_loaders/pipelines/xml2db/pipeline.py +++ b/src/cdm_data_loaders/pipelines/xml2db/pipeline.py @@ -3,23 +3,23 @@ from collections.abc import Generator, Iterator from logging import Logger, getLogger from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import Any -import dlt from dlt.common.pipeline import LoadInfo from dlt.common.storages.fsspec_filesystem import FileItemDict from dlt.common.typing import TDataItems -from dlt.sources.filesystem import filesystem from xml2db import DataModel, load_config -from cdm_data_loaders.pipelines.core import PipelineCli, run_pipeline +from cdm_data_loaders.pipelines.core import ( + PipelineCli, + build_file_ingest_kwargs, + build_filesystem_resource, + run_pipeline, +) from cdm_data_loaders.pipelines.xml2db.compaction import compact_reused_tables from cdm_data_loaders.pipelines.xml2db.settings import PIPELINE_NAME, Xml2DbSettings from cdm_data_loaders.readers.xml2db_doc import build_xml2db_model, process_xml_file_with_xml2db -if TYPE_CHECKING: - from dlt.extract import DltResource - logger: Logger = getLogger(__name__) @@ -50,22 +50,14 @@ def run_xml2db_pipeline(settings: Xml2DbSettings) -> LoadInfo | None: model_config = load_config(str(settings.xml2db_config_file)) if settings.xml2db_config_file else None model = build_xml2db_model(settings.xsd_file, short_name=settings.short_name, model_config=model_config) - reader = dlt.transformer(_read_items, name="xml2db_reader", parallelized=True) - - files = filesystem(bucket_url=str(settings.input_dir), file_glob=settings.file_glob) - - xml2db_resource: DltResource = files | reader(settings, model) + xml2db_resource = build_filesystem_resource( + settings, _read_items, settings, model, name="xml2db_reader" + ) load_info = run_pipeline( settings=settings, resource=xml2db_resource, - pipeline_kwargs={ - "pipeline_name": PIPELINE_NAME, - "dataset_name": settings.dataset_name, - }, - pipeline_run_kwargs={ - "loader_file_format": str(settings.loader_file_format), - }, + **build_file_ingest_kwargs(settings, PIPELINE_NAME), ) _maybe_compact(settings, model, load_info) diff --git a/src/cdm_data_loaders/pipelines/xml2db/settings.py b/src/cdm_data_loaders/pipelines/xml2db/settings.py index 9f7d69d6..86512653 100644 --- a/src/cdm_data_loaders/pipelines/xml2db/settings.py +++ b/src/cdm_data_loaders/pipelines/xml2db/settings.py @@ -5,19 +5,20 @@ from pydantic import Field, FilePath, PositiveInt from cdm_data_loaders.core.fields import ( - DEFAULT_XML_FILE_GLOB, - FILE_GLOB, - FileGlob, NonEmptyStr, ) -from cdm_data_loaders.core.settings import FileFormatIngestSettings, default_settings_with_shortcuts +from cdm_data_loaders.core.settings import ( + XML_CLI_SHORTCUTS, + XmlIngestSettings, + default_settings_with_shortcuts, +) PIPELINE_NAME: Final[str] = "xml2db" DEFAULT_XML2DB_SHORT_NAME: Final[str] = "xml2db" DEFAULT_XML2DB_CHUNK_SIZE: Final[int] = 1000 -class Xml2DbSettings(FileFormatIngestSettings): +class Xml2DbSettings(XmlIngestSettings): """Settings for the xml2db-based XML ingestion pipeline. xml2db loads an entire XML document into memory before it can be converted into rows, which @@ -36,10 +37,7 @@ class Xml2DbSettings(FileFormatIngestSettings): model_config = default_settings_with_shortcuts( cli_prog_name=PIPELINE_NAME, - cli_shortcuts={ - **FileFormatIngestSettings.model_config["cli_shortcuts"], - FILE_GLOB: "g", - }, + cli_shortcuts=XML_CLI_SHORTCUTS, ) xml_tag: Annotated[ @@ -75,13 +73,6 @@ class Xml2DbSettings(FileFormatIngestSettings): ), ), ] - file_glob: Annotated[ - FileGlob, - Field( - default=DEFAULT_XML_FILE_GLOB, - description="Glob pattern for XML files inside settings.input_dir.", - ), - ] short_name: Annotated[ NonEmptyStr, Field( diff --git a/src/cdm_data_loaders/pipelines/xmltodict/pipeline.py b/src/cdm_data_loaders/pipelines/xmltodict/pipeline.py index d7b4afc2..a87dda43 100644 --- a/src/cdm_data_loaders/pipelines/xmltodict/pipeline.py +++ b/src/cdm_data_loaders/pipelines/xmltodict/pipeline.py @@ -2,21 +2,21 @@ from collections.abc import Generator, Iterator from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import Any -import dlt from dlt.common.pipeline import LoadInfo from dlt.common.storages.fsspec_filesystem import FileItemDict from dlt.common.typing import TDataItems -from dlt.sources.filesystem import filesystem -from cdm_data_loaders.pipelines.core import PipelineCli, run_pipeline +from cdm_data_loaders.pipelines.core import ( + PipelineCli, + build_file_ingest_kwargs, + build_filesystem_resource, + run_pipeline, +) from cdm_data_loaders.pipelines.xmltodict.settings import PIPELINE_NAME, XmlToDictSettings from cdm_data_loaders.readers.xml import process_xml_file_to_dict -if TYPE_CHECKING: - from dlt.extract import DltResource - def _read_xmltodict_items( items: Iterator[FileItemDict], settings: XmlToDictSettings @@ -40,21 +40,14 @@ def run_xmltodict_pipeline(settings: XmlToDictSettings) -> LoadInfo | None: :param settings: pipeline configuration :type settings: XmlToDictSettings """ - reader = dlt.transformer(_read_xmltodict_items, name="xmltodict_reader", parallelized=True) - files = filesystem(bucket_url=str(settings.input_dir), file_glob=settings.file_glob) - - xmltodict_resource: DltResource = files | reader(settings) + xmltodict_resource = build_filesystem_resource( + settings, _read_xmltodict_items, settings, name="xmltodict_reader" + ) return run_pipeline( settings=settings, resource=xmltodict_resource, - pipeline_kwargs={ - "pipeline_name": PIPELINE_NAME, - "dataset_name": settings.dataset_name, - }, - pipeline_run_kwargs={ - "loader_file_format": str(settings.loader_file_format), - }, + **build_file_ingest_kwargs(settings, PIPELINE_NAME), ) diff --git a/src/cdm_data_loaders/pipelines/xmltodict/settings.py b/src/cdm_data_loaders/pipelines/xmltodict/settings.py index 43b44fdf..d1d46179 100644 --- a/src/cdm_data_loaders/pipelines/xmltodict/settings.py +++ b/src/cdm_data_loaders/pipelines/xmltodict/settings.py @@ -6,38 +6,26 @@ from pydantic_settings import SettingsConfigDict from cdm_data_loaders.core.fields import ( - DEFAULT_XML_FILE_GLOB, - FILE_GLOB, - FileGlob, NonEmptyStr, TableName, ) from cdm_data_loaders.core.settings import ( - FileFormatIngestSettings, + XML_CLI_SHORTCUTS, + XmlIngestSettings, default_settings_with_shortcuts, ) PIPELINE_NAME: Final[str] = "xmltodict" -class XmlToDictSettings(FileFormatIngestSettings): +class XmlToDictSettings(XmlIngestSettings): """Settings for the XML ingestion pipeline.""" model_config: SettingsConfigDict = default_settings_with_shortcuts( cli_prog_name=PIPELINE_NAME, - cli_shortcuts={ - **FileFormatIngestSettings.model_config["cli_shortcuts"], - FILE_GLOB: "g", - }, + cli_shortcuts=XML_CLI_SHORTCUTS, ) - file_glob: Annotated[ - FileGlob, - Field( - default=DEFAULT_XML_FILE_GLOB, - description="Glob pattern for XML files inside each entity's input subdirectory.", - ), - ] table_name: TableName xml_tag: Annotated[ NonEmptyStr, diff --git a/src/cdm_data_loaders/pipelines/xmltopyarrow/pipeline.py b/src/cdm_data_loaders/pipelines/xmltopyarrow/pipeline.py index 94aeb548..ab1248ee 100644 --- a/src/cdm_data_loaders/pipelines/xmltopyarrow/pipeline.py +++ b/src/cdm_data_loaders/pipelines/xmltopyarrow/pipeline.py @@ -2,21 +2,21 @@ from collections.abc import Generator, Iterator from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import Any -import dlt from dlt.common.pipeline import LoadInfo from dlt.common.storages.fsspec_filesystem import FileItemDict from dlt.common.typing import TDataItems -from dlt.sources.filesystem import filesystem -from cdm_data_loaders.pipelines.core import PipelineCli, run_pipeline +from cdm_data_loaders.pipelines.core import ( + PipelineCli, + build_file_ingest_kwargs, + build_filesystem_resource, + run_pipeline, +) from cdm_data_loaders.pipelines.xmltopyarrow.settings import PIPELINE_NAME, XmlToPyArrowSettings from cdm_data_loaders.readers.xml import process_xml_file_to_pyarrow -if TYPE_CHECKING: - from dlt.extract import DltResource - def _read_items(items: Iterator[FileItemDict], settings: XmlToPyArrowSettings) -> Generator[TDataItems, Any, Any]: """Read each file in items. Yields PyArrow tables for matched `xml_tag` elements. @@ -38,22 +38,15 @@ def run_xmltopyarrow_pipeline(settings: XmlToPyArrowSettings) -> LoadInfo | None :param settings: pipeline configuration :type settings: XmlToPyArrowSettings """ - reader = dlt.transformer(_read_items, name="xmltopyarrow_reader", parallelized=True) - files = filesystem(bucket_url=str(settings.input_dir), file_glob=settings.file_glob) - - xmltopyarrow_resource: DltResource = files | reader(settings) + xmltopyarrow_resource = build_filesystem_resource( + settings, _read_items, settings, name="xmltopyarrow_reader" + ) pipeline_name = str(settings.model_config.get("cli_prog_name") or PIPELINE_NAME) return run_pipeline( settings=settings, resource=xmltopyarrow_resource, - pipeline_kwargs={ - "pipeline_name": pipeline_name, - "dataset_name": settings.dataset_name, - }, - pipeline_run_kwargs={ - "loader_file_format": str(settings.loader_file_format), - }, + **build_file_ingest_kwargs(settings, pipeline_name), ) diff --git a/src/cdm_data_loaders/pipelines/xmltopyarrow/settings.py b/src/cdm_data_loaders/pipelines/xmltopyarrow/settings.py index 4db39d25..ca480cd4 100644 --- a/src/cdm_data_loaders/pipelines/xmltopyarrow/settings.py +++ b/src/cdm_data_loaders/pipelines/xmltopyarrow/settings.py @@ -1,7 +1,7 @@ """Settings class for the XML to PyArrow pipeline for the KBase CTS.""" from pathlib import Path -from typing import Annotated, Final, Self +from typing import Annotated, ClassVar, Final, Self import pyarrow as pa import xmlschema @@ -9,15 +9,13 @@ from pydantic_settings import SettingsConfigDict from cdm_data_loaders.core.fields import ( - DEFAULT_XML_FILE_GLOB, - FILE_GLOB, DatasetName, - FileGlob, NonEmptyStr, TableName, ) from cdm_data_loaders.core.settings import ( - FileFormatIngestSettings, + XML_CLI_SHORTCUTS, + XmlIngestSettings, default_settings_with_shortcuts, ) from cdm_data_loaders.readers.xsd import ( @@ -40,26 +38,16 @@ UNIREF_XSD_FILE: Final[Path] = SCHEMAS_DIR / "uniref" / "uniref.xsd" -class XmlToPyArrowSettings(FileFormatIngestSettings): +class XmlToPyArrowSettings(XmlIngestSettings): """Settings for the XML to PyArrow ingestion pipeline.""" model_config: SettingsConfigDict = default_settings_with_shortcuts( cli_prog_name=PIPELINE_NAME, - cli_shortcuts={ - **FileFormatIngestSettings.model_config["cli_shortcuts"], - FILE_GLOB: "g", - }, + cli_shortcuts=XML_CLI_SHORTCUTS, ) _xsd_schema: xmlschema.XMLSchema = PrivateAttr() _pyarrow_schema: pa.Schema = PrivateAttr() - file_glob: Annotated[ - FileGlob, - Field( - default=DEFAULT_XML_FILE_GLOB, - description="Glob pattern for XML files inside each entity's input subdirectory.", - ), - ] table_name: TableName xml_tag: Annotated[ NonEmptyStr, @@ -112,24 +100,11 @@ def pyarrow_schema(self) -> pa.Schema: return self._pyarrow_schema -class UniProtXmlToPyArrowSettings(XmlToPyArrowSettings): - """Settings for the UniProt XML to PyArrow ingestion pipeline.""" +class PresetXmlToPyArrowSettings(XmlToPyArrowSettings): + """Base settings for XML-to-PyArrow pipelines backed by a bundled XSD and precomputed PyArrow schema.""" - model_config: SettingsConfigDict = default_settings_with_shortcuts( - cli_prog_name=UNIPROT_PIPELINE_NAME, - cli_shortcuts={ - **FileFormatIngestSettings.model_config["cli_shortcuts"], - FILE_GLOB: "g", - }, - ) + _PRECOMPUTED_SCHEMA: ClassVar[pa.Schema] - dataset_name: Annotated[ - DatasetName, - Field( - default=UNIPROT_PIPELINE_NAME, - description="The name of the dataset being produced", - ), - ] table_name: Annotated[ TableName, Field( @@ -144,6 +119,29 @@ class UniProtXmlToPyArrowSettings(XmlToPyArrowSettings): description="XML tag to capture the contents of", ), ] + + def _resolve_pyarrow_schema(self) -> pa.Schema: + """Return the precomputed PyArrow schema for this preset pipeline.""" + return self._PRECOMPUTED_SCHEMA + + +class UniProtXmlToPyArrowSettings(PresetXmlToPyArrowSettings): + """Settings for the UniProt XML to PyArrow ingestion pipeline.""" + + _PRECOMPUTED_SCHEMA: ClassVar[pa.Schema] = UNIPROT_SCHEMA + + model_config: SettingsConfigDict = default_settings_with_shortcuts( + cli_prog_name=UNIPROT_PIPELINE_NAME, + cli_shortcuts=XML_CLI_SHORTCUTS, + ) + + dataset_name: Annotated[ + DatasetName, + Field( + default=UNIPROT_PIPELINE_NAME, + description="The name of the dataset being produced", + ), + ] xsd_file: Annotated[ FilePath, Field( @@ -152,20 +150,15 @@ class UniProtXmlToPyArrowSettings(XmlToPyArrowSettings): ), ] - def _resolve_pyarrow_schema(self) -> pa.Schema: - """Return the precomputed UniProt PyArrow schema.""" - return UNIPROT_SCHEMA - -class UniRefXmlToPyArrowSettings(XmlToPyArrowSettings): +class UniRefXmlToPyArrowSettings(PresetXmlToPyArrowSettings): """Settings for the UniRef XML to PyArrow ingestion pipeline.""" + _PRECOMPUTED_SCHEMA: ClassVar[pa.Schema] = UNIREF_SCHEMA + model_config: SettingsConfigDict = default_settings_with_shortcuts( cli_prog_name=UNIREF_PIPELINE_NAME, - cli_shortcuts={ - **FileFormatIngestSettings.model_config["cli_shortcuts"], - FILE_GLOB: "g", - }, + cli_shortcuts=XML_CLI_SHORTCUTS, ) dataset_name: Annotated[ @@ -175,20 +168,6 @@ class UniRefXmlToPyArrowSettings(XmlToPyArrowSettings): description="The name of the dataset being produced", ), ] - table_name: Annotated[ - TableName, - Field( - default=ENTRY_TABLE_NAME, - description="The name of the table to export parsed data to", - ), - ] - xml_tag: Annotated[ - NonEmptyStr, - Field( - default=ENTRY_XML_TAG, - description="XML tag to capture the contents of", - ), - ] xsd_file: Annotated[ FilePath, Field( @@ -196,7 +175,3 @@ class UniRefXmlToPyArrowSettings(XmlToPyArrowSettings): description="Path to the UniRef XSD file describing the XML documents to be imported.", ), ] - - def _resolve_pyarrow_schema(self) -> pa.Schema: - """Return the precomputed UniRef PyArrow schema.""" - return UNIREF_SCHEMA diff --git a/src/cdm_data_loaders/pipelines/xsv/pipeline.py b/src/cdm_data_loaders/pipelines/xsv/pipeline.py index c4332c6c..65f4b8b3 100644 --- a/src/cdm_data_loaders/pipelines/xsv/pipeline.py +++ b/src/cdm_data_loaders/pipelines/xsv/pipeline.py @@ -13,14 +13,19 @@ from tempfile import TemporaryDirectory from typing import Any, Final -import dlt from dlt.common.pipeline import LoadInfo from dlt.common.schema.typing import TSchemaContractDict from dlt.common.storages.fsspec_filesystem import FileItemDict from dlt.common.typing import TDataItems -from dlt.sources.filesystem import filesystem -from cdm_data_loaders.pipelines.core import PipelineCli, run_pipeline +from cdm_data_loaders.pipelines.core import ( + DEFAULT_REJECTED_SUFFIX, + DEFAULT_SCHEMA_CONTRACT, + PipelineCli, + build_file_ingest_kwargs, + build_filesystem_resource, + run_pipeline, +) from cdm_data_loaders.pipelines.xsv.settings import PIPELINE_NAME, XsvIngestSettings from cdm_data_loaders.readers.jsonschema_xsv.xsv_reader import read_validated_rows from cdm_data_loaders.readers.jsonschema_xsv.xsv_validator.helpers import CleanerValidatorArgs, ErrorRecord @@ -28,12 +33,8 @@ from cdm_data_loaders.readers.jsonschema_xsv.xsv_validator.schema_utils import generate_header from cdm_data_loaders.utils.buffer import ListBuffer -SCHEMA_CONTRACT: Final[TSchemaContractDict] = { - "tables": "evolve", - "columns": "evolve", - "data_type": "discard_row", -} -REJECTED_SUFFIX: Final[str] = "_rejected" +SCHEMA_CONTRACT: Final[TSchemaContractDict] = DEFAULT_SCHEMA_CONTRACT +REJECTED_SUFFIX: Final[str] = DEFAULT_REJECTED_SUFFIX @dataclass @@ -149,10 +150,6 @@ def run_xsv_pipeline(settings: XsvIngestSettings) -> LoadInfo | None: """ qsv_cmd = qsv_check() - xsv_reader = dlt.transformer(_read_items, name="xsv_reader", parallelized=True) - - files = filesystem(bucket_url=str(settings.input_dir), file_glob=settings.file_glob) - with ( TemporaryDirectory(prefix="xsv_tmp_") as tmp_dir, TemporaryDirectory(prefix="xsv_qsv_output_") as qsv_output_dir, @@ -166,19 +163,19 @@ def run_xsv_pipeline(settings: XsvIngestSettings) -> LoadInfo | None: validated_dir=Path(validated_dir), ) - xsv_resource = files | xsv_reader(settings, work_paths) - xsv_resource.apply_hints(schema_contract=TSchemaContractDict(**SCHEMA_CONTRACT)) # pyright: ignore[reportCallIssue] + xsv_resource = build_filesystem_resource( + settings, + _read_items, + settings, + work_paths, + name="xsv_reader", + schema_contract=SCHEMA_CONTRACT, + ) return run_pipeline( settings=settings, resource=xsv_resource, - pipeline_kwargs={ - "pipeline_name": PIPELINE_NAME, - "dataset_name": settings.dataset_name, - }, - pipeline_run_kwargs={ - "loader_file_format": str(settings.loader_file_format), - }, + **build_file_ingest_kwargs(settings, PIPELINE_NAME), ) diff --git a/src/cdm_data_loaders/pipelines/xsv/settings.py b/src/cdm_data_loaders/pipelines/xsv/settings.py index 30c45955..26cf7ae4 100644 --- a/src/cdm_data_loaders/pipelines/xsv/settings.py +++ b/src/cdm_data_loaders/pipelines/xsv/settings.py @@ -11,7 +11,11 @@ FileGlob, TableName, ) -from cdm_data_loaders.core.settings import CLI_SHORTCUTS, FileFormatIngestSettings, default_settings_with_shortcuts +from cdm_data_loaders.core.settings import ( + FILE_FORMAT_CLI_SHORTCUTS, + FileFormatIngestSettings, + default_settings_with_shortcuts, +) from cdm_data_loaders.readers.jsonschema_xsv.xsv_reader import resolve_xsv_parsing_config from cdm_data_loaders.readers.jsonschema_xsv.xsv_validator.schema_utils import ( ValidatedSchema, @@ -25,9 +29,8 @@ SHORTCUTS: Final[dict[str, str]] = { - **CLI_SHORTCUTS, + **FILE_FORMAT_CLI_SHORTCUTS, "file_glob": "g", - "loader_file_format": "f", "schema_file": "s", "table_name": "t", } diff --git a/tests/cdm_data_loaders/core/test_settings.py b/tests/cdm_data_loaders/core/test_settings.py index fd06e723..93be2af0 100644 --- a/tests/cdm_data_loaders/core/test_settings.py +++ b/tests/cdm_data_loaders/core/test_settings.py @@ -10,10 +10,14 @@ from cdm_data_loaders.core.settings import ( DEFAULT_SETTINGS_CONFIG_DICT, + FILE_FORMAT_CLI_SHORTCUTS, + XML_CLI_SHORTCUTS, CdmDataLoadersBase, CtsSettings, + FileFormatIngestSettings, LocalInputOutputSettings, LoggerSettings, + XmlIngestSettings, default_settings_with_shortcuts, ) from tests.cdm_data_loaders.conftest import ( @@ -23,6 +27,8 @@ cts_expected, local_io_expected, ) +from tests.cdm_data_loaders.pipelines.helpers import settings_to_argv +from tests.conftest import LOCAL_FS TEST_CLASS_NAME: Final[str] = "FruitSettings" @@ -182,8 +188,9 @@ def test_cts_settings_pass_cli_shortcuts(input_dir: Path, output_dir: Path) -> N ) def test_cts_settings_fail_unrecognised_cli_args(input_dir: Path, output_dir: Path, extra_args: list[str]) -> None: """Raise SettingsError, not SystemExit, for CLI args that are not registered.""" + base = CtsSettings(input_dir=input_dir, output_dir=output_dir, use_destination=LOCAL_FS) with pytest.raises(SettingsError, match="unrecognized arguments"): - CliApp.run(CtsSettings, cli_args=["-i", str(input_dir), "-o", str(output_dir), *extra_args]) + CliApp.run(CtsSettings, cli_args=[*settings_to_argv(base), *extra_args]) @pytest.mark.parametrize( @@ -212,3 +219,76 @@ def test_settings_fail_missing_dirs( with pytest.raises(ValidationError) as exc_info: CliApp.run(settings_cls, cli_args=[]) assert {(err["loc"], err["type"]) for err in exc_info.value.errors()} == expected + + +def test_file_format_ingest_settings_pass_cli_shortcuts(input_dir: Path, output_dir: Path) -> None: + """FileFormatIngestSettings supports FILE_FORMAT_CLI_SHORTCUTS (-f) in addition to CtsSettings shortcuts.""" + assert FILE_FORMAT_CLI_SHORTCUTS["loader_file_format"] == "f" + settings = CliApp.run( + FileFormatIngestSettings, + cli_args=[ + "-i", + str(input_dir), + "-o", + str(output_dir), + "-d", + LOCAL_FS, + "--dataset-name", + "my_dataset", + "-f", + "jsonl", + ], + ) + check_settings( + settings, + { + **cts_expected(input_dir, output_dir, use_destination=LOCAL_FS, dataset_name="my_dataset"), + "loader_file_format": "jsonl", + }, + ) + + +@pytest.mark.parametrize("source", SETTINGS_SOURCES) +def test_xml_ingest_settings_pass_defaults_and_cli_shortcuts( + make_settings: SettingsFactory, source: str, input_dir: Path, output_dir: Path +) -> None: + """XmlIngestSettings defaults file_glob to '*.xml*' across sources and supports XML_CLI_SHORTCUTS (-f, -g).""" + assert XML_CLI_SHORTCUTS["loader_file_format"] == "f" + assert XML_CLI_SHORTCUTS["file_glob"] == "g" + default_glob_settings = make_settings( + XmlIngestSettings, + source, + { + "input_dir": str(input_dir), + "output_dir": str(output_dir), + "use_destination": LOCAL_FS, + "dataset_name": "xml_dataset", + }, + ) + check_settings( + default_glob_settings, + { + **cts_expected(input_dir, output_dir, use_destination=LOCAL_FS, dataset_name="xml_dataset"), + "loader_file_format": "parquet", + "file_glob": "*.xml*", + }, + ) + cli_settings = CliApp.run( + XmlIngestSettings, + cli_args=[ + *settings_to_argv(default_glob_settings, exclude=["loader_file_format", "file_glob"]), + "-f", + "jsonl", + "-g", + "*.xml.gz", + ], + ) + check_settings( + cli_settings, + { + **cts_expected(input_dir, output_dir, use_destination=LOCAL_FS, dataset_name="xml_dataset"), + "loader_file_format": "jsonl", + "file_glob": "*.xml.gz", + }, + ) + diff --git a/tests/cdm_data_loaders/pipelines/helpers.py b/tests/cdm_data_loaders/pipelines/helpers.py index 80ff6aa6..2691667e 100644 --- a/tests/cdm_data_loaders/pipelines/helpers.py +++ b/tests/cdm_data_loaders/pipelines/helpers.py @@ -3,7 +3,7 @@ import gzip import json from collections import defaultdict -from collections.abc import Callable, Iterable, Iterator +from collections.abc import Callable, Generator, Iterable from contextlib import contextmanager from datetime import datetime from logging import Logger, getLogger @@ -164,7 +164,7 @@ def fetch_all_duckdb(pipeline: dlt.Pipeline, query: str) -> list[tuple[Any, ...] @contextmanager -def duckdb_reference_from_jsonl(jsonl_path: Path) -> Iterator[duckdb.DuckDBPyConnection]: +def duckdb_reference_from_jsonl(jsonl_path: Path) -> Generator[duckdb.DuckDBPyConnection]: """Load a reference JSONL file into an in-memory DuckDB connection with a `reference` table.""" connection = duckdb.connect() connection.execute( diff --git a/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonschema_end_to_end.py b/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonschema_end_to_end.py index c67487e3..e5507091 100644 --- a/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonschema_end_to_end.py +++ b/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonschema_end_to_end.py @@ -16,6 +16,7 @@ assert_run_ok, parse_json_string, read_data_rows, + settings_to_argv, ) pytestmark = pytest.mark.parametrize("loader_file_format", LOADER_FILE_FORMATS, ids=str) @@ -131,30 +132,20 @@ def test_cli_pass_loads_selected_entity( pipeline_runner: PipelineTestRunner, ) -> None: """The real CTS CLI accepts a registry shortcut, table selection, and the requested file format.""" - settings = schema_settings_factory(input_dir=str(schema_input_dir), loader_file_format=loader_file_format) + settings = schema_settings_factory( + input_dir=str(schema_input_dir), + loader_file_format=loader_file_format, + dataset_name="schema_cli", + table_names=["gadget"], + ) load_info = assert_run_ok( pipeline_runner.run( JSONSCHEMA_PIPELINE_NAME, cli_args=[ - "--input-dir", - str(settings.input_dir), - "--output-dir", - str(settings.output_dir), - "--log-config-file", - str(settings.log_config_file), - "--use-destination", - str(settings.use_destination), - "--use-output-dir-for-pipeline-metadata", - "true", - "--loader-file-format", - loader_file_format, - "--dataset-name", - "schema_cli", + *settings_to_argv(settings, exclude=["schema_files_module"]), "-m", settings.schema_files_module, - "--table-names", - "gadget", ], ) ) diff --git a/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_reference_end_to_end.py b/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_reference_end_to_end.py index 9d794789..3ef5aa74 100644 --- a/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_reference_end_to_end.py +++ b/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_reference_end_to_end.py @@ -11,13 +11,18 @@ EXTRACT_PIPELINE_NAME, JSONSCHEMA_PIPELINE_NAME, PYDANTIC_PIPELINE_NAME, + JsonlExtractSettings, + JsonlJsonschemaIngestSettings, + JsonlPydanticIngestSettings, ) from tests.cdm_data_loaders.pipelines.framework import PipelineTestRunner from tests.cdm_data_loaders.pipelines.helpers import ( LOADER_FILE_FORMATS, assert_run_ok, + make_cts_settings, read_pipeline_tables, run_and_read, + settings_to_argv, ) from tests.cdm_data_loaders.pipelines.jsonlines.integration.jsonlines_reference_helpers import ( canonicalize_entry, @@ -269,42 +274,31 @@ def test_jsonlines_cli_pass_reference_dataset_loads( sorted_reference_entities: list[str], ) -> None: """All real CLI entry points load the reference dataset from command-line settings.""" - log_config = tmp_path / "logging.json" - log_config.write_text('{"version": 1}', encoding="utf-8") output_dir = tmp_path / "bucket" - registry_flag, registry_module = { - EXTRACT_PIPELINE_NAME: ("table-name", "dataset"), + settings_cls, registry_kwarg = { + EXTRACT_PIPELINE_NAME: (JsonlExtractSettings, {"table_name": "dataset"}), PYDANTIC_PIPELINE_NAME: ( - "entity-models-module", - "tests.cdm_data_loaders.pipelines.jsonlines.integration.dataset_report_models", + JsonlPydanticIngestSettings, + {"entity_models_module": "tests.cdm_data_loaders.pipelines.jsonlines.integration.dataset_report_models"}, ), JSONSCHEMA_PIPELINE_NAME: ( - "schema-files-module", - "tests.cdm_data_loaders.pipelines.jsonlines.integration.dataset_report_schemas", + JsonlJsonschemaIngestSettings, + {"schema_files_module": "tests.cdm_data_loaders.pipelines.jsonlines.integration.dataset_report_schemas"}, ), }[pipeline_kind] - argv = [ - pipeline_kind, - "--input-dir", - str(reference_data_dir / "chunk_52"), - "--output-dir", - str(output_dir), - "--log-config-file", - str(log_config), - "--use-destination", - dlt_destination_config, - "--dataset-name", - "reference_cli", - "--file-glob", - "**/*.jsonl", - "--loader-file-format", - str(loader_file_format), - "--use-output-dir-for-pipeline-metadata", - "true", - ] - if registry_flag: - argv.extend([f"--{registry_flag}", registry_module]) - load_info = assert_run_ok(pipeline_runner.run(pipeline_kind, cli_args=argv[1:])) + settings = make_cts_settings( + settings_cls, + tmp_path, + input_dir=str(reference_data_dir / "chunk_52"), + output_dir=str(output_dir), + use_destination=dlt_destination_config, + dataset_name="reference_cli", + file_glob="**/*.jsonl", + loader_file_format=loader_file_format, + use_output_dir_for_pipeline_metadata=True, + **registry_kwarg, + ) + load_info = assert_run_ok(pipeline_runner.run(pipeline_kind, cli_args=settings_to_argv(settings))) tables = read_pipeline_tables(output_dir / load_info.dataset_name, loader_file_format) assert ( sorted( diff --git a/tests/cdm_data_loaders/pipelines/jsonlines/test_registry.py b/tests/cdm_data_loaders/pipelines/jsonlines/test_registry.py index 7e416e7c..1cb90f52 100644 --- a/tests/cdm_data_loaders/pipelines/jsonlines/test_registry.py +++ b/tests/cdm_data_loaders/pipelines/jsonlines/test_registry.py @@ -3,10 +3,12 @@ import sys import types from collections.abc import Callable +from typing import Any import pytest from pydantic import BaseModel +from cdm_data_loaders.pipelines.jsonlines.base import load_registry_dict from cdm_data_loaders.pipelines.jsonlines.extract_pydantic_validate_pipeline import load_entity_models @@ -49,3 +51,57 @@ def test_load_entity_models_fail_invalid_registry( load_entity_models(module_name) finally: sys.modules.pop(module_name, None) + + +def test_load_registry_dict_pass_returns_mapping( + schema_module_factory: Callable[[dict[str, dict[str, Any]]], str], + widget_schema: dict[str, Any], +) -> None: + """load_registry_dict returns a non-empty dict attribute from the imported module.""" + expected = {"widget": widget_schema} + module_name = schema_module_factory(expected) + assert load_registry_dict(module_name, "ENTITY_SCHEMAS") == expected + + +@pytest.mark.parametrize( + ("attribute_present", "value", "type_hint", "expected_message"), + [ + pytest.param( + False, + None, + "", + "_test_load_registry_dict_invalid does not define a non-empty CUSTOM_REG", + id="missing_no_type_hint", + ), + pytest.param( + True, + {}, + " dict[str, int]", + "_test_load_registry_dict_invalid does not define a non-empty CUSTOM_REG dict[str, int]", + id="empty_with_type_hint", + ), + pytest.param( + True, + ("not", "a", "dict"), + " dict[str, Any]", + "_test_load_registry_dict_invalid does not define a non-empty CUSTOM_REG dict[str, Any]", + id="wrong_type_with_type_hint", + ), + ], +) +def test_load_registry_dict_fail_invalid_attribute( + monkeypatch: pytest.MonkeyPatch, + attribute_present: bool, + value: object, + type_hint: str, + expected_message: str, +) -> None: + """load_registry_dict raises ValueError with optional type_hint when attribute is missing, empty, or not a dict.""" + module_name = "_test_load_registry_dict_invalid" + module = types.ModuleType(module_name) + if attribute_present: + module.CUSTOM_REG = value # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, module_name, module) + with pytest.raises(ValueError, match=r"^" + expected_message.replace("[", r"\[").replace("]", r"\]") + r"$"): + load_registry_dict(module_name, "CUSTOM_REG", type_hint=type_hint) + diff --git a/tests/cdm_data_loaders/pipelines/ncbi_ftp_download/test_ncbi_ftp_download.py b/tests/cdm_data_loaders/pipelines/ncbi_ftp_download/test_ncbi_ftp_download.py index 960ea282..84a83169 100644 --- a/tests/cdm_data_loaders/pipelines/ncbi_ftp_download/test_ncbi_ftp_download.py +++ b/tests/cdm_data_loaders/pipelines/ncbi_ftp_download/test_ncbi_ftp_download.py @@ -127,6 +127,41 @@ def test_download_batch_handles_download_failure(tmp_path: Path) -> None: assert report["succeeded"] == 0 +def test_download_batch_pass_callback_and_comments(tmp_path: Path, output_dir: Path) -> None: + """download_batch skips blank/comment lines and calls on_assembly_downloaded only on success.""" + manifest = tmp_path / "manifest.txt" + good_path = "/genomes/all/GCF/000/001/215/GCF_000001215.4_Release_6_plus_ISO1_MT/" + bad_path = "/genomes/all/GCF/000/001/405/GCF_000001405.40_GRCh38.p14/" + manifest.write_text(f"# header comment\n\n{good_path}\n{bad_path}\n") + + callback_paths: list[PurePosixPath] = [] + + def _fake_download(path: PurePosixPath, output_dir: Path, **kwargs: object) -> dict[str, str | int]: # noqa: ARG001 + if str(path) == bad_path.rstrip("/"): + msg = "simulated error" + raise RuntimeError(msg) + return _MOCK_STATS + + with ( + patch("cdm_data_loaders.pipelines.ncbi_ftp_download.ThreadLocalFTP"), + patch( + "cdm_data_loaders.pipelines.ncbi_ftp_download.download_assembly_to_local", + side_effect=_fake_download, + ), + ): + report = download_batch( + manifest_path=manifest, + output_dir=output_dir, + threads=1, + on_assembly_downloaded=callback_paths.append, + ) + + assert report["total_attempted"] == _EXPECTED_ATTEMPTED + assert report["succeeded"] == 1 + assert report["failed"] == 1 + assert callback_paths == [PurePosixPath(good_path)] + + # Helpers shared by download_and_stage tests _MANIFEST_CONTENT = ( @@ -458,3 +493,42 @@ def test_download_and_stage_report_shape(tmp_path: Path, monkeypatch: pytest.Mon assert report["succeeded"] == _EXPECTED_ATTEMPTED reset_s3_client() + + +@pytest.mark.usefixtures("mock_s3_client") +def test_download_and_stage_pass_missing_dir_and_upload_failure(tmp_path: Path) -> None: + """download_and_stage handles missing local assembly directory and failed S3 uploads gracefully.""" + manifest_local = tmp_path / "manifest.txt" + manifest_local.write_text(_MANIFEST_CONTENT) + + assembly_rel = "raw_data/GCF/000/001/215/GCF_000001215.4_Release_6_plus_ISO1_MT" + + def _fake_download(path: PurePosixPath, output_dir: Path, **kwargs: object) -> dict[str, str | int]: # noqa: ARG001 + if "000001215" in str(path): + asm_dir = Path(output_dir) / assembly_rel + asm_dir.mkdir(parents=True) + (asm_dir / "genomic.fna.gz").write_bytes(b"fasta_data") + # For the second assembly, do not create any directory to exercise the missing-dir path + return _MOCK_STATS + + with ( + patch("cdm_data_loaders.pipelines.ncbi_ftp_download.ThreadLocalFTP"), + patch( + "cdm_data_loaders.pipelines.ncbi_ftp_download.download_assembly_to_local", + side_effect=_fake_download, + ), + patch( + "cdm_data_loaders.pipelines.ncbi_ftp_download.upload_file", + return_value=False, + ), + ): + report = download_and_stage( + bucket=_TEST_BUCKET, + staging_key_prefix=_STAGING_PREFIX, + manifest_local_path=manifest_local, + dry_run=False, + threads=1, + ) + + assert report["succeeded"] == _EXPECTED_ATTEMPTED + assert report["staged_objects"] == 0 diff --git a/tests/cdm_data_loaders/pipelines/ncbi_rest_api/test_ncbi_rest_api_settings.py b/tests/cdm_data_loaders/pipelines/ncbi_rest_api/test_ncbi_rest_api_settings.py index c8552e18..de89c758 100644 --- a/tests/cdm_data_loaders/pipelines/ncbi_rest_api/test_ncbi_rest_api_settings.py +++ b/tests/cdm_data_loaders/pipelines/ncbi_rest_api/test_ncbi_rest_api_settings.py @@ -19,6 +19,7 @@ set_settings, ) from tests.cdm_data_loaders.conftest import check_settings +from tests.cdm_data_loaders.pipelines.helpers import settings_to_argv from tests.helpers import assert_cli_field_roundtrips, assert_no_cli_clashes BATCH_SIZE = 200 @@ -104,14 +105,23 @@ def test_settings_valid_batch_size( ], ) @pytest.mark.parametrize("use_cliapp", [True, False]) -def test_cli_invalid_batch_size_via_cli_raises(bad_batch_size: str, message: str, use_cliapp: bool) -> None: +def test_cli_invalid_batch_size_via_cli_raises( + bad_batch_size: str, + message: str, + use_cliapp: bool, + test_settings: NcbiRestApiSettings, + test_defaults: dict[str, Any], +) -> None: """Ensure that an invalid batch size passed via CLI raises an error.""" if use_cliapp: with pytest.raises(ValidationError, match=message): - CliApp.run(NcbiRestApiSettings, cli_args=["--batch-size", bad_batch_size]) + CliApp.run( + NcbiRestApiSettings, + cli_args=[*settings_to_argv(test_settings, exclude=["batch_size"]), "--batch-size", bad_batch_size], + ) else: with pytest.raises(ValidationError, match=message): - NcbiRestApiSettings(batch_size=bad_batch_size) # pyright: ignore[reportCallIssue] + NcbiRestApiSettings(batch_size=bad_batch_size, **test_defaults) # pyright: ignore[reportCallIssue] @pytest.mark.parametrize( @@ -153,14 +163,22 @@ def test_cli_valid_query_type( ], ) @pytest.mark.parametrize("use_cliapp", [True, False]) -def test_cli_invalid_query_type(query_type: str, use_cliapp: bool) -> None: +def test_cli_invalid_query_type( + query_type: str, + use_cliapp: bool, + test_settings: NcbiRestApiSettings, + test_defaults: dict[str, Any], +) -> None: """Ensure that an invalid batch size passed via CLI raises an error.""" if use_cliapp: with pytest.raises(ValidationError, match=STRING_MATCH_MESSAGE): - CliApp.run(NcbiRestApiSettings, cli_args=["--query-type", query_type]) + CliApp.run( + NcbiRestApiSettings, + cli_args=[*settings_to_argv(test_settings, exclude=["query_type"]), "--query-type", query_type], + ) else: with pytest.raises(ValidationError, match=STRING_MATCH_MESSAGE): - NcbiRestApiSettings(query_type=query_type) # pyright: ignore[reportCallIssue] + NcbiRestApiSettings(query_type=query_type, **test_defaults) # pyright: ignore[reportCallIssue] def test_settings_all_params_set( diff --git a/tests/cdm_data_loaders/pipelines/test_core.py b/tests/cdm_data_loaders/pipelines/test_core.py index 3d442d45..b2a563e6 100644 --- a/tests/cdm_data_loaders/pipelines/test_core.py +++ b/tests/cdm_data_loaders/pipelines/test_core.py @@ -23,14 +23,24 @@ from requests import RequestException from cdm_data_loaders.core.fields import OUTPUT_DIR, USE_DESTINATION -from cdm_data_loaders.core.settings import CdmDataLoadersBase, CtsSettings, LocalInputOutputSettings, LoggerSettings +from cdm_data_loaders.core.settings import ( + CdmDataLoadersBase, + CtsSettings, + FileFormatIngestSettings, + LocalInputOutputSettings, + LoggerSettings, +) from cdm_data_loaders.pipelines import core from cdm_data_loaders.pipelines.core import ( + DEFAULT_REJECTED_SUFFIX, + DEFAULT_SCHEMA_CONTRACT, LOAD_INFO_TABLE_NAME, NO_MESSAGE, WEBHOOK_NOT_CONFIGURED, PipelineCli, PipelineReport, + build_file_ingest_kwargs, + build_filesystem_resource, run_cli, run_pipeline, save_pipeline_metadata, @@ -38,7 +48,14 @@ write_pipeline_report, ) from tests.cdm_data_loaders.pipelines.conftest import HOOK_ENV_VAR -from tests.cdm_data_loaders.pipelines.helpers import read_destination_rows, read_jsonl_file +from tests.cdm_data_loaders.pipelines.helpers import ( + make_cts_settings as make_pipeline_cts_settings, +) +from tests.cdm_data_loaders.pipelines.helpers import ( + read_destination_rows, + read_jsonl_file, + settings_to_argv, +) from tests.conftest import LOCAL_FS, S3 from tests.dlt_config_isolation import dlt_config_unset, isolated_dlt_config @@ -941,12 +958,14 @@ def test_pipeline_cli_pass_run(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) @pytest.mark.usefixtures("restore_logging") -def test_pipeline_cli_pass_call_reads_argv(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +def test_pipeline_cli_pass_call_reads_argv( + dlt_test_settings: CtsSettings, monkeypatch: pytest.MonkeyPatch +) -> None: """Calling the CLI parses sys.argv, runs the pipeline and returns None so the process exits with 0.""" received, pipeline_fn = make_recorder() - monkeypatch.setattr(sys, "argv", ["prog", "-i", str(tmp_path), "-o", str(tmp_path / "out"), "-d", LOCAL_FS]) + monkeypatch.setattr(sys, "argv", ["prog", *settings_to_argv(dlt_test_settings)]) assert PipelineCli(CtsSettings, pipeline_fn)() is None - assert [s.input_dir for s in received] == [tmp_path.resolve()] + assert [s.input_dir for s in received] == [dlt_test_settings.input_dir] @pytest.mark.parametrize( @@ -1597,3 +1616,90 @@ def test_write_pipeline_report_fail_oserror_logged( error_messages = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR] assert error_messages == ["Could not write pipeline report file: disk full"] + + +def test_build_filesystem_resource_pass_default_glob_and_callable_transformer( + input_dir: Path, output_dir: Path +) -> None: + """build_filesystem_resource uses settings.file_glob or DEFAULT_XML_FILE_GLOB and wraps a callable in dlt.transformer.""" + file_path = input_dir / "sample.xml" + file_path.write_text("hello", encoding="utf-8") + settings = LocalInputOutputSettings(input_dir=input_dir, output_dir=output_dir) + + def read_items(items: TDataItems, prefix: str) -> Iterator[dict[str, Any]]: + for item in items: + text = Path(item["file_url"].removeprefix("file://")).read_text(encoding="utf-8") + yield {"text": f"{prefix}:{text}"} + + resource = build_filesystem_resource(settings, read_items, "tag", name="sample_reader") + rows = list(resource) + assert rows == [{"text": "tag:hello"}] + + +def test_build_filesystem_resource_pass_custom_glob_resource_transformer_and_contract( + input_dir: Path, output_dir: Path +) -> None: + """build_filesystem_resource accepts a DltResource transformer, explicit file_glob, and schema_contract.""" + matched = input_dir / "data.jsonl" + matched.write_text("matched", encoding="utf-8") + ignored = input_dir / "ignore.txt" + ignored.write_text("ignored", encoding="utf-8") + settings = LocalInputOutputSettings(input_dir=input_dir, output_dir=output_dir) + + @dlt.transformer(name="jsonl_test_reader") + def read_jsonl_items(items: TDataItems) -> Iterator[dict[str, Any]]: + for item in items: + yield {"name": Path(item["file_url"].removeprefix("file://")).name} + + resource = build_filesystem_resource( + settings, + read_jsonl_items, + name="jsonl_test_reader", + file_glob="*.jsonl", + schema_contract=DEFAULT_SCHEMA_CONTRACT, + ) + rows = list(resource) + assert rows == [{"name": "data.jsonl"}] + + +@pytest.mark.parametrize( + ("dataset_name", "fallback_dataset_name", "expected_dataset_name"), + [ + pytest.param("custom_ds", False, "custom_ds", id="explicit_no_fallback"), + pytest.param("custom_ds", True, "custom_ds", id="explicit_with_fallback"), + pytest.param(None, False, None, id="none_no_fallback"), + pytest.param(None, True, TINY_PIPELINE_NAME, id="none_with_fallback"), + ], +) +def test_build_file_ingest_kwargs_pass( + tmp_path: Path, + dataset_name: str | None, + fallback_dataset_name: bool, + expected_dataset_name: str | None, +) -> None: + """build_file_ingest_kwargs constructs pipeline_kwargs and pipeline_run_kwargs from FileFormatIngestSettings.""" + base_settings: FileFormatIngestSettings = make_pipeline_cts_settings( + FileFormatIngestSettings, + tmp_path, + dataset_name="custom_ds", + loader_file_format="jsonl", + ) + settings = base_settings.model_copy(update={"dataset_name": dataset_name}) + + result = build_file_ingest_kwargs( + settings, + TINY_PIPELINE_NAME, + fallback_dataset_name=fallback_dataset_name, + ) + + assert DEFAULT_REJECTED_SUFFIX == "_rejected" + assert result == { + "pipeline_kwargs": { + "pipeline_name": TINY_PIPELINE_NAME, + "dataset_name": expected_dataset_name, + }, + "pipeline_run_kwargs": { + "loader_file_format": "jsonl", + }, + } + diff --git a/tests/cdm_data_loaders/pipelines/test_framework.py b/tests/cdm_data_loaders/pipelines/test_framework.py index ec66cda9..03fa804d 100644 --- a/tests/cdm_data_loaders/pipelines/test_framework.py +++ b/tests/cdm_data_loaders/pipelines/test_framework.py @@ -8,6 +8,7 @@ import pytest from pydantic import BaseModel +from cdm_data_loaders.core.settings import CtsSettings from cdm_data_loaders.pipelines import uniprot_kb_cdm from cdm_data_loaders.pipelines.jsonlines.extract_pipeline import ( cli as jsonl_extract_cli, @@ -35,6 +36,7 @@ from tests.cdm_data_loaders.pipelines.helpers import ( assert_pipeline_report_written, assert_run_ok, + make_cts_settings, read_data_rows, settings_to_argv, ) @@ -107,7 +109,7 @@ def test_settings_to_argv_pass_excludes_computed_fields_and_expands_lists(tmp_pa loader_file_format="jsonl", schema_files_module="some.module", table_names=["alpha", "beta"], - ) + ) # pyright: ignore[reportCallIssue] argv = settings_to_argv(settings) assert "--pipeline-dir" not in argv assert "--log-config-file" not in argv @@ -287,7 +289,7 @@ def fake_docker_run(cmd: list[str], **_kwargs: object) -> subprocess.CompletedPr table_name="entries", xml_tag="entry", loader_file_format="parquet", - ) + ) # pyright: ignore[reportCallIssue] runner = PipelineTestRunner(mode=ExecutionMode.DOCKER, docker_image="test-image:latest") cli_args = [ @@ -326,10 +328,7 @@ def test_pipeline_test_runner_fail_docker_mode_nonzero_exit( monkeypatch: pytest.MonkeyPatch, ) -> None: """Docker mode raises ValueError on validation errors and RuntimeError on general container failures.""" - input_dir = tmp_path / "input" - input_dir.mkdir() - output_dir = tmp_path / "output" - output_dir.mkdir() + settings = make_cts_settings(CtsSettings, tmp_path) monkeypatch.setattr(framework, "_built_docker_images", {"test-image:latest"}) def fake_fail_runtime(cmd: list[str], **_kwargs: object) -> subprocess.CompletedProcess[str]: @@ -340,7 +339,7 @@ def fake_fail_runtime(cmd: list[str], **_kwargs: object) -> subprocess.Completed with pytest.raises(RuntimeError, match="failed with exit code 2"): runner.run( "jsonlines_extract", - cli_args=["--input-dir", str(input_dir), "--output-dir", str(output_dir)], + cli_args=settings_to_argv(settings), ) def fake_fail_value_error(cmd: list[str], **_kwargs: object) -> subprocess.CompletedProcess[str]: @@ -350,7 +349,7 @@ def fake_fail_value_error(cmd: list[str], **_kwargs: object) -> subprocess.Compl with pytest.raises(ValueError, match="Unknown table name"): runner.run( "jsonlines_extract", - cli_args=["--input-dir", str(input_dir), "--output-dir", str(output_dir)], + cli_args=settings_to_argv(settings), ) @@ -413,7 +412,7 @@ def test_pipeline_test_runner_pass_real_docker_container(tmp_path: Path, docker_ dataset_name="docker_framework_ds", table_name="items", loader_file_format="jsonl", - ) + ) # pyright: ignore[reportCallIssue] runner = PipelineTestRunner(mode=ExecutionMode.DOCKER, docker_image=docker_image) load_info = assert_run_ok(runner.run("jsonlines_extract", settings=settings)) diff --git a/tests/cdm_data_loaders/pipelines/uniref_cdm/test_uniref_cdm_settings.py b/tests/cdm_data_loaders/pipelines/uniref_cdm/test_uniref_cdm_settings.py index 22bdfda2..9623f804 100644 --- a/tests/cdm_data_loaders/pipelines/uniref_cdm/test_uniref_cdm_settings.py +++ b/tests/cdm_data_loaders/pipelines/uniref_cdm/test_uniref_cdm_settings.py @@ -7,12 +7,14 @@ from pydantic import ValidationError from pydantic_settings import CliApp +from cdm_data_loaders.core.settings import CtsSettings from cdm_data_loaders.pipelines.uniref_cdm import ( UNIREF_VARIANTS, VARIANT, UnirefSettings, ) from tests.cdm_data_loaders.conftest import check_settings +from tests.cdm_data_loaders.pipelines.helpers import settings_to_argv from tests.cdm_data_loaders.pipelines.uniref_cdm.conftest import ( UNIREF_VARIANT_ALIASES, ) @@ -63,10 +65,10 @@ def test_settings_valid_variants_accepted(uniref_variant_value: str, test_defaul @pytest.mark.parametrize(VARIANT, UNIREF_VARIANT_ALIASES) def test_cli_valid_variants_accepted(uniref_variant_value: str, variant: str, test_defaults: dict[str, Any]) -> None: """Ensure that each valid uniref variant value is accepted without error when passed via CLI.""" - default_args = [arg for k, v in test_defaults.items() for arg in (make_cli_arg(k), str(v))] + base_settings = CtsSettings(**test_defaults) s = CliApp.run( UnirefSettings, - cli_args=[*default_args, make_cli_arg(variant), uniref_variant_value], + cli_args=[*settings_to_argv(base_settings), make_cli_arg(variant), uniref_variant_value], ) assert isinstance(s, UnirefSettings) assert s.variant == uniref_variant_value @@ -90,9 +92,9 @@ def test_cli_invalid_variant_via_cli_raises( test_defaults: dict[str, Any], ) -> None: """Ensure that an invalid uniref variant passed via CLI raises an error.""" - default_args = [arg for k, v in test_defaults.items() for arg in (make_cli_arg(k), str(v))] + base_settings = CtsSettings(**test_defaults) with pytest.raises(ValidationError, match="1 validation error for UnirefSettings") as exc_info: - CliApp.run(UnirefSettings, cli_args=[*default_args, make_cli_arg(variant), value]) + CliApp.run(UnirefSettings, cli_args=[*settings_to_argv(base_settings), make_cli_arg(variant), value]) exc_message = str(exc_info.value) assert "Input should be '50', '90' or '100'" in exc_message @@ -109,9 +111,9 @@ def test_missing_required_uniref_variant_raises(test_defaults: dict[str, Any]) - def test_cli_missing_required_uniref_variant_raises(test_defaults: dict[str, Any]) -> None: """Ensure that omitting the required uniref variant argument raises an error.""" - default_args = [arg for k, v in test_defaults.items() for arg in (make_cli_arg(k), str(v))] + base_settings = CtsSettings(**test_defaults) with pytest.raises(ValidationError, match="1 validation error for UnirefSettings") as exc_info: - CliApp.run(UnirefSettings, cli_args=default_args) + CliApp.run(UnirefSettings, cli_args=settings_to_argv(base_settings)) exc_message = str(exc_info.value) assert "Field required" in exc_message @@ -126,14 +128,12 @@ def test_cli_invalid_variant_and_destination_via_cli_raises( N.b. Pydantic only reports the first error!!! """ - default_args = [ - arg for k, v in test_defaults.items() if k != "use_destination" for arg in (make_cli_arg(k), str(v)) - ] + base_settings = CtsSettings(**test_defaults) with pytest.raises(ValidationError, match="1 validation error for UnirefSettings") as exc_info: CliApp.run( UnirefSettings, cli_args=[ - *default_args, + *settings_to_argv(base_settings, exclude=["use_destination"]), make_cli_arg(variant), value, "--use-destination", diff --git a/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_settings.py b/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_settings.py index 5ee2981b..383574d2 100644 --- a/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_settings.py +++ b/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_settings.py @@ -15,7 +15,7 @@ PIPELINE_NAME, Xml2DbSettings, ) -from tests.cdm_data_loaders.pipelines.helpers import make_cts_settings +from tests.cdm_data_loaders.pipelines.helpers import make_cts_settings, settings_to_argv from tests.conftest import S3 MINIMAL_SETTINGS_KWARGS: dict[str, object] = { @@ -199,33 +199,13 @@ def test_xml2db_settings_pass_pipeline_name_constant() -> None: assert Xml2DbSettings.model_config.get("cli_prog_name") == PIPELINE_NAME -def test_xml2db_settings_pass_cli_shortcut_g_targets_file_glob(tmp_path: Path, test_data_dir: Path) -> None: +def test_xml2db_settings_pass_cli_shortcut_g_targets_file_glob( + settings_factory: Callable[..., Xml2DbSettings], +) -> None: """The '-g' shortcut added on top of the shared shortcuts parses as file_glob.""" - input_dir = tmp_path / "input" - input_dir.mkdir() - output_dir = tmp_path / "output" - output_dir.mkdir() - xsd_file = _write_xsd_fixture(input_dir, test_data_dir, "uniref.xsd") - log_config_file = tmp_path / "logging.json" - log_config_file.write_text('{"version": 1}', encoding="utf-8") - + base_settings = settings_factory() settings = CliApp.run( Xml2DbSettings, - cli_args=[ - "--input-dir", - str(input_dir), - "--output-dir", - str(output_dir), - "--use-destination", - "local_fs", - "--log-config-file", - str(log_config_file), - "--dataset-name", - "ds", - "--xsd-file", - str(input_dir / xsd_file), - "-g", - "*.gz", - ], + cli_args=[*settings_to_argv(base_settings, exclude=["file_glob"]), "-g", "*.gz"], ) assert settings.file_glob == "*.gz" diff --git a/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_pipeline.py b/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_pipeline.py index d39392af..b3ab9399 100644 --- a/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_pipeline.py +++ b/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_pipeline.py @@ -20,6 +20,7 @@ ) from cdm_data_loaders.pipelines.xmltodict.settings import PIPELINE_NAME, XmlToDictSettings from tests.cdm_data_loaders.pipelines.conftest import duckdb_pipeline +from tests.cdm_data_loaders.pipelines.helpers import assert_run_ok, fetch_all_duckdb, settings_to_argv from tests.xml_samples import TWO_BOOK_LIBRARY_XML @@ -64,6 +65,7 @@ def test_run_xmltodict_pipeline_pass_binds_reader_before_run_pipeline( def test_cli_pass_runs_end_to_end_from_command_line_arguments( tmp_path: Path, + settings_factory: Callable[..., XmlToDictSettings], monkeypatch: pytest.MonkeyPatch, ) -> None: """cli() reads command-line arguments and runs the pipeline via run_xmltodict_pipeline. @@ -72,36 +74,9 @@ def test_cli_pass_runs_end_to_end_from_command_line_arguments( config isolation fixture), and core.run_pipeline is redirected to a real DuckDB pipeline so the full flow is validated. """ - input_dir = tmp_path / "cli_input" - input_dir.mkdir() - (input_dir / "library.xml").write_text(TWO_BOOK_LIBRARY_XML, encoding="utf-8") - output_dir = tmp_path / "cli_output" - output_dir.mkdir() - log_config_file = tmp_path / "logging.json" - log_config_file.write_text('{"version": 1}') - - argv = [ - PIPELINE_NAME, - "--input-dir", - str(input_dir), - "--output-dir", - str(output_dir), - "--use-destination", - "local_fs", - "--log-config-file", - str(log_config_file), - "--dataset-name", - "cli_dataset", - "--table-name", - "book", - "--xml-tag", - "book", - "--use-output-dir-for-pipeline-metadata", - "false", - "--dlt-dev-mode", - "false", - ] - monkeypatch.setattr(sys, "argv", argv) + settings = settings_factory(dataset_name="cli_dataset", dlt_dev_mode=False) + (Path(settings.input_dir) / "library.xml").write_text(TWO_BOOK_LIBRARY_XML, encoding="utf-8") + monkeypatch.setattr(sys, "argv", [PIPELINE_NAME, *settings_to_argv(settings)]) # unique pipeline name: the duckdb database file is derived from it, so # repeated runs of the suite do not see rows from previous runs @@ -118,55 +93,20 @@ def fake_run_pipeline( return pipeline.run(resource) with patch.object(xmltodict_module, "run_pipeline", fake_run_pipeline): - load_info = cli.run() - - assert load_info is not None - assert not load_info.has_failed_jobs + load_info = assert_run_ok(cli.run()) - with ( - load_info.pipeline.sql_client() as client, - client.execute_query("SELECT COUNT(*) FROM book") as cur, - ): - (row_count,) = cur.fetchone() - assert row_count == 2 # noqa: PLR2004 + assert fetch_all_duckdb(load_info.pipeline, "SELECT COUNT(*) FROM book") == [(2,)] def test_cli_pass_writes_rows_for_matching_xml_files( tmp_path: Path, + settings_factory: Callable[..., XmlToDictSettings], monkeypatch: pytest.MonkeyPatch, ) -> None: """cli() with xml files in the input dir loads one row per matching element.""" - input_dir = tmp_path / "xml_input" - input_dir.mkdir() - (input_dir / "data.xml").write_text(TWO_BOOK_LIBRARY_XML, encoding="utf-8") - - output_dir = tmp_path / "cli_output" - output_dir.mkdir() - log_config_file = tmp_path / "logging.json" - log_config_file.write_text('{"version": 1}') - - argv = [ - PIPELINE_NAME, - "--input-dir", - str(input_dir), - "--output-dir", - str(output_dir), - "--use-destination", - "local_fs", - "--log-config-file", - str(log_config_file), - "--dataset-name", - "cli_dataset", - "--table-name", - "book", - "--xml-tag", - "book", - "--use-output-dir-for-pipeline-metadata", - "false", - "--dlt-dev-mode", - "false", - ] - monkeypatch.setattr(sys, "argv", argv) + settings = settings_factory(dataset_name="cli_dataset", dlt_dev_mode=False) + (Path(settings.input_dir) / "data.xml").write_text(TWO_BOOK_LIBRARY_XML, encoding="utf-8") + monkeypatch.setattr(sys, "argv", [PIPELINE_NAME, *settings_to_argv(settings)]) pipeline_name = f"test_xml_cli_rows_pipeline_{uuid4().hex}" @@ -179,52 +119,24 @@ def fake_run_pipeline( return pipeline.run(resource) with patch.object(xmltodict_module, "run_pipeline", fake_run_pipeline): - load_info = cli.run() + load_info = assert_run_ok(cli.run()) - assert load_info is not None - assert not load_info.has_failed_jobs - pipeline = load_info.pipeline - with ( - pipeline.sql_client() as client, - client.execute_query("SELECT COUNT(*) FROM book") as cur, - ): - (row_count,) = cur.fetchone() - assert row_count == 2 # noqa: PLR2004 + assert fetch_all_duckdb(load_info.pipeline, "SELECT COUNT(*) FROM book") == [(2,)] -@pytest.mark.parametrize( - ("missing_args", "expected_fragment"), - [ - (["--table-name", "book", "--xml-tag", "book"], "dataset_name"), - (["--dataset-name", "cli_dataset", "--xml-tag", "book"], "table_name"), - (["--dataset-name", "cli_dataset", "--table-name", "book"], "xml_tag"), - ], -) +@pytest.mark.parametrize("missing_field", ["dataset_name", "table_name", "xml_tag"]) def test_cli_fail_missing_required_args_raise( - tmp_path: Path, + settings_factory: Callable[..., XmlToDictSettings], monkeypatch: pytest.MonkeyPatch, - missing_args: list[str], - expected_fragment: str, + missing_field: str, ) -> None: """cli() without required pipeline-specific args raises before running the pipeline.""" - log_config_file = tmp_path / "logging.json" - log_config_file.write_text('{"version": 1}') - - argv = [ - PIPELINE_NAME, - "--input-dir", - str(tmp_path / "input"), - "--output-dir", - str(tmp_path / "output"), - "--log-config-file", - str(log_config_file), - *missing_args, - ] - monkeypatch.setattr(sys, "argv", argv) + settings = settings_factory() + monkeypatch.setattr(sys, "argv", [PIPELINE_NAME, *settings_to_argv(settings, exclude=[missing_field])]) with ( patch.object(xmltodict_module, "run_pipeline") as mock_run_pipeline, - pytest.raises(Exception, match=expected_fragment), + pytest.raises(Exception, match=missing_field), ): cli() diff --git a/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_settings.py b/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_settings.py index ae0e53b6..61011a95 100644 --- a/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_settings.py +++ b/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_settings.py @@ -1,7 +1,6 @@ """Tests for XmlToDictSettings field validation and defaults.""" from collections.abc import Callable -from pathlib import Path import pytest from pydantic import ValidationError @@ -9,6 +8,7 @@ from cdm_data_loaders.core.fields import DEFAULTS from cdm_data_loaders.pipelines.xmltodict.settings import XmlToDictSettings +from tests.cdm_data_loaders.pipelines.helpers import settings_to_argv from tests.conftest import S3 @@ -100,33 +100,13 @@ def test_xmltodict_settings_pass_unknown_use_destination_rejected( @pytest.mark.parametrize("file_glob_arg", ["--file-glob", "-g"]) -def test_xmltodict_settings_pass_xml_tag_shortcut(tmp_path: Path, file_glob_arg: str) -> None: +def test_xmltodict_settings_pass_xml_tag_shortcut( + settings_factory: Callable[..., XmlToDictSettings], + file_glob_arg: str, +) -> None: """The file_glob field is exposed as both --file-glob and -g on the command line.""" - log_config_file = tmp_path / "logging.json" - log_config_file.write_text('{"version": 1}') - input_dir = tmp_path / "input" - input_dir.mkdir() - output_dir = tmp_path / "output" - output_dir.mkdir() - - argv = [ - "--input-dir", - str(input_dir), - "--output-dir", - str(output_dir), - "--use-destination", - "local_fs", - "--log-config-file", - str(log_config_file), - file_glob_arg, - "*.xml.gz", - "--dataset-name", - "cli_dataset", - "--table-name", - "entry", - "--xml-tag", - "entry", - ] + base_settings = settings_factory(xml_tag="entry") + argv = [*settings_to_argv(base_settings, exclude=["file_glob"]), file_glob_arg, "*.xml.gz"] settings = CliApp.run(XmlToDictSettings, cli_args=argv) assert settings.file_glob == "*.xml.gz" diff --git a/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py b/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py index 3dfd03f5..e7f07dce 100644 --- a/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py +++ b/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py @@ -2,7 +2,7 @@ from collections.abc import Callable from pathlib import Path -from typing import Any +from typing import Any, ClassVar from unittest.mock import patch import pyarrow as pa @@ -15,10 +15,13 @@ from cdm_data_loaders.core.fields import DEFAULTS from cdm_data_loaders.core.settings import CtsSettings from cdm_data_loaders.pipelines.xmltopyarrow.settings import ( + ENTRY_TABLE_NAME, + ENTRY_XML_TAG, UNIPROT_PIPELINE_NAME, UNIPROT_XSD_FILE, UNIREF_PIPELINE_NAME, UNIREF_XSD_FILE, + PresetXmlToPyArrowSettings, UniProtXmlToPyArrowSettings, UniRefXmlToPyArrowSettings, XmlToPyArrowSettings, @@ -257,3 +260,34 @@ def test_subclass_xmltopyarrow_settings_pass_cli_defaults( assert settings.xml_tag == "entry" assert settings.xsd_file == expected_xsd_file assert settings.pyarrow_schema is expected_schema + + +def test_preset_xmltopyarrow_settings_pass_base_defaults( + settings_factory: Callable[..., Any], + copy_xsd_fixture: Callable[[str], str], +) -> None: + """PresetXmlToPyArrowSettings subclasses inherit default table_name and xml_tag as 'entry' and use _PRECOMPUTED_SCHEMA.""" + + class CustomPresetSettings(PresetXmlToPyArrowSettings): + _PRECOMPUTED_SCHEMA: ClassVar[pa.Schema] = UNIREF_SCHEMA + + xsd_path = copy_xsd_fixture(UNIREF_LIKE_XSD) + settings = settings_factory( + CustomPresetSettings, + dataset_name="preset_ds", + xsd_file=xsd_path, + ) + assert ENTRY_TABLE_NAME == "entry" + assert ENTRY_XML_TAG == "entry" + assert settings.table_name == ENTRY_TABLE_NAME + assert settings.xml_tag == ENTRY_XML_TAG + assert settings.pyarrow_schema is UNIREF_SCHEMA + assert isinstance(settings.xsd_schema, xmlschema.XMLSchema) + + with pytest.raises(ValidationError, match="_PRECOMPUTED_SCHEMA"): + settings_factory( + PresetXmlToPyArrowSettings, + dataset_name="preset_ds", + xsd_file=xsd_path, + ) + diff --git a/tests/cdm_data_loaders/readers/xml/test_xml_parse_head_matter.py b/tests/cdm_data_loaders/readers/xml/test_xml_parse_head_matter.py index 70f8ed49..1b289a0f 100644 --- a/tests/cdm_data_loaders/readers/xml/test_xml_parse_head_matter.py +++ b/tests/cdm_data_loaders/readers/xml/test_xml_parse_head_matter.py @@ -207,10 +207,11 @@ def test_parse_head_matter_pass_body_malformation_after_root_is_tolerated( assert parse_head_matter(path) == {"": DEFAULT_NS_URI} -def test_parse_head_matter_fail_non_gzip_content_with_gz_extension_raises_error(tmp_path: Path) -> None: +def test_parse_head_matter_fail_non_gzip_content_with_gz_extension_raises_error( + xml_path_factory: Callable[..., Path], +) -> None: """Verify a file named `*.gz` but containing plain (non-gzip-compressed) text raises an error when read.""" - fake_gz_path = tmp_path / "not_really_gzipped.xml.gz" - fake_gz_path.write_text(DEFAULT_NAMESPACE_XML, encoding="utf-8") + fake_gz_path = xml_path_factory(DEFAULT_NAMESPACE_XML, gzip_compress=False, filename="not_really_gzipped.xml.gz") with pytest.raises(OSError, match="Not a gzipped file"): parse_head_matter(fake_gz_path) diff --git a/tests/cdm_data_loaders/readers/xml/test_xml_process_xml_file_and_batches.py b/tests/cdm_data_loaders/readers/xml/test_xml_process_xml_file_and_batches.py index fe3980c3..06756e40 100644 --- a/tests/cdm_data_loaders/readers/xml/test_xml_process_xml_file_and_batches.py +++ b/tests/cdm_data_loaders/readers/xml/test_xml_process_xml_file_and_batches.py @@ -1232,10 +1232,10 @@ def _raising_process(*_: Any, **__: Any) -> Iterator[DataItemWithMeta]: # noqa: def test_process_xml_file_to_dict_pass_skips_empty_xmltodict_result( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch + xml_path_factory: Callable[..., Path], monkeypatch: pytest.MonkeyPatch ) -> None: """Verify an element whose xmltodict.parse output is empty is skipped and not buffered.""" - file_path = _write_xml(tmp_path, "two.xml", PEOPLE_XML_2) + file_path = xml_path_factory(PEOPLE_XML_2, filename="two.xml") settings = fake_xmltodict_settings(table_name="people", xml_tag="person") original_parse = xmltodict.parse call_count = 0 @@ -1255,7 +1255,7 @@ def _parse_skipping_first(*args: Any, **kwargs: Any) -> dict[str, Any]: # noqa: def test_process_xml_file_to_pyarrow_pass_clark_notation_tag_skips_head_matter( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch + xml_path_factory: Callable[..., Path], monkeypatch: pytest.MonkeyPatch ) -> None: """Verify process_xml_file_to_pyarrow skips parse_head_matter when xml_tag is already in Clark notation.""" namespaced_xsd = """ @@ -1284,19 +1284,13 @@ def test_process_xml_file_to_pyarrow_pass_clark_notation_tag_skips_head_matter( Anne Example """ - xsd_path = _write_xml(tmp_path, "people_ns.xsd", namespaced_xsd) - xml_path = _write_xml(tmp_path, "people_ns.xml", namespaced_xml) + xml_path = xml_path_factory(namespaced_xml, filename="people_ns.xml") clark_tag = "{http://example.org/ns}person" - xsd_schema = xmlschema.XMLSchema(str(xsd_path)) - pyarrow_schema = generate_pyarrow_schema(xsd_schema, clark_tag) - settings = MagicMock(spec=XmlToPyArrowSettings) - settings.table_name = "people" - settings.xml_tag = clark_tag - settings.buffer_size = DEFAULTS[BUFFER_SIZE] - settings.log_interval = DEFAULTS[LOG_INTERVAL] - settings.xsd_schema = xsd_schema - settings.pyarrow_schema = pyarrow_schema - settings.max_recursion_depth = DEFAULT_MAX_RECURSION_DEPTH + settings = fake_xmltopyarrow_settings( + table_name="people", + xml_tag=clark_tag, + xsd_text=namespaced_xsd, + ) head_matter_mock = MagicMock() monkeypatch.setattr(xml_module, "parse_head_matter", head_matter_mock) From 2ca59626db128d8e0d8b485a196d8986e17ad90a Mon Sep 17 00:00:00 2001 From: ialarmedalien Date: Thu, 22 Oct 2026 11:34:32 -0700 Subject: [PATCH 3/4] formatting --- .../jsonlines/extract_pydantic_validate_pipeline.py | 4 +--- src/cdm_data_loaders/pipelines/jsonlines/settings.py | 6 +----- src/cdm_data_loaders/pipelines/ncbi_ftp_download.py | 4 +--- src/cdm_data_loaders/pipelines/xml2db/pipeline.py | 4 +--- src/cdm_data_loaders/pipelines/xmltodict/pipeline.py | 4 +--- src/cdm_data_loaders/pipelines/xmltopyarrow/pipeline.py | 4 +--- tests/cdm_data_loaders/core/test_settings.py | 1 - tests/cdm_data_loaders/pipelines/jsonlines/test_registry.py | 1 - tests/cdm_data_loaders/pipelines/test_core.py | 5 +---- .../pipelines/xmltopyarrow/test_xmltopyarrow_settings.py | 1 - 10 files changed, 7 insertions(+), 27 deletions(-) diff --git a/src/cdm_data_loaders/pipelines/jsonlines/extract_pydantic_validate_pipeline.py b/src/cdm_data_loaders/pipelines/jsonlines/extract_pydantic_validate_pipeline.py index 5e30cb7f..38f69a7c 100644 --- a/src/cdm_data_loaders/pipelines/jsonlines/extract_pydantic_validate_pipeline.py +++ b/src/cdm_data_loaders/pipelines/jsonlines/extract_pydantic_validate_pipeline.py @@ -60,9 +60,7 @@ def load_entity_models(module_path: str) -> dict[str, type[BaseModel]]: :rtype: dict[str, type[BaseModel]] :raises ValueError: if ENTITY_MODELS is missing, empty, or not a dict """ - return load_registry_dict( - module_path, "ENTITY_MODELS", type_hint=": dict[str, type[BaseModel]]" - ) + return load_registry_dict(module_path, "ENTITY_MODELS", type_hint=": dict[str, type[BaseModel]]") def _make_resolve_error(model: type[BaseModel]) -> ParseErrorResolver: diff --git a/src/cdm_data_loaders/pipelines/jsonlines/settings.py b/src/cdm_data_loaders/pipelines/jsonlines/settings.py index 59e0dde1..4963962d 100644 --- a/src/cdm_data_loaders/pipelines/jsonlines/settings.py +++ b/src/cdm_data_loaders/pipelines/jsonlines/settings.py @@ -86,11 +86,7 @@ class JsonlPydanticIngestSettings(ValidatedJsonlIngestSettings): cli_prog_name=PYDANTIC_PIPELINE_NAME, cli_shortcuts={ **FILE_FORMAT_CLI_SHORTCUTS, - **{ - k: v - for k, v in LOCAL_CLI_SHORTCUTS.items() - if k in [ENTITY_MODELS_MODULE, FILE_GLOB, TABLE_NAMES] - }, + **{k: v for k, v in LOCAL_CLI_SHORTCUTS.items() if k in [ENTITY_MODELS_MODULE, FILE_GLOB, TABLE_NAMES]}, }, ) diff --git a/src/cdm_data_loaders/pipelines/ncbi_ftp_download.py b/src/cdm_data_loaders/pipelines/ncbi_ftp_download.py index ad70da34..1b97ab40 100644 --- a/src/cdm_data_loaders/pipelines/ncbi_ftp_download.py +++ b/src/cdm_data_loaders/pipelines/ncbi_ftp_download.py @@ -178,9 +178,7 @@ def _attempt() -> dict[str, Any]: try: with ( - tqdm.tqdm( - total=len(assembly_paths), unit="assembly", desc=progress_desc, smoothing=0.01 - ) as pbar, + tqdm.tqdm(total=len(assembly_paths), unit="assembly", desc=progress_desc, smoothing=0.01) as pbar, ThreadPoolExecutor(max_workers=threads) as executor, ): futures = {executor.submit(_download_one, p): p for p in assembly_paths} diff --git a/src/cdm_data_loaders/pipelines/xml2db/pipeline.py b/src/cdm_data_loaders/pipelines/xml2db/pipeline.py index a31deca6..d4f2bef5 100644 --- a/src/cdm_data_loaders/pipelines/xml2db/pipeline.py +++ b/src/cdm_data_loaders/pipelines/xml2db/pipeline.py @@ -50,9 +50,7 @@ def run_xml2db_pipeline(settings: Xml2DbSettings) -> LoadInfo | None: model_config = load_config(str(settings.xml2db_config_file)) if settings.xml2db_config_file else None model = build_xml2db_model(settings.xsd_file, short_name=settings.short_name, model_config=model_config) - xml2db_resource = build_filesystem_resource( - settings, _read_items, settings, model, name="xml2db_reader" - ) + xml2db_resource = build_filesystem_resource(settings, _read_items, settings, model, name="xml2db_reader") load_info = run_pipeline( settings=settings, diff --git a/src/cdm_data_loaders/pipelines/xmltodict/pipeline.py b/src/cdm_data_loaders/pipelines/xmltodict/pipeline.py index a87dda43..4be85fd3 100644 --- a/src/cdm_data_loaders/pipelines/xmltodict/pipeline.py +++ b/src/cdm_data_loaders/pipelines/xmltodict/pipeline.py @@ -40,9 +40,7 @@ def run_xmltodict_pipeline(settings: XmlToDictSettings) -> LoadInfo | None: :param settings: pipeline configuration :type settings: XmlToDictSettings """ - xmltodict_resource = build_filesystem_resource( - settings, _read_xmltodict_items, settings, name="xmltodict_reader" - ) + xmltodict_resource = build_filesystem_resource(settings, _read_xmltodict_items, settings, name="xmltodict_reader") return run_pipeline( settings=settings, diff --git a/src/cdm_data_loaders/pipelines/xmltopyarrow/pipeline.py b/src/cdm_data_loaders/pipelines/xmltopyarrow/pipeline.py index ab1248ee..77e55c15 100644 --- a/src/cdm_data_loaders/pipelines/xmltopyarrow/pipeline.py +++ b/src/cdm_data_loaders/pipelines/xmltopyarrow/pipeline.py @@ -38,9 +38,7 @@ def run_xmltopyarrow_pipeline(settings: XmlToPyArrowSettings) -> LoadInfo | None :param settings: pipeline configuration :type settings: XmlToPyArrowSettings """ - xmltopyarrow_resource = build_filesystem_resource( - settings, _read_items, settings, name="xmltopyarrow_reader" - ) + xmltopyarrow_resource = build_filesystem_resource(settings, _read_items, settings, name="xmltopyarrow_reader") pipeline_name = str(settings.model_config.get("cli_prog_name") or PIPELINE_NAME) return run_pipeline( diff --git a/tests/cdm_data_loaders/core/test_settings.py b/tests/cdm_data_loaders/core/test_settings.py index 93be2af0..0e748642 100644 --- a/tests/cdm_data_loaders/core/test_settings.py +++ b/tests/cdm_data_loaders/core/test_settings.py @@ -291,4 +291,3 @@ def test_xml_ingest_settings_pass_defaults_and_cli_shortcuts( "file_glob": "*.xml.gz", }, ) - diff --git a/tests/cdm_data_loaders/pipelines/jsonlines/test_registry.py b/tests/cdm_data_loaders/pipelines/jsonlines/test_registry.py index 1cb90f52..ae847df6 100644 --- a/tests/cdm_data_loaders/pipelines/jsonlines/test_registry.py +++ b/tests/cdm_data_loaders/pipelines/jsonlines/test_registry.py @@ -104,4 +104,3 @@ def test_load_registry_dict_fail_invalid_attribute( monkeypatch.setitem(sys.modules, module_name, module) with pytest.raises(ValueError, match=r"^" + expected_message.replace("[", r"\[").replace("]", r"\]") + r"$"): load_registry_dict(module_name, "CUSTOM_REG", type_hint=type_hint) - diff --git a/tests/cdm_data_loaders/pipelines/test_core.py b/tests/cdm_data_loaders/pipelines/test_core.py index b2a563e6..d2df7282 100644 --- a/tests/cdm_data_loaders/pipelines/test_core.py +++ b/tests/cdm_data_loaders/pipelines/test_core.py @@ -958,9 +958,7 @@ def test_pipeline_cli_pass_run(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) @pytest.mark.usefixtures("restore_logging") -def test_pipeline_cli_pass_call_reads_argv( - dlt_test_settings: CtsSettings, monkeypatch: pytest.MonkeyPatch -) -> None: +def test_pipeline_cli_pass_call_reads_argv(dlt_test_settings: CtsSettings, monkeypatch: pytest.MonkeyPatch) -> None: """Calling the CLI parses sys.argv, runs the pipeline and returns None so the process exits with 0.""" received, pipeline_fn = make_recorder() monkeypatch.setattr(sys, "argv", ["prog", *settings_to_argv(dlt_test_settings)]) @@ -1702,4 +1700,3 @@ def test_build_file_ingest_kwargs_pass( "loader_file_format": "jsonl", }, } - diff --git a/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py b/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py index e7f07dce..f3d3db7f 100644 --- a/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py +++ b/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py @@ -290,4 +290,3 @@ class CustomPresetSettings(PresetXmlToPyArrowSettings): dataset_name="preset_ds", xsd_file=xsd_path, ) - From e6ca398ebc417c490bc9972319ec827855df8a5a Mon Sep 17 00:00:00 2001 From: ialarmedalien Date: Thu, 22 Oct 2026 14:17:48 -0700 Subject: [PATCH 4/4] Further reformatting --- src/cdm_data_loaders/pipelines/core.py | 14 ++- tests/cdm_data_loaders/conftest.py | 3 +- tests/cdm_data_loaders/core/test_settings.py | 90 ++++++++++++------- tests/cdm_data_loaders/pipelines/conftest.py | 4 +- tests/cdm_data_loaders/pipelines/framework.py | 3 +- tests/cdm_data_loaders/pipelines/helpers.py | 60 +------------ .../pipelines/jsonlines/conftest.py | 2 +- .../jsonlines/integration/conftest.py | 3 +- .../integration/test_jsonlines_end_to_end.py | 2 +- .../integration/test_jsonschema_end_to_end.py | 2 +- .../integration/test_reference_end_to_end.py | 3 +- .../jsonlines/test_extract_pipeline.py | 2 +- .../test_ncbi_rest_api_settings.py | 3 +- tests/cdm_data_loaders/pipelines/test_core.py | 34 +++---- .../pipelines/test_framework.py | 3 +- .../uniref_cdm/test_uniref_cdm_settings.py | 3 +- .../pipelines/xml2db/integration/conftest.py | 2 +- .../integration/test_xml2db_end_to_end.py | 2 +- .../pipelines/xml2db/test_xml2db_pipeline.py | 3 +- .../pipelines/xml2db/test_xml2db_settings.py | 2 +- .../pipelines/xmltodict/conftest.py | 2 +- .../xmltodict/integration/conftest.py | 2 +- .../integration/test_xmltodict_end_to_end.py | 2 +- .../test_xmltodict_reference_tests.py | 5 +- .../xmltodict/test_xmltodict_pipeline.py | 3 +- .../xmltodict/test_xmltodict_settings.py | 2 +- .../pipelines/xmltopyarrow/conftest.py | 2 +- .../xmltopyarrow/integration/conftest.py | 2 +- .../test_xmltopyarrow_end_to_end.py | 2 +- .../test_xmltopyarrow_pipeline.py | 3 +- .../test_xmltopyarrow_settings.py | 2 +- .../pipelines/xsv/conftest.py | 2 +- .../xsv/integration/test_xsv_end_to_end.py | 2 +- .../pipelines/xsv/test_xsv_pipeline.py | 2 +- tests/helpers.py | 60 ++++++++++++- tests/jsonschema_fixtures.py | 2 +- 36 files changed, 182 insertions(+), 153 deletions(-) diff --git a/src/cdm_data_loaders/pipelines/core.py b/src/cdm_data_loaders/pipelines/core.py index cb61d9f2..faa35c11 100644 --- a/src/cdm_data_loaders/pipelines/core.py +++ b/src/cdm_data_loaders/pipelines/core.py @@ -197,7 +197,19 @@ def save_pipeline_metadata( try: # save the load info as JSONL in the same destination as the ingested data load_info_resource: DltResource = dlt.resource( - [load_info.asdict()], name=LOAD_INFO_TABLE_NAME, max_table_nesting=0 + [load_info.asdict()], + name=LOAD_INFO_TABLE_NAME, + max_table_nesting=0, + columns={ + k: {"data_type": "text"} + for k in [ + "environment", + "staging_type", + "staging_name", + "staging_display_credentials", + "dataset_name", + ] + }, ) load_info_save_info: LoadInfo = pipeline.run(load_info_resource, loader_file_format=str(JSONL)) if load_info_save_info.has_failed_jobs: diff --git a/tests/cdm_data_loaders/conftest.py b/tests/cdm_data_loaders/conftest.py index eca938f2..e8f04408 100644 --- a/tests/cdm_data_loaders/conftest.py +++ b/tests/cdm_data_loaders/conftest.py @@ -20,6 +20,7 @@ USE_OUTPUT_DIR_FOR_PIPELINE_METADATA, ) from tests.conftest import LOCAL_FS +from tests.helpers import make_cli_arg def check_settings( @@ -52,7 +53,7 @@ def _make(settings_cls: type[BaseSettings], source: str, values: Mapping[str, st monkeypatch.setenv(f"{prefix}{name}".upper(), value) return settings_cls() if source == CLI: - cli_args = [arg for name, value in values.items() for arg in (f"--{name.replace('_', '-')}", value)] + cli_args = [arg for name, value in values.items() for arg in (make_cli_arg(name), value)] return CliApp.run(settings_cls, cli_args=cli_args) err_msg = f"Unknown settings source: {source!r}" raise ValueError(err_msg) diff --git a/tests/cdm_data_loaders/core/test_settings.py b/tests/cdm_data_loaders/core/test_settings.py index 0e748642..2c50617f 100644 --- a/tests/cdm_data_loaders/core/test_settings.py +++ b/tests/cdm_data_loaders/core/test_settings.py @@ -1,7 +1,7 @@ """Tests for the settings classes and helpers in cdm_data_loaders.core.settings.""" from pathlib import Path -from typing import Any, Final +from typing import Any import pytest from frozendict import frozendict @@ -27,10 +27,45 @@ cts_expected, local_io_expected, ) -from tests.cdm_data_loaders.pipelines.helpers import settings_to_argv -from tests.conftest import LOCAL_FS +from tests.helpers import ( + assert_cli_field_roundtrips, + assert_no_cli_clashes, + settings_to_argv, +) + + +@pytest.mark.parametrize( + "settings_cls", + [LoggerSettings, LocalInputOutputSettings, CtsSettings, FileFormatIngestSettings, XmlIngestSettings], +) +def test_settings_pass_no_cli_clashes(settings_cls: type[CdmDataLoadersBase]) -> None: + """Every core settings class has no CLI argument or alias collisions.""" + assert_no_cli_clashes(settings_cls) + -TEST_CLASS_NAME: Final[str] = "FruitSettings" +def test_cts_settings_pass_all_params_set( + test_cts_settings: frozendict[str, Any], + test_cts_settings_reconciled: frozendict[str, Any], +) -> None: + """CtsSettings populated with test_cts_settings matches test_cts_settings_reconciled.""" + check_settings(CtsSettings(**test_cts_settings), test_cts_settings_reconciled) + + +@pytest.mark.parametrize("field_name", list(CtsSettings.model_fields)) +def test_cts_settings_pass_cli_field_roundtrips( + field_name: str, + test_cts_settings: frozendict[str, Any], + test_cts_settings_reconciled: frozendict[str, Any], + test_defaults: dict[str, Any], +) -> None: + """Every registered CLI option and shortcut for each CtsSettings field roundtrips to the expected value.""" + assert_cli_field_roundtrips( + CtsSettings, + field_name, + test_cts_settings[field_name], + test_cts_settings_reconciled[field_name], + test_defaults, + ) @pytest.mark.parametrize( @@ -119,9 +154,9 @@ def test_local_input_output_settings_pass_cli_shortcuts(input_dir: Path, output_ @pytest.mark.parametrize("source", SETTINGS_SOURCES) @pytest.mark.parametrize( - ("values", "overrides", "pipeline_dir_name"), + ("values", "overrides"), [ - pytest.param({"use_destination": "cts"}, {"use_destination": "cts"}, None, id="defaults"), + pytest.param({"use_destination": "cts"}, {"use_destination": "cts"}, id="defaults"), pytest.param( { "log_config_file": "logging.yaml", @@ -139,7 +174,6 @@ def test_local_input_output_settings_pass_cli_shortcuts(input_dir: Path, output_ "use_destination": "s3", "use_output_dir_for_pipeline_metadata": True, }, - ".dlt_conf", id="all_fields_set", ), ], @@ -149,16 +183,18 @@ def test_cts_settings_pass_sources( source: str, values: dict[str, str], overrides: dict[str, Any], - pipeline_dir_name: str | None, - input_dir: Path, - output_dir: Path, + test_defaults: dict[str, Any], ) -> None: """Build CtsSettings from each source and check every field, including the computed pipeline_dir.""" settings = make_settings( - CtsSettings, source, {"input_dir": str(input_dir), "output_dir": str(output_dir), **values} + CtsSettings, + source, + {**{k: str(v) for k, v in test_defaults.items()}, **values}, ) - pipeline_dir = None if pipeline_dir_name is None else output_dir.resolve() / pipeline_dir_name - check_settings(settings, cts_expected(input_dir, output_dir, **overrides, pipeline_dir=pipeline_dir)) + pipeline_dir = ( + test_defaults["output_dir"] / ".dlt_conf" if overrides.get("use_output_dir_for_pipeline_metadata") else None + ) + check_settings(settings, cts_expected(**{**test_defaults, **overrides}, pipeline_dir=pipeline_dir)) def test_cts_settings_pass_cli_shortcuts(input_dir: Path, output_dir: Path) -> None: @@ -186,9 +222,9 @@ def test_cts_settings_pass_cli_shortcuts(input_dir: Path, output_dir: Path) -> N pytest.param(["--buffer_size", "5"], id="snake_case_arg"), ], ) -def test_cts_settings_fail_unrecognised_cli_args(input_dir: Path, output_dir: Path, extra_args: list[str]) -> None: +def test_cts_settings_fail_unrecognised_cli_args(test_defaults: dict[str, Any], extra_args: list[str]) -> None: """Raise SettingsError, not SystemExit, for CLI args that are not registered.""" - base = CtsSettings(input_dir=input_dir, output_dir=output_dir, use_destination=LOCAL_FS) + base = CtsSettings(**test_defaults) with pytest.raises(SettingsError, match="unrecognized arguments"): CliApp.run(CtsSettings, cli_args=[*settings_to_argv(base), *extra_args]) @@ -221,20 +257,14 @@ def test_settings_fail_missing_dirs( assert {(err["loc"], err["type"]) for err in exc_info.value.errors()} == expected -def test_file_format_ingest_settings_pass_cli_shortcuts(input_dir: Path, output_dir: Path) -> None: +def test_file_format_ingest_settings_pass_cli_shortcuts(test_defaults: dict[str, Any]) -> None: """FileFormatIngestSettings supports FILE_FORMAT_CLI_SHORTCUTS (-f) in addition to CtsSettings shortcuts.""" assert FILE_FORMAT_CLI_SHORTCUTS["loader_file_format"] == "f" + base = FileFormatIngestSettings(**test_defaults, dataset_name="my_dataset") settings = CliApp.run( FileFormatIngestSettings, cli_args=[ - "-i", - str(input_dir), - "-o", - str(output_dir), - "-d", - LOCAL_FS, - "--dataset-name", - "my_dataset", + *settings_to_argv(base, exclude=["loader_file_format"]), "-f", "jsonl", ], @@ -242,7 +272,7 @@ def test_file_format_ingest_settings_pass_cli_shortcuts(input_dir: Path, output_ check_settings( settings, { - **cts_expected(input_dir, output_dir, use_destination=LOCAL_FS, dataset_name="my_dataset"), + **cts_expected(**test_defaults, dataset_name="my_dataset"), "loader_file_format": "jsonl", }, ) @@ -250,7 +280,7 @@ def test_file_format_ingest_settings_pass_cli_shortcuts(input_dir: Path, output_ @pytest.mark.parametrize("source", SETTINGS_SOURCES) def test_xml_ingest_settings_pass_defaults_and_cli_shortcuts( - make_settings: SettingsFactory, source: str, input_dir: Path, output_dir: Path + make_settings: SettingsFactory, source: str, test_defaults: dict[str, Any] ) -> None: """XmlIngestSettings defaults file_glob to '*.xml*' across sources and supports XML_CLI_SHORTCUTS (-f, -g).""" assert XML_CLI_SHORTCUTS["loader_file_format"] == "f" @@ -259,16 +289,14 @@ def test_xml_ingest_settings_pass_defaults_and_cli_shortcuts( XmlIngestSettings, source, { - "input_dir": str(input_dir), - "output_dir": str(output_dir), - "use_destination": LOCAL_FS, + **{k: str(v) for k, v in test_defaults.items()}, "dataset_name": "xml_dataset", }, ) check_settings( default_glob_settings, { - **cts_expected(input_dir, output_dir, use_destination=LOCAL_FS, dataset_name="xml_dataset"), + **cts_expected(**test_defaults, dataset_name="xml_dataset"), "loader_file_format": "parquet", "file_glob": "*.xml*", }, @@ -286,7 +314,7 @@ def test_xml_ingest_settings_pass_defaults_and_cli_shortcuts( check_settings( cli_settings, { - **cts_expected(input_dir, output_dir, use_destination=LOCAL_FS, dataset_name="xml_dataset"), + **cts_expected(**test_defaults, dataset_name="xml_dataset"), "loader_file_format": "jsonl", "file_glob": "*.xml.gz", }, diff --git a/tests/cdm_data_loaders/pipelines/conftest.py b/tests/cdm_data_loaders/pipelines/conftest.py index c7ad2963..2cf8d725 100644 --- a/tests/cdm_data_loaders/pipelines/conftest.py +++ b/tests/cdm_data_loaders/pipelines/conftest.py @@ -26,13 +26,11 @@ PipelineTestRunner, ensure_docker_image, ) -from tests.cdm_data_loaders.pipelines.helpers import ( - make_cts_settings, -) from tests.cdm_data_loaders.pipelines.helpers import ( read_destination_rows as _read_destination_rows, ) from tests.conftest import LOCAL_FS, REFERENCE_XML_FIXTURE_DIR, S3, _generate_dlt_config +from tests.helpers import make_cts_settings DEFAULT_DLT_TABLES = {"_dlt_version", "_dlt_loads", "_dlt_pipeline_state", LOAD_INFO_TABLE_NAME} diff --git a/tests/cdm_data_loaders/pipelines/framework.py b/tests/cdm_data_loaders/pipelines/framework.py index 5f26debe..0c311c2f 100644 --- a/tests/cdm_data_loaders/pipelines/framework.py +++ b/tests/cdm_data_loaders/pipelines/framework.py @@ -25,8 +25,9 @@ from cdm_data_loaders.pipelines import core from cdm_data_loaders.pipelines.core import LOAD_INFO_TABLE_NAME, PipelineCli from cdm_data_loaders.utils import cdm_logger -from tests.cdm_data_loaders.pipelines.helpers import read_jsonl_file, settings_to_argv +from tests.cdm_data_loaders.pipelines.helpers import read_jsonl_file from tests.conftest import BASE_DIR, CONFIG_BUCKET, LOCAL_FS +from tests.helpers import settings_to_argv DEFAULT_DOCKER_IMAGE_TAG: Final[str] = "cdm-data-loaders-test:latest" CONTAINER_INPUT_DIR: Final[str] = "/input_dir" diff --git a/tests/cdm_data_loaders/pipelines/helpers.py b/tests/cdm_data_loaders/pipelines/helpers.py index 2691667e..af8abd50 100644 --- a/tests/cdm_data_loaders/pipelines/helpers.py +++ b/tests/cdm_data_loaders/pipelines/helpers.py @@ -3,7 +3,7 @@ import gzip import json from collections import defaultdict -from collections.abc import Callable, Generator, Iterable +from collections.abc import Callable, Generator from contextlib import contextmanager from datetime import datetime from logging import Logger, getLogger @@ -19,9 +19,7 @@ from dlt.destinations import filesystem from cdm_data_loaders.core.fields import PARQUET, LoaderFileFormatEnum -from cdm_data_loaders.core.settings import CtsSettings from cdm_data_loaders.pipelines.core import LOAD_INFO_TABLE_NAME -from tests.conftest import LOCAL_FS logger: Logger = getLogger(__name__) @@ -114,32 +112,6 @@ def read_destination_rows(base_dir: Path, table_name: str) -> list[dict[str, Any return [strip_metadata_columns(row) for row in rows] -def make_cts_settings[S: CtsSettings]( - settings_cls: type[S], - base_dir: Path, - **overrides: Any, # noqa: ANN401 -) -> S: - """Create standard input/output directories and log config under base_dir and instantiate settings_cls.""" - input_dir = base_dir / "input" - input_dir.mkdir(parents=True, exist_ok=True) - output_dir = base_dir / "output" - output_dir.mkdir(parents=True, exist_ok=True) - log_config_file = base_dir / "logging.json" - if not log_config_file.exists(): - log_config_file.write_text('{"version": 1}', encoding="utf-8") - - kwargs: dict[str, Any] = { - "input_dir": str(input_dir), - "output_dir": str(output_dir), - "log_config_file": str(log_config_file), - "use_destination": LOCAL_FS, - "dlt_dev_mode": False, - "use_output_dir_for_pipeline_metadata": False, - } - kwargs.update(overrides) - return settings_cls(**kwargs) - - def make_file_item(file_path: Path, mime_type: str = "application/octet-stream") -> FileItemDict: """Build a dlt FileItemDict for a local file.""" return FileItemDict( @@ -182,36 +154,6 @@ def sorted_json(entries: list[dict[str, Any]]) -> list[str]: return sorted(json.dumps(entry, sort_keys=True, default=str) for entry in entries) -def settings_to_argv(settings: Any, *, exclude: Iterable[str] = ()) -> list[str]: # noqa: ANN401 - """Derive CLI arguments from a settings instance. - - Every serialized field becomes a ``--kebab-case`` flag, with bools rendered - as their ``true``/``false`` CLI spelling, sequence fields expanded after the - flag, and None-valued, excluded, or computed fields skipped. - - :param settings: a CtsSettings (or derivative) instance - :type settings: Any - :param exclude: field names to omit from the resulting argument list - :type exclude: Iterable[str] - :return: command-line argument list reproducing the settings - :rtype: list[str] - """ - argv: list[str] = [] - excluded_fields: set[str] = set(getattr(type(settings), "model_computed_fields", {}).keys()) | set(exclude) - for name, value in settings.model_dump(exclude=excluded_fields).items(): - flag = f"--{name.replace('_', '-')}" - if isinstance(value, bool): - argv.extend([flag, "true" if value else "false"]) - elif value is None: - continue - elif isinstance(value, list | tuple): - if value: - argv.extend([flag, *(str(item) for item in value)]) - else: - argv.extend([flag, str(value)]) - return argv - - def assert_run_ok(load_info: LoadInfo | None) -> LoadInfo: """Assert a pipeline run succeeded, and return the load info for chained use.""" assert load_info is not None diff --git a/tests/cdm_data_loaders/pipelines/jsonlines/conftest.py b/tests/cdm_data_loaders/pipelines/jsonlines/conftest.py index c7bc8c2d..ba1d5726 100644 --- a/tests/cdm_data_loaders/pipelines/jsonlines/conftest.py +++ b/tests/cdm_data_loaders/pipelines/jsonlines/conftest.py @@ -8,7 +8,7 @@ import pytest from cdm_data_loaders.pipelines.jsonlines.settings import JsonlExtractSettings, JsonlPydanticIngestSettings -from tests.cdm_data_loaders.pipelines.helpers import make_cts_settings +from tests.helpers import make_cts_settings def make_item( diff --git a/tests/cdm_data_loaders/pipelines/jsonlines/integration/conftest.py b/tests/cdm_data_loaders/pipelines/jsonlines/integration/conftest.py index 2387264b..9eb1e98c 100644 --- a/tests/cdm_data_loaders/pipelines/jsonlines/integration/conftest.py +++ b/tests/cdm_data_loaders/pipelines/jsonlines/integration/conftest.py @@ -25,13 +25,14 @@ JsonlJsonschemaIngestSettings, JsonlPydanticIngestSettings, ) -from tests.cdm_data_loaders.pipelines.helpers import duckdb_reference_from_jsonl, make_cts_settings +from tests.cdm_data_loaders.pipelines.helpers import duckdb_reference_from_jsonl from tests.cdm_data_loaders.pipelines.jsonlines.integration.dataset_report_models import ENTITY_MODELS from tests.cdm_data_loaders.pipelines.jsonlines.integration.dataset_report_schemas import ENTITY_SCHEMAS from tests.cdm_data_loaders.pipelines.jsonlines.integration.jsonlines_reference_helpers import ( canonicalize_entry, ) from tests.conftest import TEST_DATA_DIR +from tests.helpers import make_cts_settings type PipelineRunner = Callable[..., tuple[LoadInfo | None, Path]] type PrepareReference = Callable[[dict[str, Any]], dict[str, Any]] diff --git a/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonlines_end_to_end.py b/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonlines_end_to_end.py index 8b62a569..f0ec6733 100644 --- a/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonlines_end_to_end.py +++ b/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonlines_end_to_end.py @@ -13,8 +13,8 @@ from tests.cdm_data_loaders.pipelines.helpers import ( assert_pipeline_report_written, assert_run_ok, - settings_to_argv, ) +from tests.helpers import settings_to_argv def test_run_jsonlines_pipeline_pass_writes_expected_parquet( diff --git a/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonschema_end_to_end.py b/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonschema_end_to_end.py index e5507091..39d3d31c 100644 --- a/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonschema_end_to_end.py +++ b/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_jsonschema_end_to_end.py @@ -16,8 +16,8 @@ assert_run_ok, parse_json_string, read_data_rows, - settings_to_argv, ) +from tests.helpers import settings_to_argv pytestmark = pytest.mark.parametrize("loader_file_format", LOADER_FILE_FORMATS, ids=str) diff --git a/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_reference_end_to_end.py b/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_reference_end_to_end.py index 3ef5aa74..b2d28a8c 100644 --- a/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_reference_end_to_end.py +++ b/tests/cdm_data_loaders/pipelines/jsonlines/integration/test_reference_end_to_end.py @@ -19,15 +19,14 @@ from tests.cdm_data_loaders.pipelines.helpers import ( LOADER_FILE_FORMATS, assert_run_ok, - make_cts_settings, read_pipeline_tables, run_and_read, - settings_to_argv, ) from tests.cdm_data_loaders.pipelines.jsonlines.integration.jsonlines_reference_helpers import ( canonicalize_entry, reconstructed_entries, ) +from tests.helpers import make_cts_settings, settings_to_argv pytestmark = pytest.mark.parametrize("loader_file_format", LOADER_FILE_FORMATS, ids=str) diff --git a/tests/cdm_data_loaders/pipelines/jsonlines/test_extract_pipeline.py b/tests/cdm_data_loaders/pipelines/jsonlines/test_extract_pipeline.py index 25ed1c9f..a4a9b4fb 100644 --- a/tests/cdm_data_loaders/pipelines/jsonlines/test_extract_pipeline.py +++ b/tests/cdm_data_loaders/pipelines/jsonlines/test_extract_pipeline.py @@ -19,8 +19,8 @@ from tests.cdm_data_loaders.pipelines.helpers import ( assert_pipeline_report_written, assert_run_ok, - settings_to_argv, ) +from tests.helpers import settings_to_argv SIMPLE_JSONL = """{"widget_id": "a", "count": 1} {"widget_id": "b", "count": 2} diff --git a/tests/cdm_data_loaders/pipelines/ncbi_rest_api/test_ncbi_rest_api_settings.py b/tests/cdm_data_loaders/pipelines/ncbi_rest_api/test_ncbi_rest_api_settings.py index de89c758..3e30d3ed 100644 --- a/tests/cdm_data_loaders/pipelines/ncbi_rest_api/test_ncbi_rest_api_settings.py +++ b/tests/cdm_data_loaders/pipelines/ncbi_rest_api/test_ncbi_rest_api_settings.py @@ -19,8 +19,7 @@ set_settings, ) from tests.cdm_data_loaders.conftest import check_settings -from tests.cdm_data_loaders.pipelines.helpers import settings_to_argv -from tests.helpers import assert_cli_field_roundtrips, assert_no_cli_clashes +from tests.helpers import assert_cli_field_roundtrips, assert_no_cli_clashes, settings_to_argv BATCH_SIZE = 200 BATCH_SIZE_STRING = str(BATCH_SIZE) diff --git a/tests/cdm_data_loaders/pipelines/test_core.py b/tests/cdm_data_loaders/pipelines/test_core.py index d2df7282..d986d96b 100644 --- a/tests/cdm_data_loaders/pipelines/test_core.py +++ b/tests/cdm_data_loaders/pipelines/test_core.py @@ -48,16 +48,13 @@ write_pipeline_report, ) from tests.cdm_data_loaders.pipelines.conftest import HOOK_ENV_VAR -from tests.cdm_data_loaders.pipelines.helpers import ( - make_cts_settings as make_pipeline_cts_settings, -) from tests.cdm_data_loaders.pipelines.helpers import ( read_destination_rows, read_jsonl_file, - settings_to_argv, ) from tests.conftest import LOCAL_FS, S3 from tests.dlt_config_isolation import dlt_config_unset, isolated_dlt_config +from tests.helpers import make_cts_settings, settings_to_argv TINY_RESOURCE_DATA: Final[list[dict[str, Any]]] = [ {"entity_id": "tiny:one", "value": 1}, @@ -74,19 +71,13 @@ SETTINGS_CLASSES: Final[list[type[CdmDataLoadersBase]]] = [LoggerSettings, LocalInputOutputSettings, CtsSettings] -def make_cts_settings(**kwargs: str | int | bool | Path) -> CtsSettings: - """Generate a validated CtsSettings object.""" - return CtsSettings.model_validate(kwargs) - - @pytest.fixture def dlt_test_settings(tmp_path: Path, dlt_destination_config: str) -> CtsSettings: """Settings that load into the local_fs destination and keep pipeline metadata in a separate output_dir.""" - input_dir = tmp_path / "input" - input_dir.mkdir(exist_ok=True, parents=True) return make_cts_settings( - input_dir=str(tmp_path / "input"), - output_dir=str(tmp_path / "output"), + CtsSettings, + tmp_path, + log_config_file=None, use_destination=dlt_destination_config, use_output_dir_for_pipeline_metadata=True, ) @@ -423,8 +414,8 @@ def test_run_cli_pipeline_fn_not_called_on_settings_instantiation_error( def test_run_pipeline_builds_destination_from_settings(mock_dlt: MagicMock, tmp_path: Path) -> None: """The destination comes from settings.use_destination only; output_dir has no effect on it.""" settings = make_cts_settings( - input_dir=tmp_path, - output_dir=tmp_path / "output", + CtsSettings, + tmp_path, use_destination=S3, dlt_dev_mode=True, ) @@ -449,7 +440,7 @@ def test_run_pipeline_forwards_destination_kwargs( mock_dlt: MagicMock, use_destination: str, destination_kwargs: dict[str, Any], tmp_path: Path ) -> None: """destination_kwargs are forwarded to the dlt.destination constructor.""" - settings = make_cts_settings(input_dir=tmp_path, output_dir=tmp_path, use_destination=use_destination) + settings = make_cts_settings(CtsSettings, tmp_path, use_destination=use_destination) run_pipeline(settings, MagicMock(), destination_kwargs=dict(destination_kwargs)) @@ -459,7 +450,7 @@ def test_run_pipeline_forwards_destination_kwargs( def test_run_pipeline_fail_destination_not_configured(mock_dlt: MagicMock, tmp_path: Path) -> None: """An unconfigured use_destination raises ValueError before the pipeline runs.""" mock_dlt.destination.side_effect = ValueError("not configured") - settings = make_cts_settings(input_dir=tmp_path, output_dir=tmp_path, use_destination="missing_dest") + settings = make_cts_settings(CtsSettings, tmp_path, use_destination="missing_dest") with pytest.raises(ValueError, match=r"^Invalid destination configuration for missing_dest$"): run_pipeline(settings, MagicMock()) @@ -1010,11 +1001,10 @@ def test_run_pipeline_pass_failed_jobs_reported( """With raise_on_failed_jobs off, failed jobs are returned in the LoadInfo and Slack says 'with errors'.""" monkeypatch.setenv("LOAD__RAISE_ON_FAILED_JOBS", "false") monkeypatch.setenv(HOOK_ENV_VAR, SLACK_HOOK) - input_dir = tmp_path / "input" - input_dir.mkdir(exist_ok=True) settings = make_cts_settings( - input_dir=str(input_dir), - output_dir=str(tmp_path / "output"), + CtsSettings, + tmp_path, + log_config_file=None, use_destination=FAILING_SINK, use_output_dir_for_pipeline_metadata=True, ) @@ -1676,7 +1666,7 @@ def test_build_file_ingest_kwargs_pass( expected_dataset_name: str | None, ) -> None: """build_file_ingest_kwargs constructs pipeline_kwargs and pipeline_run_kwargs from FileFormatIngestSettings.""" - base_settings: FileFormatIngestSettings = make_pipeline_cts_settings( + base_settings: FileFormatIngestSettings = make_cts_settings( FileFormatIngestSettings, tmp_path, dataset_name="custom_ds", diff --git a/tests/cdm_data_loaders/pipelines/test_framework.py b/tests/cdm_data_loaders/pipelines/test_framework.py index 03fa804d..3016e855 100644 --- a/tests/cdm_data_loaders/pipelines/test_framework.py +++ b/tests/cdm_data_loaders/pipelines/test_framework.py @@ -36,11 +36,10 @@ from tests.cdm_data_loaders.pipelines.helpers import ( assert_pipeline_report_written, assert_run_ok, - make_cts_settings, read_data_rows, - settings_to_argv, ) from tests.conftest import LOCAL_FS +from tests.helpers import make_cts_settings, settings_to_argv class _SampleEntityModel(BaseModel): diff --git a/tests/cdm_data_loaders/pipelines/uniref_cdm/test_uniref_cdm_settings.py b/tests/cdm_data_loaders/pipelines/uniref_cdm/test_uniref_cdm_settings.py index 9623f804..73d94591 100644 --- a/tests/cdm_data_loaders/pipelines/uniref_cdm/test_uniref_cdm_settings.py +++ b/tests/cdm_data_loaders/pipelines/uniref_cdm/test_uniref_cdm_settings.py @@ -14,11 +14,10 @@ UnirefSettings, ) from tests.cdm_data_loaders.conftest import check_settings -from tests.cdm_data_loaders.pipelines.helpers import settings_to_argv from tests.cdm_data_loaders.pipelines.uniref_cdm.conftest import ( UNIREF_VARIANT_ALIASES, ) -from tests.helpers import assert_cli_field_roundtrips, assert_no_cli_clashes, make_cli_arg +from tests.helpers import assert_cli_field_roundtrips, assert_no_cli_clashes, make_cli_arg, settings_to_argv def test_uniref_settings_all_params_set( diff --git a/tests/cdm_data_loaders/pipelines/xml2db/integration/conftest.py b/tests/cdm_data_loaders/pipelines/xml2db/integration/conftest.py index 34b09b33..5ddae9a1 100644 --- a/tests/cdm_data_loaders/pipelines/xml2db/integration/conftest.py +++ b/tests/cdm_data_loaders/pipelines/xml2db/integration/conftest.py @@ -10,8 +10,8 @@ from cdm_data_loaders.pipelines.xml2db.pipeline import run_xml2db_pipeline from cdm_data_loaders.pipelines.xml2db.settings import Xml2DbSettings -from tests.cdm_data_loaders.pipelines.helpers import make_cts_settings from tests.conftest import REFERENCE_XSD +from tests.helpers import make_cts_settings from tests.xml_samples import LIBRARY_XSD DEFAULT_XML2DB_SETTINGS: frozendict = frozendict( diff --git a/tests/cdm_data_loaders/pipelines/xml2db/integration/test_xml2db_end_to_end.py b/tests/cdm_data_loaders/pipelines/xml2db/integration/test_xml2db_end_to_end.py index 0b4d715d..d48fad4b 100644 --- a/tests/cdm_data_loaders/pipelines/xml2db/integration/test_xml2db_end_to_end.py +++ b/tests/cdm_data_loaders/pipelines/xml2db/integration/test_xml2db_end_to_end.py @@ -12,8 +12,8 @@ from tests.cdm_data_loaders.pipelines.helpers import ( assert_pipeline_report_written, assert_run_ok, - settings_to_argv, ) +from tests.helpers import settings_to_argv from tests.xml_samples import SIMPLE_LIBRARY_XML EXPECTED_BOOK_COUNT = 3 diff --git a/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_pipeline.py b/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_pipeline.py index a3d390af..3d514623 100644 --- a/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_pipeline.py +++ b/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_pipeline.py @@ -21,10 +21,9 @@ from tests.cdm_data_loaders.pipelines.helpers import ( assert_run_ok, fetch_all_duckdb, - make_cts_settings, make_file_item, - settings_to_argv, ) +from tests.helpers import make_cts_settings, settings_to_argv SIMPLE_LIBRARY_XML = """ diff --git a/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_settings.py b/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_settings.py index 383574d2..6ebb6f8b 100644 --- a/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_settings.py +++ b/tests/cdm_data_loaders/pipelines/xml2db/test_xml2db_settings.py @@ -15,8 +15,8 @@ PIPELINE_NAME, Xml2DbSettings, ) -from tests.cdm_data_loaders.pipelines.helpers import make_cts_settings, settings_to_argv from tests.conftest import S3 +from tests.helpers import make_cts_settings, settings_to_argv MINIMAL_SETTINGS_KWARGS: dict[str, object] = { "buffer_size": 100, diff --git a/tests/cdm_data_loaders/pipelines/xmltodict/conftest.py b/tests/cdm_data_loaders/pipelines/xmltodict/conftest.py index dfa561bc..3b8bd0c8 100644 --- a/tests/cdm_data_loaders/pipelines/xmltodict/conftest.py +++ b/tests/cdm_data_loaders/pipelines/xmltodict/conftest.py @@ -7,7 +7,7 @@ import pytest from cdm_data_loaders.pipelines.xmltodict.settings import XmlToDictSettings -from tests.cdm_data_loaders.pipelines.helpers import make_cts_settings +from tests.helpers import make_cts_settings @pytest.fixture diff --git a/tests/cdm_data_loaders/pipelines/xmltodict/integration/conftest.py b/tests/cdm_data_loaders/pipelines/xmltodict/integration/conftest.py index 1a47b253..cac7b306 100644 --- a/tests/cdm_data_loaders/pipelines/xmltodict/integration/conftest.py +++ b/tests/cdm_data_loaders/pipelines/xmltodict/integration/conftest.py @@ -17,10 +17,10 @@ from cdm_data_loaders.readers.xml import DEFAULT_XMLTODICT_ARGS from tests.cdm_data_loaders.pipelines.helpers import ( duckdb_reference_from_jsonl, - make_cts_settings, read_jsonl_file, ) from tests.conftest import N_REFERENCE_XML_ENTRIES, REFERENCE_XML_TAG +from tests.helpers import make_cts_settings DEFAULT_XMLTODICT_SETTINGS: frozendict = frozendict( { diff --git a/tests/cdm_data_loaders/pipelines/xmltodict/integration/test_xmltodict_end_to_end.py b/tests/cdm_data_loaders/pipelines/xmltodict/integration/test_xmltodict_end_to_end.py index 4db0e66d..e3641a1b 100644 --- a/tests/cdm_data_loaders/pipelines/xmltodict/integration/test_xmltodict_end_to_end.py +++ b/tests/cdm_data_loaders/pipelines/xmltodict/integration/test_xmltodict_end_to_end.py @@ -14,8 +14,8 @@ LOADER_FILE_FORMATS, assert_pipeline_report_written, assert_run_ok, - settings_to_argv, ) +from tests.helpers import settings_to_argv from tests.xml_samples import SIMPLE_LIBRARY_XML diff --git a/tests/cdm_data_loaders/pipelines/xmltodict/integration/test_xmltodict_reference_tests.py b/tests/cdm_data_loaders/pipelines/xmltodict/integration/test_xmltodict_reference_tests.py index 9727c356..2fafd8de 100644 --- a/tests/cdm_data_loaders/pipelines/xmltodict/integration/test_xmltodict_reference_tests.py +++ b/tests/cdm_data_loaders/pipelines/xmltodict/integration/test_xmltodict_reference_tests.py @@ -60,8 +60,9 @@ def test_xml_pass_reference_dataset_has_nested_structures( member_type = entry_fields["representativeMember"] assert property_type.id == "list" assert member_type.id == "struct" - count = reference_xml_table.count("*").fetchone()[0] - assert count == EXPECTED_ENTRY_COUNT + + count = reference_xml_dataset.execute("SELECT COUNT(*) FROM reference").fetchone() + assert count == (EXPECTED_ENTRY_COUNT,) def test_xml_pass_diff_format_same_results(run_xmltodict_pipeline_fxt: Callable[..., Any]) -> None: diff --git a/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_pipeline.py b/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_pipeline.py index b3ab9399..70b902bb 100644 --- a/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_pipeline.py +++ b/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_pipeline.py @@ -20,7 +20,8 @@ ) from cdm_data_loaders.pipelines.xmltodict.settings import PIPELINE_NAME, XmlToDictSettings from tests.cdm_data_loaders.pipelines.conftest import duckdb_pipeline -from tests.cdm_data_loaders.pipelines.helpers import assert_run_ok, fetch_all_duckdb, settings_to_argv +from tests.cdm_data_loaders.pipelines.helpers import assert_run_ok, fetch_all_duckdb +from tests.helpers import settings_to_argv from tests.xml_samples import TWO_BOOK_LIBRARY_XML diff --git a/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_settings.py b/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_settings.py index 61011a95..a44fd608 100644 --- a/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_settings.py +++ b/tests/cdm_data_loaders/pipelines/xmltodict/test_xmltodict_settings.py @@ -8,8 +8,8 @@ from cdm_data_loaders.core.fields import DEFAULTS from cdm_data_loaders.pipelines.xmltodict.settings import XmlToDictSettings -from tests.cdm_data_loaders.pipelines.helpers import settings_to_argv from tests.conftest import S3 +from tests.helpers import settings_to_argv def test_xmltodict_settings_pass_defaults(settings_factory: Callable[..., XmlToDictSettings]) -> None: diff --git a/tests/cdm_data_loaders/pipelines/xmltopyarrow/conftest.py b/tests/cdm_data_loaders/pipelines/xmltopyarrow/conftest.py index 67deda9d..7d169ea9 100644 --- a/tests/cdm_data_loaders/pipelines/xmltopyarrow/conftest.py +++ b/tests/cdm_data_loaders/pipelines/xmltopyarrow/conftest.py @@ -9,7 +9,7 @@ from cdm_data_loaders.core.settings import CtsSettings from cdm_data_loaders.pipelines.xmltopyarrow.settings import XmlToPyArrowSettings -from tests.cdm_data_loaders.pipelines.helpers import make_cts_settings +from tests.helpers import make_cts_settings from tests.xml_samples import LIBRARY_XSD diff --git a/tests/cdm_data_loaders/pipelines/xmltopyarrow/integration/conftest.py b/tests/cdm_data_loaders/pipelines/xmltopyarrow/integration/conftest.py index c92fc94f..3f9d0d12 100644 --- a/tests/cdm_data_loaders/pipelines/xmltopyarrow/integration/conftest.py +++ b/tests/cdm_data_loaders/pipelines/xmltopyarrow/integration/conftest.py @@ -18,11 +18,11 @@ from tests.cdm_data_loaders.pipelines.helpers import ( denormalize_identifier, duckdb_reference_from_jsonl, - make_cts_settings, read_jsonl_file, strip_metadata_columns, ) from tests.conftest import N_REFERENCE_XML_ENTRIES, REFERENCE_XML_TAG +from tests.helpers import make_cts_settings from tests.xml_samples import LIBRARY_XSD DEFAULT_XMLTOPYARROW_SETTINGS: frozendict = frozendict( diff --git a/tests/cdm_data_loaders/pipelines/xmltopyarrow/integration/test_xmltopyarrow_end_to_end.py b/tests/cdm_data_loaders/pipelines/xmltopyarrow/integration/test_xmltopyarrow_end_to_end.py index f33c5635..6f45b029 100644 --- a/tests/cdm_data_loaders/pipelines/xmltopyarrow/integration/test_xmltopyarrow_end_to_end.py +++ b/tests/cdm_data_loaders/pipelines/xmltopyarrow/integration/test_xmltopyarrow_end_to_end.py @@ -22,7 +22,6 @@ LOADER_FILE_FORMATS, assert_pipeline_report_written, assert_run_ok, - settings_to_argv, sorted_json, ) from tests.cdm_data_loaders.pipelines.xmltopyarrow.integration.conftest import ( @@ -30,6 +29,7 @@ strip_xsd_arrow_row, ) from tests.conftest import N_REFERENCE_XML_ENTRIES +from tests.helpers import settings_to_argv from tests.xml_samples import SIMPLE_LIBRARY_XML diff --git a/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_pipeline.py b/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_pipeline.py index dbd5d649..e1eaf5a0 100644 --- a/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_pipeline.py +++ b/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_pipeline.py @@ -32,7 +32,8 @@ from cdm_data_loaders.pipelines.xmltopyarrow.uniref import cli as uniref_cli from cdm_data_loaders.pipelines.xmltopyarrow.uniref import run_uniref_pipeline from tests.cdm_data_loaders.pipelines.conftest import copy_test_files, duckdb_pipeline -from tests.cdm_data_loaders.pipelines.helpers import assert_run_ok, fetch_all_duckdb, settings_to_argv +from tests.cdm_data_loaders.pipelines.helpers import assert_run_ok, fetch_all_duckdb +from tests.helpers import settings_to_argv from tests.xml_samples import TWO_BOOK_LIBRARY_XML diff --git a/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py b/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py index f3d3db7f..450b5cca 100644 --- a/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py +++ b/tests/cdm_data_loaders/pipelines/xmltopyarrow/test_xmltopyarrow_settings.py @@ -29,8 +29,8 @@ from cdm_data_loaders.readers.xsd import DEFAULT_MAX_RECURSION_DEPTH, generate_pyarrow_schema, load_schema from cdm_data_loaders.schemas.uniprot.schema import SCHEMA as UNIPROT_SCHEMA from cdm_data_loaders.schemas.uniref.schema import SCHEMA as UNIREF_SCHEMA -from tests.cdm_data_loaders.pipelines.helpers import settings_to_argv from tests.conftest import S3 +from tests.helpers import settings_to_argv UNIREF_LIKE_XSD: str = "uniref_like.xsd" diff --git a/tests/cdm_data_loaders/pipelines/xsv/conftest.py b/tests/cdm_data_loaders/pipelines/xsv/conftest.py index bcd9ac69..b5a053ce 100644 --- a/tests/cdm_data_loaders/pipelines/xsv/conftest.py +++ b/tests/cdm_data_loaders/pipelines/xsv/conftest.py @@ -12,7 +12,7 @@ from cdm_data_loaders.pipelines.xsv.pipeline import XsvWorkPaths from cdm_data_loaders.pipelines.xsv.settings import XsvIngestSettings from cdm_data_loaders.readers.jsonschema_xsv.xsv_validator.schema_utils import generate_header -from tests.cdm_data_loaders.pipelines.helpers import make_cts_settings +from tests.helpers import make_cts_settings VALID_SCHEMA_URI: Final[str] = "https://json-schema.org/draft/2020-12/schema" COLUMNS: Final[list[str]] = ["number", "date", "float", "boolean", "string"] diff --git a/tests/cdm_data_loaders/pipelines/xsv/integration/test_xsv_end_to_end.py b/tests/cdm_data_loaders/pipelines/xsv/integration/test_xsv_end_to_end.py index 17e7ef49..31f71189 100644 --- a/tests/cdm_data_loaders/pipelines/xsv/integration/test_xsv_end_to_end.py +++ b/tests/cdm_data_loaders/pipelines/xsv/integration/test_xsv_end_to_end.py @@ -13,7 +13,6 @@ from tests.cdm_data_loaders.pipelines.helpers import ( assert_pipeline_report_written, assert_run_ok, - settings_to_argv, ) from tests.cdm_data_loaders.pipelines.xsv.conftest import ( ALL_RAGGED_ROWS, @@ -23,6 +22,7 @@ VALID_ROWS, rows_to_csv, ) +from tests.helpers import settings_to_argv pytestmark = pytest.mark.usefixtures("qsv_cmd") diff --git a/tests/cdm_data_loaders/pipelines/xsv/test_xsv_pipeline.py b/tests/cdm_data_loaders/pipelines/xsv/test_xsv_pipeline.py index 7d917138..a1e06560 100644 --- a/tests/cdm_data_loaders/pipelines/xsv/test_xsv_pipeline.py +++ b/tests/cdm_data_loaders/pipelines/xsv/test_xsv_pipeline.py @@ -28,9 +28,9 @@ assert_run_ok, fetch_all_duckdb, make_file_item, - settings_to_argv, ) from tests.cdm_data_loaders.pipelines.xsv.conftest import VALID_SCHEMA_URI +from tests.helpers import settings_to_argv SIMPLE_SCHEMA: dict[str, Any] = { "$schema": VALID_SCHEMA_URI, diff --git a/tests/helpers.py b/tests/helpers.py index 806373d8..6d984483 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,6 +1,7 @@ """Utilities for running tests.""" import json +from collections.abc import Iterable from dataclasses import dataclass from pathlib import Path from typing import Any @@ -10,7 +11,8 @@ from pyspark.sql import Row, SparkSession from pyspark.sql.types import StructType -from cdm_data_loaders.core.settings import CdmDataLoadersBase +from cdm_data_loaders.core.settings import CdmDataLoadersBase, CtsSettings +from tests.conftest import LOCAL_FS def make_cli_arg(arg: str) -> str: @@ -18,6 +20,62 @@ def make_cli_arg(arg: str) -> str: return f"{'--' if len(arg) > 1 else '-'}{arg.replace('_', '-')}" +def make_cts_settings[S: CtsSettings]( + settings_cls: type[S], + base_dir: Path, + **overrides: Any, # noqa: ANN401 +) -> S: + """Create standard input/output directories and log config under base_dir and instantiate settings_cls.""" + input_dir = base_dir / "input" + input_dir.mkdir(parents=True, exist_ok=True) + output_dir = base_dir / "output" + output_dir.mkdir(parents=True, exist_ok=True) + log_config_file = base_dir / "logging.json" + if not log_config_file.exists(): + log_config_file.write_text('{"version": 1}', encoding="utf-8") + + kwargs: dict[str, Any] = { + "input_dir": str(input_dir), + "output_dir": str(output_dir), + "log_config_file": str(log_config_file), + "use_destination": LOCAL_FS, + "dlt_dev_mode": False, + "use_output_dir_for_pipeline_metadata": False, + } + kwargs.update(overrides) + return settings_cls(**kwargs) + + +def settings_to_argv(settings: Any, *, exclude: Iterable[str] = ()) -> list[str]: # noqa: ANN401 + """Derive CLI arguments from a settings instance. + + Every serialized field becomes a ``--kebab-case`` flag, with bools rendered + as their ``true``/``false`` CLI spelling, sequence fields expanded after the + flag, and None-valued, excluded, or computed fields skipped. + + :param settings: a CtsSettings (or derivative) instance + :type settings: Any + :param exclude: field names to omit from the resulting argument list + :type exclude: Iterable[str] + :return: command-line argument list reproducing the settings + :rtype: list[str] + """ + argv: list[str] = [] + excluded_fields: set[str] = set(getattr(type(settings), "model_computed_fields", {}).keys()) | set(exclude) + for name, value in settings.model_dump(exclude=excluded_fields).items(): + flag = make_cli_arg(name) + if isinstance(value, bool): + argv.extend([flag, "true" if value else "false"]) + elif value is None: + continue + elif isinstance(value, list | tuple): + if value: + argv.extend([flag, *(str(item) for item in value)]) + else: + argv.extend([flag, str(value)]) + return argv + + @dataclass(frozen=True) class CliArgSpec: """Everything a test needs to drive a single field from the CLI.""" diff --git a/tests/jsonschema_fixtures.py b/tests/jsonschema_fixtures.py index bc4f1273..83368f72 100644 --- a/tests/jsonschema_fixtures.py +++ b/tests/jsonschema_fixtures.py @@ -12,7 +12,7 @@ import pytest from cdm_data_loaders.pipelines.jsonlines.settings import JsonlJsonschemaIngestSettings -from tests.cdm_data_loaders.pipelines.helpers import make_cts_settings +from tests.helpers import make_cts_settings @pytest.fixture