Skip to content

Commit

Permalink
UW-631 Provide mechanism to schema-check external drivers (#534)
Browse files Browse the repository at this point in the history
* Change validate_yaml to validate_external

* Update _validate method defintion

* Add schema_file param to all Classes in drivers.driver

* test coverage

* Updated Asset driver class tests

* Add external validation to Driver class

* Remove static docstrings

* Updated docstring test

* PR comment, add platform validation

* Update src/uwtools/drivers/driver.py

Co-authored-by: Paul Madden <[email protected]>

* Update src/uwtools/drivers/driver.py

Co-authored-by: Paul Madden <[email protected]>

* Update src/uwtools/drivers/driver.py

Co-authored-by: Paul Madden <[email protected]>

* Update src/uwtools/drivers/driver.py

Co-authored-by: Paul Madden <[email protected]>

* Update src/uwtools/drivers/driver.py

Co-authored-by: Paul Madden <[email protected]>

* Update src/uwtools/drivers/driver.py

Co-authored-by: Paul Madden <[email protected]>

* Remove static docstrings

Co-authored-by: Paul Madden <[email protected]>

---------

Co-authored-by: Paul Madden <[email protected]>
  • Loading branch information
NaureenBharwaniNOAA and maddenp-noaa authored Jul 19, 2024
1 parent 113c35b commit 3bbf437
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 32 deletions.
4 changes: 2 additions & 2 deletions src/uwtools/api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from uwtools.config.formats.yaml import YAMLConfig as _YAMLConfig
from uwtools.config.tools import compare_configs as _compare
from uwtools.config.tools import realize_config as _realize
from uwtools.config.validator import validate_yaml as _validate_yaml
from uwtools.config.validator import validate_external as _validate_external
from uwtools.utils.api import ensure_data_source as _ensure_data_source
from uwtools.utils.api import str2path as _str2path
from uwtools.utils.file import FORMAT as _FORMAT
Expand Down Expand Up @@ -177,7 +177,7 @@ def validate(
:param stdin_ok: OK to read from ``stdin``?
:return: ``True`` if the YAML file conforms to the schema, ``False`` otherwise
"""
return _validate_yaml(
return _validate_external(
schema_file=_str2path(schema_file), config=_ensure_data_source(_str2path(config), stdin_ok)
)

Expand Down
4 changes: 2 additions & 2 deletions src/uwtools/config/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ def validate_internal(
log.info("Validating config against internal schema %s", schema_name)
schema_file = get_schema_file(schema_name)
log.debug("Using schema file: %s", schema_file)
if not validate_yaml(config=config, schema_file=schema_file):
if not validate_external(config=config, schema_file=schema_file):
raise UWConfigError("YAML validation errors")


def validate_yaml(
def validate_external(
schema_file: Path, config: Union[dict, YAMLConfig, Optional[Path]] = None
) -> bool:
"""
Expand Down
74 changes: 61 additions & 13 deletions src/uwtools/drivers/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from uwtools.config.formats.base import Config
from uwtools.config.formats.yaml import YAMLConfig
from uwtools.config.validator import get_schema_file, validate, validate_internal
from uwtools.config.validator import get_schema_file, validate, validate_external, validate_internal
from uwtools.exceptions import UWConfigError
from uwtools.logging import log
from uwtools.scheduler import JobScheduler
Expand All @@ -39,6 +39,7 @@ def __init__(
config: Optional[Union[dict, Path]] = None,
dry_run: bool = False,
key_path: Optional[list[str]] = None,
schema_file: Optional[Path] = None,
) -> None:
self._config = YAMLConfig(config=config)
self._config.dereference(
Expand All @@ -50,7 +51,7 @@ def __init__(
)
for key in key_path or []:
self._config = self._config[key]
self._validate()
self._validate(schema_file)
dryrun(enable=dry_run)

def __repr__(self) -> str:
Expand Down Expand Up @@ -199,12 +200,15 @@ def _taskname(self, suffix: str) -> str:
)
return " ".join(filter(None, [timestr, self._driver_name, suffix]))

def _validate(self) -> None:
def _validate(self, schema_file: Optional[Path] = None) -> None:
"""
Perform all necessary schema validation.
"""
schema_name = self._driver_name.replace("_", "-")
validate_internal(schema_name=schema_name, config=self._config)
if schema_file:
validate_external(schema_file=schema_file, config=self._config)
else:
validate_internal(schema_name=schema_name, config=self._config)


class AssetsCycleBased(Assets):
Expand All @@ -218,8 +222,15 @@ def __init__(
config: Optional[Union[dict, Path]] = None,
dry_run: bool = False,
key_path: Optional[list[str]] = None,
schema_file: Optional[Path] = None,
):
super().__init__(cycle=cycle, config=config, dry_run=dry_run, key_path=key_path)
super().__init__(
cycle=cycle,
config=config,
dry_run=dry_run,
key_path=key_path,
schema_file=schema_file,
)
self._cycle = cycle


Expand All @@ -235,9 +246,15 @@ def __init__(
config: Optional[Union[dict, Path]] = None,
dry_run: bool = False,
key_path: Optional[list[str]] = None,
schema_file: Optional[Path] = None,
):
super().__init__(
cycle=cycle, leadtime=leadtime, config=config, dry_run=dry_run, key_path=key_path
cycle=cycle,
leadtime=leadtime,
config=config,
dry_run=dry_run,
key_path=key_path,
schema_file=schema_file,
)
self._cycle = cycle
self._leadtime = leadtime
Expand All @@ -253,8 +270,14 @@ def __init__(
config: Optional[Union[dict, Path]] = None,
dry_run: bool = False,
key_path: Optional[list[str]] = None,
schema_file: Optional[Path] = None,
):
super().__init__(config=config, dry_run=dry_run, key_path=key_path)
super().__init__(
config=config,
dry_run=dry_run,
key_path=key_path,
schema_file=schema_file,
)


class Driver(Assets):
Expand All @@ -270,9 +293,15 @@ def __init__(
dry_run: bool = False,
key_path: Optional[list[str]] = None,
batch: bool = False,
schema_file: Optional[Path] = None,
):
super().__init__(
cycle=cycle, leadtime=leadtime, config=config, dry_run=dry_run, key_path=key_path
cycle=cycle,
leadtime=leadtime,
config=config,
dry_run=dry_run,
key_path=key_path,
schema_file=schema_file,
)
self._batch = batch

Expand Down Expand Up @@ -410,12 +439,15 @@ def _scheduler(self) -> JobScheduler:
"""
return JobScheduler.get_scheduler(self._resources)

def _validate(self) -> None:
def _validate(self, schema_file: Optional[Path] = None) -> None:
"""
Perform all necessary schema validation.
"""
for schema_name in (self._driver_name.replace("_", "-"), "platform"):
validate_internal(schema_name=schema_name, config=self._config)
if schema_file:
validate_external(schema_file=schema_file, config=self._config)
else:
validate_internal(schema_name=self._driver_name.replace("_", "-"), config=self._config)
validate_internal(schema_name="platform", config=self._config)

def _write_runscript(self, path: Path, envvars: Optional[dict[str, str]] = None) -> None:
"""
Expand Down Expand Up @@ -448,9 +480,15 @@ def __init__(
dry_run: bool = False,
key_path: Optional[list[str]] = None,
batch: bool = False,
schema_file: Optional[Path] = None,
):
super().__init__(
cycle=cycle, config=config, dry_run=dry_run, key_path=key_path, batch=batch
cycle=cycle,
config=config,
dry_run=dry_run,
key_path=key_path,
batch=batch,
schema_file=schema_file,
)
self._cycle = cycle

Expand All @@ -468,6 +506,7 @@ def __init__(
dry_run: bool = False,
key_path: Optional[list[str]] = None,
batch: bool = False,
schema_file: Optional[Path] = None,
):
super().__init__(
cycle=cycle,
Expand All @@ -476,6 +515,7 @@ def __init__(
dry_run=dry_run,
key_path=key_path,
batch=batch,
schema_file=schema_file,
)
self._cycle = cycle
self._leadtime = leadtime
Expand All @@ -492,8 +532,15 @@ def __init__(
dry_run: bool = False,
key_path: Optional[list[str]] = None,
batch: bool = False,
schema_file: Optional[Path] = None,
):
super().__init__(config=config, dry_run=dry_run, key_path=key_path, batch=batch)
super().__init__(
config=config,
dry_run=dry_run,
key_path=key_path,
batch=batch,
schema_file=schema_file,
)


DriverT = Union[type[Assets], type[Driver]]
Expand All @@ -515,6 +562,7 @@ def _add_docstring(class_: type, omit: Optional[list[str]] = None) -> None:
:param dry_run: Run in dry-run mode?
:param key_path: Keys leading through the config to the driver's configuration block.
:param batch: Run component via the batch system?
:param schema_file: Path to schema file to use to validate an external driver.
"""
setattr(
class_,
Expand Down
2 changes: 1 addition & 1 deletion src/uwtools/rocoto.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from lxml.etree import Element, SubElement, _Element

from uwtools.config.formats.yaml import YAMLConfig
from uwtools.config.validator import validate_yaml
from uwtools.config.validator import validate_external as validate_yaml
from uwtools.exceptions import UWConfigError, UWError
from uwtools.logging import log
from uwtools.utils.file import readable, resource_path, writable
Expand Down
8 changes: 4 additions & 4 deletions src/uwtools/tests/api/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ def test_realize_to_dict():
@mark.parametrize("cfg", [{"foo": "bar"}, YAMLConfig(config={})])
def test_validate(cfg):
kwargs: dict = {"schema_file": "schema-file", "config": cfg}
with patch.object(config, "_validate_yaml", return_value=True) as _validate_yaml:
with patch.object(config, "_validate_external", return_value=True) as _validate_external:
assert config.validate(**kwargs)
_validate_yaml.assert_called_once_with(
_validate_external.assert_called_once_with(
schema_file=Path(kwargs["schema_file"]), config=kwargs["config"]
)

Expand All @@ -106,6 +106,6 @@ def test_validate_config_file(tmp_path):
with open(cfg, "w", encoding="utf-8") as f:
yaml.dump({}, f)
kwargs: dict = {"schema_file": "schema-file", "config": cfg}
with patch.object(config, "_validate_yaml", return_value=True) as _validate_yaml:
with patch.object(config, "_validate_external", return_value=True) as _validate_external:
assert config.validate(**kwargs)
_validate_yaml.assert_called_once_with(schema_file=Path(kwargs["schema_file"]), config=cfg)
_validate_external.assert_called_once_with(schema_file=Path(kwargs["schema_file"]), config=cfg)
4 changes: 2 additions & 2 deletions src/uwtools/tests/config/test_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,10 @@ def test_validate_internal_ok(schema_file):
validator.validate_internal(schema_name="a", config={"color": "blue"})


def test_validate_yaml(assets, config, schema):
def test_validate_external(assets, config, schema):
schema_file, _, cfgobj = assets
with patch.object(validator, "validate") as validate:
validator.validate_yaml(schema_file=schema_file, config=cfgobj)
validator.validate_external(schema_file=schema_file, config=cfgobj)
validate.assert_called_once_with(schema=schema, config=config)


Expand Down
31 changes: 27 additions & 4 deletions src/uwtools/tests/drivers/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import logging
from pathlib import Path
from textwrap import dedent
from typing import Optional
from unittest.mock import Mock, PropertyMock, patch

import yaml
Expand Down Expand Up @@ -42,7 +43,7 @@ def provisioned_rundir(self):
def _driver_name(self) -> str:
return "concrete"

def _validate(self) -> None:
def _validate(self, schema_file: Optional[Path] = None) -> None:
pass


Expand Down Expand Up @@ -235,7 +236,7 @@ def test_Assets__rundir(assetsobj):
assert assetsobj._rundir == Path(assetsobj._driver_config["rundir"])


def test_Assets__validate(assetsobj):
def test_Assets__validate_internal(assetsobj):
with patch.object(assetsobj, "_validate", driver.Assets._validate):
with patch.object(driver, "validate_internal") as validate_internal:
assetsobj._validate(assetsobj)
Expand All @@ -245,6 +246,17 @@ def test_Assets__validate(assetsobj):
}


def test_Assets__validate_external(config):
schema_file = Path("/path/to/jsonschema")
with patch.object(ConcreteAssetsTimeInvariant, "_validate", driver.Assets._validate):
with patch.object(driver, "validate_external") as validate_external:
assetsobj = ConcreteAssetsTimeInvariant(schema_file=schema_file, config=config)
assert validate_external.call_args_list[0].kwargs == {
"schema_file": schema_file,
"config": assetsobj._config,
}


# Driver Tests


Expand Down Expand Up @@ -479,7 +491,7 @@ def test_Driver__scheduler(driverobj):
JobScheduler.get_scheduler.assert_called_with(driverobj._resources)


def test_Driver__validate(assetsobj):
def test_Driver__validate_internal(assetsobj):
with patch.object(assetsobj, "_validate", driver.Driver._validate):
with patch.object(driver, "validate_internal") as validate_internal:
assetsobj._validate(assetsobj)
Expand All @@ -493,6 +505,17 @@ def test_Driver__validate(assetsobj):
}


def test_Driver__validate_external(config):
schema_file = Path("/path/to/jsonschema")
with patch.object(ConcreteAssetsTimeInvariant, "_validate", driver.Driver._validate):
with patch.object(driver, "validate_external") as validate_external:
assetsobj = ConcreteAssetsTimeInvariant(schema_file=schema_file, config=config)
assert validate_external.call_args_list[0].kwargs == {
"schema_file": schema_file,
"config": assetsobj._config,
}


def test_Driver__write_runscript(driverobj):
rundir = driverobj._driver_config["rundir"]
path = Path(rundir) / "runscript"
Expand Down Expand Up @@ -526,6 +549,6 @@ class C:
assert getattr(C, "__doc__") is None
with patch.object(driver, "C", C, create=True):
class_ = driver.C # type: ignore # pylint: disable=no-member
omit = ["cycle", "leadtime", "config", "dry_run", "key_path", "batch"]
omit = ["cycle", "leadtime", "config", "dry_run", "key_path", "batch", "schema_file"]
driver._add_docstring(class_=class_, omit=omit)
assert getattr(C, "__doc__").strip() == "The driver."
5 changes: 4 additions & 1 deletion src/uwtools/tests/drivers/test_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"""
Tests for uwtools.drivers.support module.
"""
from pathlib import Path
from typing import Optional

from iotaa import asset, external, task, tasks

from uwtools.drivers import support
Expand Down Expand Up @@ -48,7 +51,7 @@ def _resources(self):
def _taskname(self, suffix):
pass

def _validate(self):
def _validate(self, schema_file: Optional[Path] = None):
pass

assert support.tasks(SomeDriver) == {
Expand Down
6 changes: 3 additions & 3 deletions src/uwtools/tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,13 +332,13 @@ def test__dispatch_config_validate_config_obj():
STR.schemafile: Path("/path/to/a.jsonschema"),
STR.infile: Path("/path/to/config.yaml"),
}
with patch.object(uwtools.api.config, "_validate_yaml") as _validate_yaml:
with patch.object(uwtools.api.config, "_validate_external") as _validate_external:
cli._dispatch_config_validate(_dispatch_config_validate_args)
_validate_yaml_args = {
_validate_external_args = {
STR.schemafile: _dispatch_config_validate_args[STR.schemafile],
STR.config: _dispatch_config_validate_args[STR.infile],
}
_validate_yaml.assert_called_once_with(**_validate_yaml_args)
_validate_external.assert_called_once_with(**_validate_external_args)


@mark.parametrize(
Expand Down

0 comments on commit 3bbf437

Please sign in to comment.