config: generalize config_validate into a shared schema engine
require_schema(config, schema) is a generic recursive declarative
checker (nested-dict schema, leaves name required type(s), each
intermediate object node gets its own dict-type check so a wrong-typed
section is named directly instead of surfacing as a confusing "missing
child" error). validate_temp_ctrl() is now just require_schema(config,
TEMPCTRL_SCHEMA) against a declarative schema dict, replacing the
hand-rolled nested loops.
utils/sud_validate.py's static structural check ('steps' must be a
list) now goes through the same require_schema()/SUD_SCHEMA - one
shared mechanism for both server config.json and sud.json documents.
The genuinely dynamic per-step check (ramp.rate required only when a
step sets its own temperature) stays hand-written, since it depends on
the document's content, not just its shape.
Added TestRequireSchema, exercising the generic engine directly
against a schema unrelated to TempCtrl.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F4oEE99rkKVU8L8tkzXWdG
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
import unittest
|
import unittest
|
||||||
|
|
||||||
from utils.config_validate import ConfigError, require, validate_temp_ctrl
|
from utils.config_validate import ConfigError, require, require_schema, validate_temp_ctrl
|
||||||
|
|
||||||
|
|
||||||
def _valid_temp_ctrl_config():
|
def _valid_temp_ctrl_config():
|
||||||
@@ -42,6 +42,36 @@ class TestRequire(unittest.TestCase):
|
|||||||
self.assertEqual(require({"a": 1.5}, "a", type=(int, float)), 1.5)
|
self.assertEqual(require({"a": 1.5}, "a", type=(int, float)), 1.5)
|
||||||
|
|
||||||
|
|
||||||
|
class TestRequireSchema(unittest.TestCase):
|
||||||
|
"""require_schema() is the generic engine both validate_temp_ctrl()
|
||||||
|
(config.json's TempCtrl section) and utils/sud_validate.py's
|
||||||
|
validate_sud() (a sud.json's fixed 'steps' shape) are built on - tested
|
||||||
|
here directly, against a schema unrelated to either, to confirm it's
|
||||||
|
not accidentally coupled to TempCtrl's specific shape."""
|
||||||
|
|
||||||
|
SCHEMA = {'a': {'b': int, 'c': (int, float)}, 'd': str}
|
||||||
|
|
||||||
|
def test_valid_document_passes(self):
|
||||||
|
require_schema({'a': {'b': 1, 'c': 2.5}, 'd': 'x'}, self.SCHEMA)
|
||||||
|
|
||||||
|
def test_missing_nested_leaf_raises(self):
|
||||||
|
with self.assertRaisesRegex(ConfigError, r'a\.b'):
|
||||||
|
require_schema({'a': {'c': 2.5}, 'd': 'x'}, self.SCHEMA)
|
||||||
|
|
||||||
|
def test_missing_top_level_leaf_raises(self):
|
||||||
|
with self.assertRaisesRegex(ConfigError, r'missing required config key: d$'):
|
||||||
|
require_schema({'a': {'b': 1, 'c': 2.5}}, self.SCHEMA)
|
||||||
|
|
||||||
|
def test_wrong_type_on_nested_object_itself_is_named_directly(self):
|
||||||
|
# 'a' present but not an object - must be reported as 'a' itself,
|
||||||
|
# not as a confusing "missing a.b"/"missing a.c".
|
||||||
|
with self.assertRaisesRegex(ConfigError, r'^config key a must be dict'):
|
||||||
|
require_schema({'a': 'not an object', 'd': 'x'}, self.SCHEMA)
|
||||||
|
|
||||||
|
def test_extra_keys_not_in_schema_are_ignored(self):
|
||||||
|
require_schema({'a': {'b': 1, 'c': 2.5}, 'd': 'x', 'extra': 'stray'}, self.SCHEMA)
|
||||||
|
|
||||||
|
|
||||||
class TestValidateTempCtrl(unittest.TestCase):
|
class TestValidateTempCtrl(unittest.TestCase):
|
||||||
def test_valid_config_passes(self):
|
def test_valid_config_passes(self):
|
||||||
validate_temp_ctrl(_valid_temp_ctrl_config())
|
validate_temp_ctrl(_valid_temp_ctrl_config())
|
||||||
|
|||||||
+49
-11
@@ -6,6 +6,10 @@ class ConfigError(Exception):
|
|||||||
|
|
||||||
|
|
||||||
def require(config, path, type=None):
|
def require(config, path, type=None):
|
||||||
|
"""Generic dotted-path lookup against any nested-dict document - not
|
||||||
|
tied to server config.json's own shape, so this is exactly as usable
|
||||||
|
for a sud.json schedule (see utils/sud_validate.py) as for TempCtrl
|
||||||
|
below."""
|
||||||
node = config
|
node = config
|
||||||
parts = path.split('.')
|
parts = path.split('.')
|
||||||
for i, key in enumerate(parts):
|
for i, key in enumerate(parts):
|
||||||
@@ -18,18 +22,52 @@ def require(config, path, type=None):
|
|||||||
return node
|
return node
|
||||||
|
|
||||||
|
|
||||||
|
def require_schema(config, schema, _prefix=''):
|
||||||
|
"""Generic declarative counterpart to require(): schema mirrors the
|
||||||
|
document's own shape, with every leaf naming the required type(s) (see
|
||||||
|
TEMPCTRL_SCHEMA below for an example) - one shared mechanism for any
|
||||||
|
config-shaped document with a fixed structure, whether that's server
|
||||||
|
config.json's TempCtrl section or (for the parts of it that are static -
|
||||||
|
see utils/sud_validate.py for the parts that aren't, e.g. per-step
|
||||||
|
conditional requirements) a sud.json schedule.
|
||||||
|
|
||||||
|
A schema value that's itself a dict describes a required sub-object:
|
||||||
|
checked (and reported, on failure, as its own dotted path) before
|
||||||
|
recursing into it, so a wrong-typed intermediate node (e.g. 'TempCtrl'
|
||||||
|
itself not being an object) is named directly rather than surfacing as
|
||||||
|
a confusing "missing" error on one of its children instead."""
|
||||||
|
for key, spec in schema.items():
|
||||||
|
path = '{}.{}'.format(_prefix, key) if _prefix else key
|
||||||
|
if isinstance(spec, dict):
|
||||||
|
require(config, path, type=dict)
|
||||||
|
require_schema(config, spec, path)
|
||||||
|
else:
|
||||||
|
require(config, path, type=spec)
|
||||||
|
|
||||||
|
|
||||||
|
_GAIN_SCHEMA = {'kp': (int, float), 'ki': (int, float), 'kd': (int, float), 'kt': (int, float)}
|
||||||
|
|
||||||
|
# Only the gains every pid_type actually reads unconditionally are
|
||||||
|
# required; anything with its own default elsewhere (Outer.y_hold_min,
|
||||||
|
# Inner.Hold.yi_max, the whole Thresholds section - see
|
||||||
|
# temp_controller_base.py/temp_controller_fsm.py's DEFAULT_THRESHOLDS)
|
||||||
|
# stays optional here too, on purpose.
|
||||||
|
TEMPCTRL_SCHEMA = {
|
||||||
|
'TempCtrl': {
|
||||||
|
'pid_type': str,
|
||||||
|
'Outer': _GAIN_SCHEMA,
|
||||||
|
'Inner': {
|
||||||
|
'Heat': _GAIN_SCHEMA,
|
||||||
|
'Hold': _GAIN_SCHEMA,
|
||||||
|
'Cool': _GAIN_SCHEMA,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def validate_temp_ctrl(config):
|
def validate_temp_ctrl(config):
|
||||||
"""Checked eagerly at startup so a missing/misnamed PID gain fails with
|
"""Checked eagerly at startup so a missing/misnamed PID gain fails with
|
||||||
a clear message here instead of a bare KeyError mid-__init__ or on the
|
a clear message here instead of a bare KeyError mid-__init__ or on the
|
||||||
first process() tick (this bit us once already: config.json.sim's gain/
|
first process() tick (this bit us once already: config.json.sim's gain/
|
||||||
Model nesting mismatch). Only the gains every pid_type actually reads
|
Model nesting mismatch)."""
|
||||||
unconditionally are required; anything with its own default elsewhere
|
require_schema(config, TEMPCTRL_SCHEMA)
|
||||||
(Outer.y_hold_min, Inner.Hold.yi_max, the whole Thresholds section -
|
|
||||||
see temp_controller_base.py/temp_controller_fsm.py's DEFAULT_THRESHOLDS)
|
|
||||||
stays optional here too, on purpose."""
|
|
||||||
require(config, 'TempCtrl', dict)
|
|
||||||
require(config, 'TempCtrl.pid_type', str)
|
|
||||||
for gain in ('kp', 'ki', 'kd', 'kt'):
|
|
||||||
require(config, 'TempCtrl.Outer.{}'.format(gain), (int, float))
|
|
||||||
for branch in ('Heat', 'Hold', 'Cool'):
|
|
||||||
require(config, 'TempCtrl.Inner.{}.{}'.format(branch, gain), (int, float))
|
|
||||||
|
|||||||
+14
-2
@@ -1,6 +1,18 @@
|
|||||||
from utils.config_validate import ConfigError, require
|
from utils.config_validate import ConfigError, require, require_schema
|
||||||
from components.sud import Sud
|
from components.sud import Sud
|
||||||
|
|
||||||
|
# The only part of a sud.json document's shape that's fixed regardless of
|
||||||
|
# content - same declarative mechanism validate_temp_ctrl() uses for
|
||||||
|
# config.json's TempCtrl section (see config_validate.py's require_schema()).
|
||||||
|
# Everything else about a schedule (how many steps, which fields each one
|
||||||
|
# overrides vs. inherits from default.step, whether a step needs ramp.rate
|
||||||
|
# at all) depends on the document's own content, not just its shape, so it
|
||||||
|
# can't be expressed as a static schema - handled by the step loop below
|
||||||
|
# instead, still built on the same require()/ConfigError primitives.
|
||||||
|
SUD_SCHEMA = {
|
||||||
|
'steps': list,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def validate_sud(data):
|
def validate_sud(data):
|
||||||
"""Fail-fast structural check for a sud.json document - same rationale
|
"""Fail-fast structural check for a sud.json document - same rationale
|
||||||
@@ -15,7 +27,7 @@ def validate_sud(data):
|
|||||||
Reuses Sud._parse_data() itself (rather than re-implementing default-
|
Reuses Sud._parse_data() itself (rather than re-implementing default-
|
||||||
merging/step-building) so what's checked here can never drift from what
|
merging/step-building) so what's checked here can never drift from what
|
||||||
actually gets parsed at runtime."""
|
actually gets parsed at runtime."""
|
||||||
require(data, 'steps', type=list)
|
require_schema(data, SUD_SCHEMA)
|
||||||
if not data['steps']:
|
if not data['steps']:
|
||||||
raise ConfigError("'steps' must be a non-empty list")
|
raise ConfigError("'steps' must be a non-empty list")
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user