diff --git a/docs/PIPELINE_MIGRATION.md b/docs/PIPELINE_MIGRATION.md new file mode 100644 index 0000000..cf89205 --- /dev/null +++ b/docs/PIPELINE_MIGRATION.md @@ -0,0 +1,299 @@ +# Pipeline Command Migration Guide + +## Overview + +The `rompy pipeline` command has been redesigned to support a cleaner, more consistent configuration structure with YAML include functionality. + +## What Changed + +### Old Structure (Deprecated) + +```bash +# Separate files required +rompy pipeline model.yaml \ + --run-backend local \ + --processor-config processor.yaml +``` + +**Problems with old approach:** +- `--run-backend` only accepted type string ("local", "docker") +- No way to configure backend timeout, commands, or environment variables +- Mandatory separate processor config file +- Inconsistent with `rompy run` command structure + +### New Structure + +```bash +# All-in-one pipeline config +rompy pipeline pipeline.yaml + +# Or with overrides +rompy pipeline pipeline.yaml \ + --backend-config custom_backend.yaml \ + --processor-config custom_processor.yaml +``` + +**Benefits:** +- Single file for complete workflows +- Full backend configuration support +- Optional inline or referenced configs +- YAML `!include` directives for composition +- Consistent nested structure + +## New Config Structure + +All pipeline configs must now use this three-section structure: + +```yaml +config: # ModelRun configuration + run_id: my_run + period: + start: "2023-01-01T00:00:00" + end: "2023-01-02T00:00:00" + config: + model_type: shel + # ... model-specific config + +backend: # Backend configuration + type: local # or docker, slurm + timeout: 7200 + command: ./run.sh + env_vars: + VAR: value + +postprocessor: # Postprocessor configuration + type: ww3_transfer + destinations: + - "file:///backup/" + timeout: 3600 +``` + +## YAML Include Support + +### Basic Include + +Compose configs from multiple files: + +```yaml +config: !include model_config.yaml +backend: !include backends/local.yaml +postprocessor: !include postprocessors/transfer.yaml +``` + +### Mixed Inline and Includes + +```yaml +config: !include shared/global_model.yaml + +backend: + type: local + timeout: 7200 + +postprocessor: !include postprocessors/ww3_transfer.yaml +``` + +### Include Features + +- **Relative paths**: Resolved from the including file's directory +- **Absolute paths**: Supported +- **Environment variables**: `!include $HOME/configs/backend.yaml` +- **Nested includes**: Includes can reference other includes +- **Circular detection**: Prevents infinite loops +- **Depth limit**: Maximum 10 levels of nesting + +## Migration Examples + +### Example 1: Basic Migration + +**Old:** +```bash +# model.yaml +run_id: test +period: {...} +config: {...} + +# Command +rompy pipeline model.yaml \ + --run-backend local \ + --processor-config proc.yaml +``` + +**New:** +```yaml +# pipeline.yaml +config: + run_id: test + period: {...} + config: {...} + +backend: + type: local + +postprocessor: !include proc.yaml +``` + +```bash +rompy pipeline pipeline.yaml +``` + +### Example 2: With Backend Options + +**Old (not possible):** +```bash +# Could NOT configure timeout or custom commands +rompy pipeline model.yaml --run-backend local --processor-config proc.yaml +``` + +**New:** +```yaml +config: !include model.yaml + +backend: + type: local + timeout: 7200 + command: ./custom_run.sh + env_vars: + OMP_NUM_THREADS: "4" + +postprocessor: !include proc.yaml +``` + +### Example 3: Modular Configs + +**Organize by environment:** +``` +configs/ +├── models/ +│ ├── global_3deg.yaml +│ └── regional_1deg.yaml +├── backends/ +│ ├── local_dev.yaml +│ ├── local_prod.yaml +│ └── docker.yaml +├── postprocessors/ +│ ├── transfer_s3.yaml +│ └── transfer_local.yaml +└── pipelines/ + ├── dev_pipeline.yaml + └── prod_pipeline.yaml +``` + +**dev_pipeline.yaml:** +```yaml +config: !include ../models/global_3deg.yaml +backend: !include ../backends/local_dev.yaml +postprocessor: !include ../postprocessors/transfer_local.yaml +``` + +**prod_pipeline.yaml:** +```yaml +config: !include ../models/global_3deg.yaml +backend: !include ../backends/local_prod.yaml +postprocessor: !include ../postprocessors/transfer_s3.yaml +``` + +## CLI Override Behavior + +CLI flags override inline configurations: + +```yaml +# pipeline.yaml +config: {...} +backend: + type: local + timeout: 3600 +postprocessor: + type: noop +``` + +```bash +# Override backend +rompy pipeline pipeline.yaml --backend-config docker_backend.yaml + +# Override postprocessor +rompy pipeline pipeline.yaml --processor-config custom_proc.yaml + +# Override both +rompy pipeline pipeline.yaml \ + --backend-config docker_backend.yaml \ + --processor-config custom_proc.yaml +``` + +## Breaking Changes + +### Removed + +1. **`--run-backend` flag** - Replaced with nested `backend` config or `--backend-config` +2. **Flat config structure** - All configs must now use nested `config`/`backend`/`postprocessor` sections + +### Required Changes + +| Old | New | +|-----|-----| +| `--run-backend local` | `backend: {type: local}` in config or `--backend-config` | +| `--processor-config FILE` (required) | `postprocessor:` in config or `--processor-config` (optional) | +| Top-level `run_id`, `period` | Nest under `config:` section | + +## Error Messages + +### Missing Sections + +``` +Error: Pipeline config must contain 'config' section with ModelRun configuration. +``` +**Fix:** Wrap your model config in a `config:` section. + +``` +Error: Backend configuration required. Provide either: + - 'backend' section in pipeline config, or + - --backend-config flag +``` +**Fix:** Add `backend:` section or use `--backend-config`. + +``` +Error: Postprocessor configuration required. Provide either: + - 'postprocessor' section in pipeline config, or + - --processor-config flag +``` +**Fix:** Add `postprocessor:` section or use `--processor-config`. + +### Include Errors + +``` +Error: Included file not found: configs/model.yaml +Referenced from: /path/to/pipeline.yaml +``` +**Fix:** Check the file path is correct relative to the including file. + +``` +Error: Circular include detected: /path/to/file1.yaml +Include chain: file1.yaml -> file2.yaml -> file1.yaml +``` +**Fix:** Remove circular references in your includes. + +## Backward Compatibility + +**None.** This is a breaking change. The old `--run-backend` string flag is no longer supported. + +All existing pipeline commands must be updated to use the new structure. + +## Best Practices + +1. **Use includes for reusability**: Keep backend and postprocessor configs in separate files for reuse +2. **Environment-specific configs**: Create separate pipeline files for dev/staging/prod +3. **Validate early**: Use `rompy pipeline --help` to see expected structure +4. **Keep model configs portable**: Model configs (`config:` section) should not contain backend/processor details +5. **Version control**: Commit all config files including referenced includes + +## Examples + +See the `examples/` directory for complete working examples: + +- `global_3deg_pipeline.yaml` - All inline configuration +- `global_3deg_pipeline_with_includes.yaml` - Using !include directives + +## Questions? + +- Run `rompy pipeline --help` for command documentation +- Check existing tests: `tests/test_yaml_loader.py` for include examples +- See `src/rompy/core/yaml_loader.py` for include implementation details diff --git a/examples/configs/basic_pipeline.yml b/examples/configs/basic_pipeline.yml index 1249f38..ea7a32d 100644 --- a/examples/configs/basic_pipeline.yml +++ b/examples/configs/basic_pipeline.yml @@ -1,9 +1,14 @@ # Complete Pipeline Configuration for CLI Testing -# This demonstrates how to use the basic model run with different backends via CLI - -pipeline_backend: local # or 'docker', 'slurm' depending on your system - -model_run: +# This demonstrates the new unified pipeline configuration structure +# +# Usage: +# rompy pipeline --config basic_pipeline.yml +# +# Or with external configs: +# rompy pipeline --config basic_pipeline.yml --backend-config my_backend.yml --processor-config my_processor.yml + +# Model run configuration (required) +config: run_id: "cli_test_backend_run" output_dir: "./output/cli_test" delete_existing: true @@ -11,27 +16,20 @@ model_run: start: "2023-01-01T00:00:00" end: "2023-01-02T00:00:00" interval: "1H" - -# This would be the backend for the actual model run execution -# Uncomment the appropriate section based on your system: - -# Local backend configuration -run_backend: - backend_type: local + # Add your model-specific configuration here + # config: + # model_type: "swan" + # ... + +# Backend configuration (optional - can be overridden with --backend-config) +backend: + type: local timeout: 3600 command: "echo 'Running basic model test'" env_vars: MODEL_TYPE: "test" ENVIRONMENT: "cli" -# To run with local backend: -# rompy run --config basic_modelrun.yml --backend-config local_backend.yml - -# To run with Docker backend: -# rompy run --config basic_modelrun.yml --backend-config docker_backend.yml - -# To run with SLURM backend: -# rompy run --config basic_modelrun.yml --backend-config slurm_backend.yml - -postprocessing: - processor: "noop" # or other available processors \ No newline at end of file +# Postprocessor configuration (optional - can be overridden with --processor-config) +postprocessor: + type: noop \ No newline at end of file diff --git a/examples/configs/pipeline_config.yml b/examples/configs/pipeline_config.yml index ec853b9..c07f6c6 100644 --- a/examples/configs/pipeline_config.yml +++ b/examples/configs/pipeline_config.yml @@ -1,12 +1,14 @@ # Pipeline Configuration Example -# This file demonstrates how to configure complete model pipelines for ROMPY models. +# This file demonstrates the new unified pipeline configuration structure for ROMPY models. # Use this file with CLI commands like: rompy pipeline --config pipeline_config.yml - -# Pipeline backend type - determines how the pipeline is executed -pipeline_backend: local - -# Model run configuration -model_run: +# +# New structure: +# - config: ModelRun configuration (required) +# - backend: Backend configuration (optional, can be overridden with --backend-config) +# - postprocessor: Postprocessor configuration (optional, can be overridden with --processor-config) + +# Example 1: Basic local pipeline +config: run_id: "example_pipeline_run" output_dir: "./simulations" delete_existing: true @@ -23,36 +25,23 @@ model_run: model_type: "swan" # or other supported model types # Add model-specific configuration here -# Run backend configuration -run_backend: - backend_type: local +# Backend configuration +backend: + type: local timeout: 7200 # 2 hours command: "python run_model.py" env_vars: OMP_NUM_THREADS: "4" MODEL_CONFIG: "production" -# Postprocessing configuration -postprocessing: - processor: "noop" # or other available processors +# Postprocessor configuration +postprocessor: + type: noop # or other available processors # Add processor-specific parameters here -# Pipeline-specific settings -pipeline_settings: - # Whether to continue if individual steps fail - continue_on_error: false - - # Whether to clean up intermediate files - cleanup_intermediate: true - - # Maximum time for entire pipeline - max_pipeline_time: 14400 # 4 hours - --- -# Alternative configuration using Docker -pipeline_backend: local - -model_run: +# Example 2: Docker-based pipeline +config: run_id: "docker_pipeline_run" output_dir: "./docker_simulations" delete_existing: true @@ -65,9 +54,9 @@ model_run: config: model_type: "swan" -# Run backend configuration -run_backend: - backend_type: docker +# Docker backend configuration +backend: + type: docker image: "swan:latest" timeout: 10800 # 3 hours cpu: 8 @@ -81,19 +70,12 @@ run_backend: MODEL_PARALLEL: "true" DATA_DIR: "/app/data" -postprocessing: - processor: "noop" - -pipeline_settings: - continue_on_error: false - cleanup_intermediate: false # Keep files for debugging - max_pipeline_time: 21600 # 6 hours +postprocessor: + type: noop --- -# Multi-stage pipeline configuration -pipeline_backend: local - -model_run: +# Example 3: High-performance Docker pipeline with custom postprocessing +config: run_id: "multi_stage_pipeline" output_dir: "./multi_stage_output" delete_existing: true @@ -106,10 +88,9 @@ model_run: config: model_type: "schism" -# Run configuration for computationally intensive model -# Run backend configuration -run_backend: - backend_type: docker +# HPC Docker backend configuration +backend: + type: docker dockerfile: "./docker/Dockerfile.hpc" build_context: "./docker" build_args: @@ -130,36 +111,18 @@ run_backend: SLURM_NTASKS: "32" SCHISM_NPROC: "32" -# Custom postprocessing -postprocessing: - processor: "custom_analysis" +# Custom postprocessor configuration +postprocessor: + type: custom_analysis # Custom processor parameters analysis_type: "wave_statistics" output_format: "netcdf" generate_plots: true statistics_window: "24H" -pipeline_settings: - continue_on_error: false - cleanup_intermediate: true - max_pipeline_time: 108000 # 30 hours - - # Additional pipeline-specific settings - notification: - email: "user@example.com" - on_success: true - on_failure: true - - monitoring: - log_level: "INFO" - progress_updates: true - resource_monitoring: true - --- -# Development/Testing pipeline configuration -pipeline_backend: local - -model_run: +# Example 4: Development/Testing pipeline +config: run_id: "test_pipeline" output_dir: "./test_output" delete_existing: true @@ -173,10 +136,9 @@ model_run: config: model_type: "swan" -# Fast local execution for testing -# Run backend configuration -run_backend: - backend_type: local +# Fast local backend for testing +backend: + type: local timeout: 1800 # 30 minutes command: "echo 'Test run' && python -m pytest tests/" capture_output: true @@ -184,15 +146,45 @@ run_backend: ENV: "testing" LOG_LEVEL: "DEBUG" -postprocessing: - processor: "noop" +postprocessor: + type: noop + +--- +# Example 5: Pipeline using YAML includes for composition +config: + run_id: "modular_pipeline" + output_dir: "./modular_output" + delete_existing: true + + period: + start: "2023-01-01T00:00:00" + end: "2023-01-02T00:00:00" + interval: "1H" + + # Include model config from external file + config: !include ./model_configs/swan_config.yml + +# Include backend config from external file +backend: !include ./backend_configs/docker_backend.yml + +# Include postprocessor config from external file +postprocessor: !include ./processor_configs/ww3_transfer.yml + +--- +# Example 6: Minimal pipeline (backend and postprocessor optional) +# Backend and postprocessor can be omitted from config and provided via CLI flags +config: + run_id: "minimal_pipeline" + output_dir: "./minimal_output" + delete_existing: true + + period: + start: "2023-01-01T00:00:00" + end: "2023-01-01T12:00:00" + interval: "1H" -pipeline_settings: - continue_on_error: true # Continue for testing - cleanup_intermediate: false # Keep files for inspection - max_pipeline_time: 3600 # 1 hour max + config: + model_type: "swan" - # Testing-specific settings - dry_run: false - validate_only: false - debug_mode: true +# No backend or postprocessor defined - must be provided via CLI: +# rompy pipeline --config this_file.yml --backend-config my_backend.yml --processor-config my_processor.yml diff --git a/src/rompy/cli.py b/src/rompy/cli.py index a129754..4b16c6f 100644 --- a/src/rompy/cli.py +++ b/src/rompy/cli.py @@ -129,6 +129,9 @@ def load_config( ) -> Dict[str, Any]: """Load configuration from file, string, or environment variable. + Supports YAML files with !include directives for composing configs from + multiple files. + Args: config_path: Path to config file or raw config string from_env: If True, load from environment variable instead of config_path @@ -140,35 +143,68 @@ def load_config( Raises: click.UsageError: If config cannot be loaded or parsed """ + from rompy.core.yaml_loader import load_yaml_with_includes, safe_load_with_includes + from pathlib import Path + if from_env: content = os.environ.get(env_var) if content is None: raise click.UsageError(f"Environment variable {env_var} is not set") logger.info(f"Loading config from environment variable: {env_var}") - else: + + # Try JSON first try: - with open(config_path, "r") as f: - content = f.read() - except (FileNotFoundError, IsADirectoryError, OSError): - # Not a file, treat as raw string + config = json.loads(content) + logger.info("Parsed config as JSON") + return config + except json.JSONDecodeError: + pass + + # Try YAML with includes (use cwd as root dir) + try: + config = safe_load_with_includes(content, root_dir=Path.cwd()) + logger.info("Parsed config as YAML") + except yaml.YAMLError as e: + logger.error(f"Failed to parse config as JSON or YAML: {e}") + raise click.UsageError("Config is not valid JSON or YAML") + else: + # Check if it's a file path + config_file = Path(config_path) + if config_file.exists() and config_file.is_file(): + logger.info(f"Loading config from: {config_path}") + + # Try YAML with includes support first (most common case) + try: + config = load_yaml_with_includes(config_file) + logger.info("Parsed config as YAML") + except yaml.YAMLError: + # If YAML fails, try JSON + try: + with open(config_file, "r") as f: + config = json.load(f) + logger.info("Parsed config as JSON") + except json.JSONDecodeError as e: + logger.error(f"Failed to parse config as JSON or YAML: {e}") + raise click.UsageError("Config file is not valid JSON or YAML") + else: + # Not a file, treat as raw string content content = config_path - logger.info(f"Loading config from: {config_path}") - # Try JSON first - try: - config = json.loads(content) - logger.info("Parsed config as JSON") - return config - except json.JSONDecodeError: - pass + # Try JSON first + try: + config = json.loads(content) + logger.info("Parsed config as JSON") + return config + except json.JSONDecodeError: + pass - # If JSON failed, try YAML - try: - config = yaml.safe_load(content) - logger.info("Parsed config as YAML") - except yaml.YAMLError as e: - logger.error(f"Failed to parse config as JSON or YAML: {e}") - raise click.UsageError("Config file is not valid JSON or YAML") + # Try YAML + try: + config = yaml.safe_load(content) + logger.info("Parsed config as YAML") + except yaml.YAMLError as e: + logger.error(f"Failed to parse config as JSON or YAML: {e}") + raise click.UsageError("Config is not valid JSON or YAML") # Render template variables in config try: @@ -363,12 +399,17 @@ def _load_backend_config(backend_config_file): @cli.command() @click.argument("config", type=click.Path(exists=True), required=False) -@click.option("--run-backend", default="local", help="Execution backend for run stage") +@click.option( + "--backend-config", + type=click.Path(exists=True), + required=False, + help="YAML/JSON file with backend configuration (optional, can be inline in config)", +) @click.option( "--processor-config", type=click.Path(exists=True), - required=True, - help="YAML/JSON file with postprocessor configuration (required)", + required=False, + help="YAML/JSON file with postprocessor configuration (optional, can be inline in config)", ) @click.option( "--cleanup-on-failure/--no-cleanup", default=False, help="Clean up on failure" @@ -379,7 +420,7 @@ def _load_backend_config(backend_config_file): @add_common_options def pipeline( config, - run_backend, + backend_config, processor_config, cleanup_on_failure, validate_stages, @@ -390,7 +431,31 @@ def pipeline( simple_logs, config_from_env, ): - """Run full model pipeline: generate → run → postprocess.""" + """Run full model pipeline: generate → run → postprocess. + + The pipeline config should have the following structure: + + \b + config: # ModelRun configuration + run_id: ... + period: ... + config: ... + backend: # Backend configuration (or use --backend-config) + type: local + timeout: 7200 + postprocessor: # Postprocessor configuration (or use --processor-config) + type: ww3_transfer + destinations: [...] + + Sections can use !include to reference external files: + + \b + config: !include model_config.yaml + backend: !include backends/local.yaml + postprocessor: !include postprocessors/transfer.yaml + + CLI flags override inline configurations. + """ configure_logging(verbose, log_dir, simple_logs, ascii_only, show_warnings) # Validate config source @@ -400,27 +465,76 @@ def pipeline( raise click.UsageError("Must specify either config file or --config-from-env") try: - # Load configuration - config_data = load_config(config, from_env=config_from_env) - model_run = ModelRun(**config_data) + # Load pipeline configuration (supports !include directives) + pipeline_data = load_config(config, from_env=config_from_env) + + # Extract sections from pipeline config + model_config_data = pipeline_data.get("config") + backend_data = pipeline_data.get("backend") + processor_data = pipeline_data.get("postprocessor") + + # Apply CLI overrides + if backend_config: + logger.info(f"Overriding backend config with: {backend_config}") + backend_data = load_config(backend_config) + + if processor_config: + logger.info(f"Overriding processor config with: {processor_config}") + processor_data = load_config(processor_config) + + # Validate required sections + if not model_config_data: + raise click.UsageError( + "Pipeline config must contain 'config' section with ModelRun configuration.\n" + "See 'rompy pipeline --help' for expected structure." + ) + + if not backend_data: + raise click.UsageError( + "Backend configuration required. Provide either:\n" + " - 'backend' section in pipeline config, or\n" + " - --backend-config flag" + ) + + if not processor_data: + raise click.UsageError( + "Postprocessor configuration required. Provide either:\n" + " - 'postprocessor' section in pipeline config, or\n" + " - --processor-config flag" + ) + + # Instantiate configurations + model_run = ModelRun(**model_config_data) + + # Instantiate backend config + if "type" not in backend_data: + raise click.UsageError("Backend configuration must include a 'type' field") + backend_type = backend_data.pop("type") + registry = _get_backend_config_registry() + if backend_type not in registry: + available = ", ".join(registry.keys()) + raise click.UsageError( + f"Unknown backend type: {backend_type}. Available: {available}" + ) + backend_cfg = registry[backend_type](**backend_data) # Load processor configuration - from rompy.postprocess.config import _load_processor_config + from rompy.postprocess.config import _load_processor_config_from_dict - processor_cfg = _load_processor_config(processor_config) + processor_cfg = _load_processor_config_from_dict(processor_data) logger.info(f"Running pipeline for: {model_run.config.model_type}") logger.info(f"Run ID: {model_run.run_id}") logger.info( - f"Pipeline: generate → run({run_backend}) → postprocess({processor_cfg.type})" + f"Pipeline: generate → run({backend_type}) → postprocess({processor_cfg.type})" ) start_time = datetime.now() - # Execute pipeline + # Execute pipeline with backend config instead of string results = model_run.pipeline( pipeline_backend="local", - run_backend=run_backend, + backend_config=backend_cfg, processor=processor_cfg, cleanup_on_failure=cleanup_on_failure, validate_stages=validate_stages, diff --git a/src/rompy/core/yaml_loader.py b/src/rompy/core/yaml_loader.py new file mode 100644 index 0000000..28a19cd --- /dev/null +++ b/src/rompy/core/yaml_loader.py @@ -0,0 +1,212 @@ +""" +YAML loader with support for !include directive. + +This module provides a custom YAML loader that extends the standard safe_load +functionality with the ability to include external YAML files using the !include tag. + +Example: + config: !include model_config.yaml + backend: !include backends/local.yaml +""" + +import os +from pathlib import Path +from typing import Any, Dict, Set + +import yaml + + +class IncludeLoader(yaml.SafeLoader): + """Custom YAML loader with support for !include directive. + + This loader extends yaml.SafeLoader to add the !include constructor, + which allows referencing external YAML files. It tracks the chain of + included files to prevent circular dependencies. + + Features: + - Relative path resolution from the including file's directory + - Absolute path support + - Circular dependency detection + - Environment variable expansion in paths + - Maximum include depth limit + + Attributes: + _root_dir: Directory of the root YAML file being loaded + _include_chain: Set of file paths already included (for cycle detection) + _include_depth: Current nesting depth of includes + """ + + # Maximum allowed include depth to prevent infinite recursion + MAX_INCLUDE_DEPTH = 10 + + def __init__(self, stream): + """Initialize the loader with stream and include tracking. + + Args: + stream: YAML stream to load from + """ + self._root_dir = Path.cwd() + self._include_chain: Set[Path] = set() + self._include_depth = 0 + + # Try to get the directory of the file being loaded + if hasattr(stream, "name"): + self._root_dir = Path(stream.name).parent.resolve() + + super().__init__(stream) + + +def include_constructor(loader: IncludeLoader, node: yaml.Node) -> Any: + """Constructor function for !include tag. + + This function is called by the YAML loader when it encounters an !include tag. + It resolves the path, loads the referenced file, and returns its contents. + + Args: + loader: The IncludeLoader instance + node: The YAML node containing the include path + + Returns: + The loaded content from the included file (dict, list, or scalar) + + Raises: + yaml.YAMLError: If the file cannot be loaded or circular dependency detected + FileNotFoundError: If the included file doesn't exist + ValueError: If include depth exceeds maximum + """ + # Check include depth + if loader._include_depth >= IncludeLoader.MAX_INCLUDE_DEPTH: + raise ValueError( + f"Include depth exceeds maximum of {IncludeLoader.MAX_INCLUDE_DEPTH}. " + "This may indicate a circular dependency or overly nested includes." + ) + + # Get the path from the node + include_path = loader.construct_scalar(node) + + # Expand environment variables in the path + include_path = os.path.expandvars(include_path) + + # Resolve the path relative to the current file's directory + if not os.path.isabs(include_path): + include_path = loader._root_dir / include_path + else: + include_path = Path(include_path) + + # Resolve to absolute path to check for circular dependencies + include_path = include_path.resolve() + + # Check if file exists + if not include_path.exists(): + raise FileNotFoundError( + f"Included file not found: {include_path}\n" + f"Referenced from: {loader._root_dir}" + ) + + # Check for circular dependencies + if include_path in loader._include_chain: + chain_str = " -> ".join(str(p) for p in loader._include_chain) + raise yaml.YAMLError( + f"Circular include detected: {include_path}\n" + f"Include chain: {chain_str} -> {include_path}" + ) + + # Add to include chain + loader._include_chain.add(include_path) + loader._include_depth += 1 + + try: + # Load the included file with a fresh file handle + with open(include_path, "r") as f: + # Read the content + content = f.read() + + # Create a StringIO for the content + from io import StringIO + + stream = StringIO(content) + + # Create a new loader that inherits the include chain + included_loader = IncludeLoader(stream) + included_loader._root_dir = include_path.parent + included_loader._include_chain = loader._include_chain.copy() + included_loader._include_depth = loader._include_depth + + # Load and return the content + result = included_loader.get_single_data() + stream.close() + return result + + finally: + # Remove from include chain when done + loader._include_chain.discard(include_path) + loader._include_depth -= 1 + + +# Register the include constructor +IncludeLoader.add_constructor("!include", include_constructor) + + +def load_yaml_with_includes(stream) -> Dict[str, Any]: + """Load a YAML file with support for !include directives. + + This is the main entry point for loading YAML files that may contain + !include tags. It uses the custom IncludeLoader to handle includes. + + Args: + stream: File path (str/Path) or file-like object to load from + + Returns: + Loaded YAML content as a dictionary + + Raises: + FileNotFoundError: If the file doesn't exist + yaml.YAMLError: If the YAML is invalid or includes have errors + + Example: + >>> content = load_yaml_with_includes('pipeline.yaml') + >>> print(content['config']) + """ + # If stream is a string or Path, open it as a file + if isinstance(stream, (str, Path)): + stream = Path(stream) + if not stream.exists(): + raise FileNotFoundError(f"YAML file not found: {stream}") + + with open(stream, "r") as f: + return yaml.load(f, Loader=IncludeLoader) + else: + # Assume it's a file-like object + return yaml.load(stream, Loader=IncludeLoader) + + +def safe_load_with_includes(content: str, root_dir: Path = None) -> Dict[str, Any]: + """Load YAML content from a string with include support. + + This function is useful when you have YAML content as a string but + still want to support includes. You can specify a root directory for + resolving relative paths. + + Args: + content: YAML content as a string + root_dir: Directory to use for resolving relative includes (default: cwd) + + Returns: + Loaded YAML content as a dictionary + + Example: + >>> yaml_str = "config: !include model.yaml" + >>> content = safe_load_with_includes(yaml_str, Path('/configs')) + """ + from io import StringIO + + stream = StringIO(content) + loader = IncludeLoader(stream) + + if root_dir: + loader._root_dir = Path(root_dir).resolve() + + try: + return loader.get_single_data() + finally: + stream.close() diff --git a/src/rompy/model.py b/src/rompy/model.py index 406b3a5..787bbec 100644 --- a/src/rompy/model.py +++ b/src/rompy/model.py @@ -423,8 +423,8 @@ def pipeline(self, pipeline_backend: str = "local", **kwargs) -> Dict[str, Any]: Args: pipeline_backend: Name of the pipeline backend to use (default: "local") **kwargs: Additional backend-specific parameters. Common parameters include: - - run_backend: Backend to use for the run stage (for local pipeline) - - processor: Processor to use for postprocessing (for local pipeline) + - backend_config: BackendConfig instance for the run stage (for local pipeline) + - processor: ProcessorConfig instance for postprocessing (for local pipeline) - run_kwargs: Additional parameters for the run stage - process_kwargs: Additional parameters for postprocessing diff --git a/src/rompy/pipeline/__init__.py b/src/rompy/pipeline/__init__.py index 5aab946..053cef7 100644 --- a/src/rompy/pipeline/__init__.py +++ b/src/rompy/pipeline/__init__.py @@ -6,10 +6,14 @@ import logging from pathlib import Path -from typing import Any, Dict, Optional, Union +from typing import TYPE_CHECKING, Any, Dict, Optional, Union from rompy.backends import DockerConfig, LocalConfig +if TYPE_CHECKING: + from rompy.backends.config import SlurmConfig + from rompy.postprocess.config import BasePostprocessorConfig + logger = logging.getLogger(__name__) @@ -23,7 +27,7 @@ class LocalPipelineBackend: def execute( self, model_run, - run_backend: str = "local", + backend_config: Union[LocalConfig, DockerConfig, "SlurmConfig"] = None, processor: "BasePostprocessorConfig" = None, run_kwargs: Optional[Dict[str, Any]] = None, process_kwargs: Optional[Dict[str, Any]] = None, @@ -35,13 +39,13 @@ def execute( Args: model_run: The ModelRun instance to execute - run_backend: Backend to use for the run stage ("local" or "docker") + backend_config: Backend configuration object (LocalConfig, DockerConfig, etc.) processor: Processor configuration for the postprocess stage - run_kwargs: Additional parameters for the run stage + run_kwargs: Additional parameters for the run stage (deprecated, use backend_config) process_kwargs: Additional parameters for the postprocess stage cleanup_on_failure: Whether to cleanup outputs on pipeline failure validate_stages: Whether to validate each stage before proceeding - **kwargs: Additional parameters (unused) + **kwargs: Additional parameters (for backward compatibility) Returns: Combined results from the pipeline execution @@ -51,6 +55,7 @@ def execute( TypeError: If processor is not a BasePostprocessorConfig instance """ from rompy.postprocess.config import BasePostprocessorConfig + from rompy.backends.config import BaseBackendConfig # Validate input parameters if not model_run: @@ -59,8 +64,26 @@ def execute( if not hasattr(model_run, "run_id"): raise ValueError("model_run must have a run_id attribute") - if not isinstance(run_backend, str) or not run_backend.strip(): - raise ValueError("run_backend must be a non-empty string") + # Handle backward compatibility: accept run_backend string from kwargs + if backend_config is None: + run_backend = kwargs.get("run_backend") + if run_backend: + logger.warning( + "Passing run_backend as string is deprecated. " + "Use backend_config parameter instead." + ) + run_kwargs = run_kwargs or {} + backend_config = self._create_backend_config(run_backend, run_kwargs) + else: + raise ValueError( + "backend_config is required. Provide a BackendConfig instance." + ) + + if not isinstance(backend_config, BaseBackendConfig): + raise TypeError( + f"backend_config must be a BaseBackendConfig instance, " + f"got {type(backend_config).__name__}" + ) if processor is None: raise ValueError("processor configuration is required") @@ -72,19 +95,19 @@ def execute( ) # Initialize parameters - run_kwargs = run_kwargs or {} process_kwargs = process_kwargs or {} + backend_type = backend_config.__class__.__name__.replace("Config", "").lower() logger.info(f"Starting pipeline execution for run_id: {model_run.run_id}") logger.info( - f"Pipeline configuration: run_backend='{run_backend}', processor='{processor.type}'" + f"Pipeline configuration: backend='{backend_type}', processor='{processor.type}'" ) pipeline_results = { "success": False, "run_id": model_run.run_id, "stages_completed": [], - "run_backend": run_backend, + "backend": backend_type, "processor": processor.type, } @@ -120,12 +143,9 @@ def execute( } # Stage 2: Run the model - logger.info(f"Stage 2: Running model using {run_backend} backend") + logger.info(f"Stage 2: Running model using {backend_type} backend") try: - # Create appropriate backend configuration - backend_config = self._create_backend_config(run_backend, run_kwargs) - # Pass the generated workspace directory to avoid duplicate generation run_success = model_run.run( backend=backend_config, workspace_dir=staging_dir diff --git a/src/rompy/postprocess/config.py b/src/rompy/postprocess/config.py index aa08174..fe9a6b8 100644 --- a/src/rompy/postprocess/config.py +++ b/src/rompy/postprocess/config.py @@ -192,6 +192,54 @@ def _load_processor_config(config_file): ) +def _load_processor_config_from_dict(config_data: dict) -> BasePostprocessorConfig: + """Load processor configuration from a dictionary. + + This function is used internally to instantiate processor configs from + dictionary data (e.g., from inline config or already-parsed YAML). + + Args: + config_data: Dictionary containing processor configuration with 'type' field + + Returns: + An instance of the appropriate postprocessor config class + + Raises: + ValueError: If the processor type is not found or config_data is invalid + """ + from importlib.metadata import entry_points + + if not isinstance(config_data, dict): + raise ValueError(f"Config data must be a dictionary, got {type(config_data)}") + + # Make a copy to avoid modifying the original + config_data = config_data.copy() + + processor_type = config_data.pop("type", None) + + if processor_type is None: + raise ValueError("Config must contain a 'type' field") + + # Load from entry point + eps = entry_points(group="rompy.postprocess.config") + for ep in eps: + if ep.name == processor_type: + config_class = ep.load() + return config_class(**config_data) + + # If we get here, type wasn't found + available = [ep.name for ep in eps] + if available: + available_str = ", ".join(available) + raise ValueError( + f"Unknown processor type: '{processor_type}'. Available types: {available_str}" + ) + else: + raise ValueError( + f"Unknown processor type: '{processor_type}'. No postprocessor types are registered." + ) + + def validate_postprocessor_config(config_file, processor_type=None): """Validate a postprocessor configuration file. diff --git a/tests/backends/test_enhanced_backends.py b/tests/backends/test_enhanced_backends.py index a1197f9..c66c89b 100644 --- a/tests/backends/test_enhanced_backends.py +++ b/tests/backends/test_enhanced_backends.py @@ -60,6 +60,12 @@ def processor_config(): return NoopPostprocessorConfig(validate_outputs=False) +@pytest.fixture +def backend_config(): + """Create a LocalConfig for testing.""" + return LocalConfig() + + class TestEnhancedLocalRunBackend: """Test the enhanced LocalRunBackend with validation and error handling.""" @@ -340,37 +346,45 @@ def test_execute_validation_invalid_model_run(self): with pytest.raises(ValueError, match="model_run must have a run_id attribute"): backend.execute(invalid_model) - def test_execute_validation_invalid_run_backend(self, model_run): - """Test that execute raises ValueError for invalid run_backend.""" + def test_execute_validation_invalid_run_backend(self, model_run, processor_config): + """Test that execute raises ValueError for invalid backend_config.""" backend = LocalPipelineBackend() - with pytest.raises(ValueError, match="run_backend must be a non-empty string"): - backend.execute(model_run, run_backend="") + with pytest.raises(TypeError, match="must be a BaseBackendConfig instance"): + backend.execute( + model_run, backend_config="invalid", processor=processor_config + ) - def test_execute_validation_invalid_processor(self, model_run): + def test_execute_validation_invalid_processor(self, model_run, backend_config): """Test that execute raises TypeError for invalid processor type.""" backend = LocalPipelineBackend() with pytest.raises( TypeError, match="must be a BasePostprocessorConfig instance" ): - backend.execute(model_run, processor="noop") + backend.execute(model_run, backend_config=backend_config, processor="noop") - def test_execute_generate_failure(self, model_run, processor_config): + def test_execute_generate_failure( + self, model_run, backend_config, processor_config + ): """Test pipeline failure during generate stage.""" backend = LocalPipelineBackend() with patch( "rompy.model.ModelRun.generate", side_effect=Exception("Generate failed") ): - result = backend.execute(model_run, processor=processor_config) + result = backend.execute( + model_run, backend_config=backend_config, processor=processor_config + ) assert result["success"] is False assert result["stage"] == "generate" assert "Generate failed" in result["message"] assert "generate" not in result["stages_completed"] - def test_execute_run_failure(self, model_run, tmp_path, processor_config): + def test_execute_run_failure( + self, model_run, tmp_path, backend_config, processor_config + ): """Test pipeline failure during run stage.""" backend = LocalPipelineBackend() @@ -379,14 +393,18 @@ def test_execute_run_failure(self, model_run, tmp_path, processor_config): with patch("rompy.model.ModelRun.generate", return_value=str(output_dir)): with patch("rompy.model.ModelRun.run", return_value=False): - result = backend.execute(model_run, processor=processor_config) + result = backend.execute( + model_run, backend_config=backend_config, processor=processor_config + ) assert result["success"] is False assert result["stage"] == "run" assert "generate" in result["stages_completed"] assert "run" not in result["stages_completed"] - def test_execute_run_exception(self, model_run, tmp_path, processor_config): + def test_execute_run_exception( + self, model_run, tmp_path, backend_config, processor_config + ): """Test pipeline failure during run stage with exception.""" backend = LocalPipelineBackend() @@ -395,13 +413,17 @@ def test_execute_run_exception(self, model_run, tmp_path, processor_config): with patch("rompy.model.ModelRun.generate", return_value=str(output_dir)): with patch("rompy.model.ModelRun.run", side_effect=Exception("Run failed")): - result = backend.execute(model_run, processor=processor_config) + result = backend.execute( + model_run, backend_config=backend_config, processor=processor_config + ) assert result["success"] is False assert result["stage"] == "run" assert "Run failed" in result["message"] - def test_execute_postprocess_failure(self, model_run, tmp_path, processor_config): + def test_execute_postprocess_failure( + self, model_run, tmp_path, backend_config, processor_config + ): """Test pipeline failure during postprocess stage.""" backend = LocalPipelineBackend() @@ -414,7 +436,11 @@ def test_execute_postprocess_failure(self, model_run, tmp_path, processor_config "rompy.model.ModelRun.postprocess", side_effect=Exception("Postprocess failed"), ): - result = backend.execute(model_run, processor=processor_config) + result = backend.execute( + model_run, + backend_config=backend_config, + processor=processor_config, + ) assert result["success"] is False assert result["stage"] == "postprocess" @@ -454,7 +480,7 @@ def test_execute_success_complete(self, model_run, tmp_path, processor_config): assert result["message"] == "Pipeline completed successfully" def test_execute_with_validation_failure( - self, model_run, tmp_path, processor_config + self, model_run, tmp_path, backend_config, processor_config ): """Test pipeline with stage validation failure.""" backend = LocalPipelineBackend() @@ -464,7 +490,10 @@ def test_execute_with_validation_failure( "rompy.model.ModelRun.generate", return_value=str(tmp_path / "nonexistent") ): result = backend.execute( - model_run, processor=processor_config, validate_stages=True + model_run, + backend_config=backend_config, + processor=processor_config, + validate_stages=True, ) assert result["success"] is False @@ -472,7 +501,7 @@ def test_execute_with_validation_failure( assert "not found after generation" in result["message"] def test_execute_with_cleanup_on_failure( - self, model_run, tmp_path, processor_config + self, model_run, tmp_path, backend_config, processor_config ): """Test pipeline with cleanup on failure.""" backend = LocalPipelineBackend() @@ -485,7 +514,10 @@ def test_execute_with_cleanup_on_failure( with patch("rompy.model.ModelRun.generate", return_value=str(output_dir)): with patch("rompy.model.ModelRun.run", return_value=False): result = backend.execute( - model_run, processor=processor_config, cleanup_on_failure=True + model_run, + backend_config=backend_config, + processor=processor_config, + cleanup_on_failure=True, ) assert result["success"] is False @@ -520,7 +552,7 @@ def test_cleanup_outputs_failure(self, model_run, tmp_path): backend._cleanup_outputs(model_run) def test_execute_postprocess_warning_on_failure( - self, model_run, tmp_path, processor_config + self, model_run, tmp_path, backend_config, processor_config ): """Test pipeline continues when postprocessing reports failure but doesn't raise.""" backend = LocalPipelineBackend() @@ -540,7 +572,11 @@ def test_execute_postprocess_warning_on_failure( "rompy.model.ModelRun.postprocess", return_value=mock_postprocess_result, ): - result = backend.execute(model_run, processor=processor_config) + result = backend.execute( + model_run, + backend_config=backend_config, + processor=processor_config, + ) # Pipeline should still succeed assert result["success"] is True diff --git a/tests/test_pipeline_config.py b/tests/test_pipeline_config.py new file mode 100644 index 0000000..93d030c --- /dev/null +++ b/tests/test_pipeline_config.py @@ -0,0 +1,389 @@ +""" +Integration tests for the new unified pipeline configuration structure. + +Tests cover: +- Loading pipeline configs with inline backend and postprocessor configs +- Loading pipeline configs with !include directives +- CLI flag overrides for backend and postprocessor configs +- Error handling for missing required sections +""" + +import tempfile +from datetime import datetime +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest +import yaml + +from rompy.backends import LocalConfig +from rompy.cli import load_config +from rompy.core.time import TimeRange +from rompy.core.yaml_loader import load_yaml_with_includes +from rompy.model import ModelRun +from rompy.postprocess.config import NoopPostprocessorConfig +from tests.test_helpers import DemoConfig + + +@pytest.fixture +def basic_inline_pipeline_config(): + """Create a basic inline pipeline configuration.""" + return { + "config": { + "run_id": "test_inline_pipeline", + "output_dir": "./output/test", + "delete_existing": True, + "period": { + "start": "2023-01-01T00:00:00", + "end": "2023-01-02T00:00:00", + "interval": "1H", + }, + "config": { + "model_type": "demo", + "arg1": "foo", + "arg2": "bar", + }, + }, + "backend": { + "type": "local", + "timeout": 3600, + "command": "echo 'test'", + }, + "postprocessor": { + "type": "noop", + }, + } + + +@pytest.fixture +def tmp_config_files(tmp_path): + """Create temporary config files for testing includes.""" + # Main config + model_config = { + "run_id": "test_with_includes", + "output_dir": str(tmp_path / "output"), + "delete_existing": True, + "period": { + "start": "2023-01-01T00:00:00", + "end": "2023-01-02T00:00:00", + "interval": "1H", + }, + "config": { + "model_type": "demo", + "arg1": "foo", + "arg2": "bar", + }, + } + + # Backend config + backend_config = { + "type": "local", + "timeout": 7200, + "command": "echo 'included backend'", + } + + # Postprocessor config + processor_config = { + "type": "noop", + "validate_outputs": False, + } + + # Write individual config files + model_config_path = tmp_path / "model_config.yml" + backend_config_path = tmp_path / "backend_config.yml" + processor_config_path = tmp_path / "processor_config.yml" + + with open(model_config_path, "w") as f: + yaml.dump(model_config, f) + + with open(backend_config_path, "w") as f: + yaml.dump(backend_config, f) + + with open(processor_config_path, "w") as f: + yaml.dump(processor_config, f) + + # Create pipeline config with includes + pipeline_config = { + "config": f"!include {model_config_path}", + "backend": f"!include {backend_config_path}", + "postprocessor": f"!include {processor_config_path}", + } + + pipeline_config_path = tmp_path / "pipeline_config.yml" + with open(pipeline_config_path, "w") as f: + # Write with !include tags (need to be careful with YAML formatting) + f.write(f"config: !include {model_config_path}\n") + f.write(f"backend: !include {backend_config_path}\n") + f.write(f"postprocessor: !include {processor_config_path}\n") + + return { + "pipeline_config": pipeline_config_path, + "model_config": model_config_path, + "backend_config": backend_config_path, + "processor_config": processor_config_path, + } + + +class TestPipelineConfigLoading: + """Test loading of pipeline configurations.""" + + def test_load_inline_pipeline_config(self, basic_inline_pipeline_config, tmp_path): + """Test loading a pipeline config with all sections inline.""" + config_path = tmp_path / "inline_pipeline.yml" + with open(config_path, "w") as f: + yaml.dump(basic_inline_pipeline_config, f) + + # Load the config + loaded_config = load_config(str(config_path)) + + # Verify all sections are present + assert "config" in loaded_config + assert "backend" in loaded_config + assert "postprocessor" in loaded_config + + # Verify config section + assert loaded_config["config"]["run_id"] == "test_inline_pipeline" + assert loaded_config["config"]["config"]["arg1"] == "foo" + + # Verify backend section + assert loaded_config["backend"]["type"] == "local" + assert loaded_config["backend"]["timeout"] == 3600 + + # Verify postprocessor section + assert loaded_config["postprocessor"]["type"] == "noop" + + def test_load_pipeline_config_with_includes(self, tmp_config_files): + """Test loading a pipeline config with !include directives.""" + # Load the pipeline config with includes + loaded_config = load_yaml_with_includes(tmp_config_files["pipeline_config"]) + + # Verify all sections are loaded from included files + assert "config" in loaded_config + assert "backend" in loaded_config + assert "postprocessor" in loaded_config + + # Verify config section + assert loaded_config["config"]["run_id"] == "test_with_includes" + assert loaded_config["config"]["config"]["arg1"] == "foo" + + # Verify backend section + assert loaded_config["backend"]["type"] == "local" + assert loaded_config["backend"]["timeout"] == 7200 + + # Verify postprocessor section + assert loaded_config["postprocessor"]["type"] == "noop" + + def test_load_minimal_pipeline_config(self, tmp_path): + """Test loading a minimal pipeline config with only config section.""" + minimal_config = { + "config": { + "run_id": "minimal_test", + "output_dir": "./output/minimal", + "period": { + "start": "2023-01-01T00:00:00", + "end": "2023-01-01T12:00:00", + "interval": "1H", + }, + "config": { + "model_type": "demo", + "arg1": "test", + "arg2": "value", + }, + } + } + + config_path = tmp_path / "minimal_pipeline.yml" + with open(config_path, "w") as f: + yaml.dump(minimal_config, f) + + loaded_config = load_config(str(config_path)) + + # Config section should be present + assert "config" in loaded_config + assert loaded_config["config"]["run_id"] == "minimal_test" + + # Backend and postprocessor should not be present + assert "backend" not in loaded_config or loaded_config.get("backend") is None + assert ( + "postprocessor" not in loaded_config + or loaded_config.get("postprocessor") is None + ) + + +class TestPipelineConfigExtraction: + """Test extraction of config sections for pipeline execution.""" + + def test_extract_config_section(self, basic_inline_pipeline_config): + """Test extracting the config section from pipeline config.""" + config_section = basic_inline_pipeline_config["config"] + + assert config_section["run_id"] == "test_inline_pipeline" + assert "period" in config_section + assert "config" in config_section + + def test_extract_backend_section(self, basic_inline_pipeline_config): + """Test extracting the backend section from pipeline config.""" + backend_section = basic_inline_pipeline_config["backend"] + + assert backend_section["type"] == "local" + assert backend_section["timeout"] == 3600 + + def test_extract_postprocessor_section(self, basic_inline_pipeline_config): + """Test extracting the postprocessor section from pipeline config.""" + processor_section = basic_inline_pipeline_config["postprocessor"] + + assert processor_section["type"] == "noop" + + +class TestPipelineConfigValidation: + """Test validation of pipeline configurations.""" + + def test_missing_config_section(self, tmp_path): + """Test that missing config section raises appropriate error.""" + invalid_config = { + "backend": {"type": "local"}, + "postprocessor": {"type": "noop"}, + } + + config_path = tmp_path / "invalid_pipeline.yml" + with open(config_path, "w") as f: + yaml.dump(invalid_config, f) + + # This should be caught when trying to use the config + loaded_config = load_config(str(config_path)) + + # Config section should be missing or None + assert "config" not in loaded_config or loaded_config.get("config") is None + + def test_invalid_backend_type(self, basic_inline_pipeline_config, tmp_path): + """Test that invalid backend type is handled.""" + basic_inline_pipeline_config["backend"]["type"] = "invalid_backend" + + config_path = tmp_path / "invalid_backend.yml" + with open(config_path, "w") as f: + yaml.dump(basic_inline_pipeline_config, f) + + loaded_config = load_config(str(config_path)) + + # Config should load, validation happens when instantiating backends + assert loaded_config["backend"]["type"] == "invalid_backend" + + def test_invalid_processor_type(self, basic_inline_pipeline_config, tmp_path): + """Test that invalid processor type is handled.""" + basic_inline_pipeline_config["postprocessor"]["type"] = "invalid_processor" + + config_path = tmp_path / "invalid_processor.yml" + with open(config_path, "w") as f: + yaml.dump(basic_inline_pipeline_config, f) + + loaded_config = load_config(str(config_path)) + + # Config should load, validation happens when instantiating processors + assert loaded_config["postprocessor"]["type"] == "invalid_processor" + + +class TestPipelineConfigComposition: + """Test composition of configs using includes and overrides.""" + + def test_partial_include_with_inline(self, tmp_config_files, tmp_path): + """Test mixing included and inline configs.""" + # Create a pipeline config with some includes and some inline + pipeline_config_path = tmp_path / "mixed_pipeline.yml" + with open(pipeline_config_path, "w") as f: + f.write(f"config: !include {tmp_config_files['model_config']}\n") + f.write("backend:\n") + f.write(" type: local\n") + f.write(" timeout: 5000\n") + f.write(f"postprocessor: !include {tmp_config_files['processor_config']}\n") + + loaded_config = load_yaml_with_includes(pipeline_config_path) + + # Verify mixed loading + assert loaded_config["config"]["run_id"] == "test_with_includes" + assert loaded_config["backend"]["type"] == "local" + assert loaded_config["backend"]["timeout"] == 5000 + assert loaded_config["postprocessor"]["type"] == "noop" + + def test_nested_includes(self, tmp_path): + """Test nested include directives.""" + # Create a nested structure + base_backend = { + "type": "local", + "timeout": 3600, + } + + base_backend_path = tmp_path / "base_backend.yml" + with open(base_backend_path, "w") as f: + yaml.dump(base_backend, f) + + extended_config = { + "config": { + "run_id": "nested_test", + "output_dir": "./output", + "period": { + "start": "2023-01-01T00:00:00", + "end": "2023-01-01T12:00:00", + "interval": "1H", + }, + "config": {"model_type": "demo"}, + }, + } + + pipeline_config_path = tmp_path / "nested_pipeline.yml" + with open(pipeline_config_path, "w") as f: + yaml.dump(extended_config, f) + f.write(f"backend: !include {base_backend_path}\n") + + loaded_config = load_yaml_with_includes(pipeline_config_path) + + assert loaded_config["backend"]["type"] == "local" + assert loaded_config["config"]["run_id"] == "nested_test" + + +class TestBackwardCompatibilityWarning: + """Test that using old parameter names triggers warnings.""" + + def test_old_run_backend_parameter_warning(self, tmp_path, caplog): + """Test that passing run_backend string logs a deprecation warning.""" + import logging + from rompy.pipeline import LocalPipelineBackend + + # Create the output directory structure that the pipeline expects + output_dir = tmp_path / "test_run" + output_dir.mkdir(parents=True, exist_ok=True) + + model_run = ModelRun( + run_id="test_run", + period=TimeRange( + start=datetime(2023, 1, 1), + end=datetime(2023, 1, 2), + interval="1H", + ), + output_dir=str(tmp_path), + config=DemoConfig(arg1="foo", arg2="bar"), + ) + + backend = LocalPipelineBackend() + processor_config = NoopPostprocessorConfig() + + with caplog.at_level(logging.WARNING): + with patch("rompy.model.ModelRun.generate", return_value=str(output_dir)): + with patch("rompy.model.ModelRun.run", return_value=True): + with patch( + "rompy.model.ModelRun.postprocess", + return_value={"success": True}, + ): + result = backend.execute( + model_run, + run_backend="local", # Old parameter name + processor=processor_config, + ) + + # Check that deprecation warning was logged + assert any( + "run_backend" in record.message and "deprecated" in record.message + for record in caplog.records + ) + + # Should still work with backward compatibility + assert result["success"] is True diff --git a/tests/test_yaml_loader.py b/tests/test_yaml_loader.py new file mode 100644 index 0000000..e9fdbc3 --- /dev/null +++ b/tests/test_yaml_loader.py @@ -0,0 +1,317 @@ +""" +Tests for YAML loader with include support. +""" + +import os +import tempfile +from pathlib import Path + +import pytest +import yaml + +from rompy.core.yaml_loader import ( + IncludeLoader, + load_yaml_with_includes, + safe_load_with_includes, +) + + +@pytest.fixture +def temp_yaml_dir(tmp_path): + """Create a temporary directory with test YAML files.""" + return tmp_path + + +def test_basic_include(temp_yaml_dir): + """Test basic file inclusion.""" + # Create included file + included = temp_yaml_dir / "included.yaml" + included.write_text("key1: value1\nkey2: value2\n") + + # Create main file with include + main = temp_yaml_dir / "main.yaml" + main.write_text("config: !include included.yaml\n") + + # Load and verify + result = load_yaml_with_includes(main) + assert result == {"config": {"key1": "value1", "key2": "value2"}} + + +def test_absolute_path_include(temp_yaml_dir): + """Test inclusion with absolute path.""" + # Create included file + included = temp_yaml_dir / "included.yaml" + included.write_text("data: test\n") + + # Create main file with absolute path include + main = temp_yaml_dir / "main.yaml" + main.write_text(f"config: !include {included}\n") + + # Load and verify + result = load_yaml_with_includes(main) + assert result == {"config": {"data": "test"}} + + +def test_relative_path_include(temp_yaml_dir): + """Test inclusion with relative path in subdirectory.""" + # Create subdirectory with included file + subdir = temp_yaml_dir / "configs" + subdir.mkdir() + included = subdir / "included.yaml" + included.write_text("nested: data\n") + + # Create main file with relative include + main = temp_yaml_dir / "main.yaml" + main.write_text("config: !include configs/included.yaml\n") + + # Load and verify + result = load_yaml_with_includes(main) + assert result == {"config": {"nested": "data"}} + + +def test_nested_include(temp_yaml_dir): + """Test nested includes (include within include).""" + # Create deepest file + level2 = temp_yaml_dir / "level2.yaml" + level2.write_text("deep: value\n") + + # Create middle file that includes level2 + level1 = temp_yaml_dir / "level1.yaml" + level1.write_text("middle: !include level2.yaml\n") + + # Create main file that includes level1 + main = temp_yaml_dir / "main.yaml" + main.write_text("top: !include level1.yaml\n") + + # Load and verify + result = load_yaml_with_includes(main) + assert result == {"top": {"middle": {"deep": "value"}}} + + +def test_multiple_includes(temp_yaml_dir): + """Test multiple includes in same file.""" + # Create multiple included files + config1 = temp_yaml_dir / "config1.yaml" + config1.write_text("data1: value1\n") + + config2 = temp_yaml_dir / "config2.yaml" + config2.write_text("data2: value2\n") + + # Create main file with multiple includes + main = temp_yaml_dir / "main.yaml" + main.write_text( + """ +config: !include config1.yaml +backend: !include config2.yaml +""" + ) + + # Load and verify + result = load_yaml_with_includes(main) + assert result == {"config": {"data1": "value1"}, "backend": {"data2": "value2"}} + + +def test_include_with_inline_content(temp_yaml_dir): + """Test mixing includes with inline content.""" + # Create included file + included = temp_yaml_dir / "backend.yaml" + included.write_text("type: local\ntimeout: 3600\n") + + # Create main file mixing include and inline + main = temp_yaml_dir / "main.yaml" + main.write_text( + """ +config: + run_id: test + period: 1d +backend: !include backend.yaml +postprocessor: + type: noop + timeout: 1800 +""" + ) + + # Load and verify + result = load_yaml_with_includes(main) + assert result["config"]["run_id"] == "test" + assert result["backend"]["type"] == "local" + assert result["postprocessor"]["type"] == "noop" + + +def test_missing_file_error(temp_yaml_dir): + """Test error handling for missing included file.""" + main = temp_yaml_dir / "main.yaml" + main.write_text("config: !include nonexistent.yaml\n") + + with pytest.raises(FileNotFoundError) as exc_info: + load_yaml_with_includes(main) + + assert "nonexistent.yaml" in str(exc_info.value) + + +def test_circular_include_detection(temp_yaml_dir): + """Test detection of circular includes.""" + # Create file1 that includes file2 + file1 = temp_yaml_dir / "file1.yaml" + file1.write_text("data: !include file2.yaml\n") + + # Create file2 that includes file1 (circular) + file2 = temp_yaml_dir / "file2.yaml" + file2.write_text("data: !include file1.yaml\n") + + # Should raise error about circular dependency + with pytest.raises(yaml.YAMLError) as exc_info: + load_yaml_with_includes(file1) + + assert "Circular include" in str(exc_info.value) + + +def test_max_include_depth(temp_yaml_dir): + """Test maximum include depth limit.""" + # Create a chain of includes exceeding max depth + for i in range(12): # Exceeds MAX_INCLUDE_DEPTH of 10 + if i < 11: + content = f"data: !include file{i + 1}.yaml\n" + else: + content = "data: final\n" + + file = temp_yaml_dir / f"file{i}.yaml" + file.write_text(content) + + # Should raise error about max depth + with pytest.raises(ValueError) as exc_info: + load_yaml_with_includes(temp_yaml_dir / "file0.yaml") + + assert "Include depth exceeds maximum" in str(exc_info.value) + + +def test_environment_variable_expansion(temp_yaml_dir): + """Test environment variable expansion in include paths.""" + # Set environment variable + os.environ["TEST_CONFIG_DIR"] = str(temp_yaml_dir) + + try: + # Create included file + included = temp_yaml_dir / "config.yaml" + included.write_text("expanded: true\n") + + # Create main file with env var in path + main = temp_yaml_dir / "main.yaml" + main.write_text("config: !include $TEST_CONFIG_DIR/config.yaml\n") + + # Load and verify + result = load_yaml_with_includes(main) + assert result == {"config": {"expanded": True}} + + finally: + # Clean up env var + del os.environ["TEST_CONFIG_DIR"] + + +def test_safe_load_with_includes_string(): + """Test loading YAML from string with includes.""" + with tempfile.TemporaryDirectory() as tmpdir: + tmpdir = Path(tmpdir) + + # Create included file + included = tmpdir / "data.yaml" + included.write_text("value: 123\n") + + # Load from string + yaml_content = "config: !include data.yaml\n" + result = safe_load_with_includes(yaml_content, root_dir=tmpdir) + + assert result == {"config": {"value": 123}} + + +def test_include_preserves_data_types(temp_yaml_dir): + """Test that includes preserve YAML data types.""" + # Create included file with various types + included = temp_yaml_dir / "types.yaml" + included.write_text( + """ +string: "text" +integer: 42 +float: 3.14 +boolean: true +null_value: null +list: + - item1 + - item2 +nested: + key: value +""" + ) + + # Create main file + main = temp_yaml_dir / "main.yaml" + main.write_text("data: !include types.yaml\n") + + # Load and verify types + result = load_yaml_with_includes(main) + data = result["data"] + + assert data["string"] == "text" + assert data["integer"] == 42 + assert data["float"] == 3.14 + assert data["boolean"] is True + assert data["null_value"] is None + assert data["list"] == ["item1", "item2"] + assert data["nested"]["key"] == "value" + + +def test_include_scalar_value(temp_yaml_dir): + """Test including a file with scalar value (not dict).""" + # Create included file with scalar + included = temp_yaml_dir / "scalar.yaml" + included.write_text("just a string value\n") + + # Create main file + main = temp_yaml_dir / "main.yaml" + main.write_text("value: !include scalar.yaml\n") + + # Load and verify + result = load_yaml_with_includes(main) + assert result == {"value": "just a string value"} + + +def test_include_list_value(temp_yaml_dir): + """Test including a file with list as root.""" + # Create included file with list + included = temp_yaml_dir / "list.yaml" + included.write_text( + """ +- item1 +- item2 +- item3 +""" + ) + + # Create main file + main = temp_yaml_dir / "main.yaml" + main.write_text("items: !include list.yaml\n") + + # Load and verify + result = load_yaml_with_includes(main) + assert result == {"items": ["item1", "item2", "item3"]} + + +def test_file_not_found_main(): + """Test error when main file doesn't exist.""" + with pytest.raises(FileNotFoundError): + load_yaml_with_includes("nonexistent_main.yaml") + + +def test_yaml_syntax_error_in_included_file(temp_yaml_dir): + """Test handling of YAML syntax errors in included files.""" + # Create included file with invalid YAML (unclosed bracket) + included = temp_yaml_dir / "invalid.yaml" + included.write_text("key: [unclosed\n") + + # Create main file + main = temp_yaml_dir / "main.yaml" + main.write_text("config: !include invalid.yaml\n") + + # Should raise YAML error + with pytest.raises(yaml.YAMLError): + load_yaml_with_includes(main)