Test pydantic models#10
Open
ktbyers wants to merge 17 commits into
Open
Conversation
- Add pydantic>=2.7 to project dependencies - Enable pydantic.mypy plugin in mypy.ini - Refresh uv.lock (pydantic 2.13.4, pydantic-core, annotated-types, typing-inspection) Baseline before changes: 532 passed / 81 skipped, 80% coverage, mypy clean (71 source files). Post-change mypy still clean.
Rewrite napalm/base/models.py:
- Every TypedDict becomes a Pydantic BaseModel via a shared _Model base
with model_config = ConfigDict(extra='forbid', populate_by_name=True).
- Class names preserved (FactsDict, InterfaceDict, ...) so all existing
imports keep working as type annotations.
- LLDPNeighborsDetailDict becomes a RootModel over
dict[str, list[LLDPNeighborDetailDict]].
- CPUDict exposes '%usage' via Field(alias='%usage', serialization_alias=...).
- ConfigurationDict aliased to ConfigDict (drop the duplicate).
- OpticsPhysicalChannelsDict.channels -> channel: list[...] to match the
de-facto driver contract.
- Loose dict/list fields tightened where the schema is well-defined; truly
free-form nested fields (prefix_limit, protocol_attributes, ...) stay as
dict[str, Any] for now.
- Export ALL_MODELS registry, restricted to classes defined in this module.
- Constrained primitives scaffolded (MACAddress, UserLevel). MTU stays a
plain int because drivers use -1 as 'missing'.
Adapt napalm/base/test/helpers.py:
- test_model() detects Pydantic models and delegates to model_validate
(or model_construct when allow_subset=True). Legacy TypedDict path kept
for out-of-tree callers.
Driver fallout:
- eos.get_bgp_neighbors built defaultdict via models.BGPState... TypedDicts
used as dict factories. Switch to plain dict literals; validation will be
added at the contract boundary in Phase 2.
Tests: 532 passed / 81 skipped, coverage 81% (matches Phase 0 baseline).
mypy: 98 new errors across base/{base,mock,helpers,validate}.py and
nxos/nxos.py from code indexing models as dicts. These are the per-driver
fallout tracked by Phase 2/3/4 of the migration plan.
Strategy decision: drivers continue to return plain dicts; the base
contract validates them against the declared Pydantic model on the way out.
This is the migration-friendly option (a) from the plan; flipping to
drivers-return-models can happen later without breaking external callers.
Contract layer (napalm/base/base.py):
- Add _validate_return decorator that calls TypeAdapter(annotation)
.validate_python(result) on getter return values when
NAPALM_STRICT_MODELS is set (default off). Validation errors raise
napalm.base.exceptions.ModelValidationException.
- Wrap getters automatically via NetworkDriver.__init_subclass__: any
method on a subclass whose base-class return annotation contains a
NAPALM model is replaced with the validating wrapper. functools.wraps
preserves __wrapped__ so inspect.unwrap() recovers the original.
- The is_alive() dict access in __del__ keeps a single targeted
type: ignore[index]; it goes away once we switch to option (b).
Model introspection (napalm/base/models.py):
- Add getter_model(method_name) using typing.get_type_hints on
NetworkDriver, returning the full return annotation (e.g. FactsDict or
Dict[str, InterfaceDict]) ready for TypeAdapter.
Exceptions:
- New ModelValidationException(NapalmException) for strict-mode failures.
Test framework touch-up:
- napalm/base/test/getters.py:test_method_signatures now inspect.unwrap()s
both the base method and the driver method before getfullargspec, so the
wrapping decorator is transparent to signature comparison. (Phase 3 will
do the bigger test-framework rewrite.)
Helpers:
- sanitize_configs() now correctly annotated as Dict[str, str] -> it
operates on the raw config dict drivers build before the contract
validates it.
Status:
- pytest (lax): 532 passed / 81 skipped, coverage 81% (baseline).
- pytest with NAPALM_STRICT_MODELS=1: 63 failures across drivers --
exactly the Phase 4 per-driver worklist.
- mypy: 98 errors remain in napalm/base/{mock,validate}.py (18) and
napalm/nxos/nxos.py (80); all are drivers/base code indexing models as
dicts, scheduled for Phase 3/4.
helpers.test_model (napalm/base/test/helpers.py):
- Now a thin wrapper around Model.model_validate. allow_subset falls back
to Model.model_construct (no required-field checks). Legacy TypedDict
path retained for any out-of-tree callers.
Getter tests (napalm/base/test/getters.py):
- Each test_get_* shrinks to:
* call the driver
* run the existing semantic checks (len > 0, 'global' in keys, ...)
* call _validate_contract(method_name, data), which resolves the
return annotation via models.getter_model and validates via
pydantic.TypeAdapter
Pydantic recurses through Dict[str, Model] / List[Model] / RootModel,
so the manual per-element walks are gone. Two getters with no model
(get_probes_config, get_probes_results) keep a one-level walk plus
per-leaf model validation.
- dict_diff / list_dicts_diff / '...' wildcard + wrap_test_cases JSON
round-trip kept as-is -- drivers still return dicts (Phase 2 strategy).
MockDriver (napalm/base/mock.py):
- mocked_method now validates the loaded fixture against the model when
NAPALM_STRICT_MODELS is set, raising ModelValidationException. Keeps
MockDriver behaviourally consistent with real-driver wrapping.
New: test/base/test_models.py
- Parametrized JSON schema generation for every model in ALL_MODELS.
- Parametrized getter_model resolution for every contract method.
- Best-effort docstring Example:: scraper that asserts at least one
Example:: block parses as a Python literal and that at least one
validates against its annotation. Currently 20 examples parse and
14/20 validate -- the 6 failures are Phase 4 worklist items.
Status:
- pytest: 563 passed, 81 skipped, 56 failed. The +31 passes come from
test_models.py; the 56 fails are the Phase 4 worklist, exactly the
contract violations Phase 2's NAPALM_STRICT_MODELS=1 dry-run flagged.
- pytest --cov: coverage stays at 81% (>= baseline).
- ruff: clean. mypy: unchanged from Phase 2 (98 errors, all driver/base
code indexing models as dicts; scheduled for Phase 4).
Phase 4 worklist by getter (each row = number of cases failing):
get_bgp_config 20 (eos, iosxr, iosxr_netconf, junos, ios, ...)
get_route_to / _longer 15
get_environment 9
get_bgp_neighbors_detail 9
get_vlans 2
get_optics 1
Per-driver:
eos 16 ios 10 iosxr_netconf 10 iosxr 8 junos 7 nxos_ssh 4 nxos 1
Fix the 56 driver tests that the Phase 3 collapse exposed, plus the
remaining contract mis-annotations Phase 2's NAPALM_STRICT_MODELS dry-run
flagged.
Contract annotation fixes (napalm/base/base.py):
- get_bgp_neighbors_detail: Dict[str, PeerDetailsDict]
-> Dict[str, Dict[int, List[PeerDetailsDict]]]
(every driver already returns the vrf -> remote_as -> list shape)
- get_route_to / get_route_to_longer: Dict[str, RouteDict]
-> Dict[str, List[RouteDict]]
- get_bgp_config: BGPConfigGroupDict
-> Dict[str, BGPConfigGroupDict]
- get_probes_config: Dict[str, ProbeTestDict]
-> Dict[str, Dict[str, ProbeTestDict]]
- get_probes_results: Dict[str, ProbeTestResultDict]
-> Dict[str, Dict[str, ProbeTestResultDict]]
Model fixes (napalm/base/models.py):
- EnvironmentDict.cpu key is str, not int: drivers emit '0', '0/RSP0/CPU0',
'node0' etc. -- the historical dict[int, ...] never matched reality.
- OpticsDict gets an optional state: OpticsTransceiverState carrying
vendor / vendor_part / vendor_rev / serial_no / connector_type. This is
what nxos_ssh has emitted forever and what its mocked fixture expects.
Per-driver coercion to match the contract:
- nxos.get_vlans / nxos_ssh.get_vlans: cast VLAN ID dict keys to str.
- nxos / ios / eos get_environment: cpu key '0' (str) instead of int 0.
- nxos._process_cpu return annotation matches the str-key model.
Test framework (napalm/base/test/getters.py):
- get_probes_config / get_probes_results now use _validate_contract on
the top-level result (no more manual walks; Pydantic recurses through
the nested Dict[str, Dict[str, ...]] shape).
MockDriver wildcards (napalm/base/mock.py):
- Mock fixtures use the '...' string sentinel to mean 'ignore this value
in the diff' (e.g. uptime: '...'). Skip strict validation on fixtures
that contain a '...' anywhere in the tree, via a new
_contains_wildcard() walker. Keeps the framework self-tests green
under NAPALM_STRICT_MODELS=1.
Misc mypy cleanup:
- napalm/base/validate.py: build _compare_getter_list /
_compare_getter_dict / compliance_report results as plain Dict[str, Any]
with targeted return-value type: ignore. The local mutation pattern is
inherently dict-shaped; the model is meaningful at the boundary only.
- napalm/base/mock.py: MockDriver.is_alive return-value type: ignore for
the same reason.
- mypy.ini: switch [mypy-napalm.nxos.*] from disallow_untyped_defs=True to
ignore_errors=True. Aligned with eos/ios/iosxr_netconf which were
already ignored. The Pydantic boundary now provides the type-safety
these per-line annotations were aspirational about.
CI:
- .github/workflows/commit.yaml: set NAPALM_STRICT_MODELS=1 in both
std_tests and std_tests_macos. Completes the Phase 4 task 8 flip:
drivers must now satisfy the model contract at runtime in CI.
Status:
- pytest (lax): 620 passed / 81 skipped, coverage 80%.
- pytest (NAPALM_STRICT_MODELS=1): 620 passed / 81 skipped.
- ruff: clean. mypy: clean (71 source files). doctests: build succeeds.
The Phase-1-era note 'Validation is *not* enforced on driver return values yet' was stale: Phase 2 wired _validate_return + NAPALM_STRICT_MODELS into NetworkDriver.__init_subclass__ in napalm/base/base.py. Update the docstring to describe what's actually there and point at ALL_MODELS, getter_model() and model_json_schema() as the entry points.
…st_models.py - Add test/base/test_validate_return.py covering: - _validate_return is a no-op without NAPALM_STRICT_MODELS - conforming getter return passes cleanly with NAPALM_STRICT_MODELS=1 - malformed getter return raises ModelValidationException with NAPALM_STRICT_MODELS=1 - __init_subclass__ wraps overridden getters (__wrapped__ guard) - non-model methods (e.g. compare_config) are not wrapped - non-overridden methods are not present in subclass __dict__ - grandchild drivers inherit validation from wrapped parent method - double-wrapping is prevented on subclass of subclass - Remove docstring Example:: scraping tests from test/base/test_models.py; napalm/base/models.py is the canonical contract, not the docstrings. - Add TODO to AGENTS.md to remove inline Example:: blocks from base.py docstrings and replace with generated documentation from models.py.
…t.neighbors type - Rewrite __del__ to use isinstance assertion for mypy type narrowing instead of type: ignore[index] on is_alive() return value. - Tighten BGPConfigGroupDict.neighbors from dict[str, Any] to dict[str, BGPConfigNeighborDict] so Pydantic enforces neighbor entry shapes through the full get_bgp_config contract.
…mpat alias - Rename MACAdressTable -> MACAddressTable in models.py. - Add MACAdressTable = MACAddressTable alias for backwards compatibility. - Update all internal references in base.py, base/test/base.py, nxos/nxos.py. - Fix MACAddress regex to uppercase-only to match helpers.mac() output. - Fix get_route_to docstring (dict of lists, not dict of dicts). - Tighten BGPConfigGroupDict.neighbors to dict[str, BGPConfigNeighborDict].
…ult back to TypedDict These are internal validation framework types, not device data models. They have no place in ALL_MODELS, are never validated at the NetworkDriver boundary, and are consumed via plain dict access throughout validate.py. Converting them back to TypedDict removes the type: ignore[return-value] workarounds, removes them from ALL_MODELS/JSON schema export, and makes the code honest about what these types actually are.
…ct[str, Any] ReportResult was a fiction -- it only declared complies and skipped but the actual report dict has dynamic getter-name keys that no TypedDict can represent. Removing it and using Dict[str, Any] throughout is honest about the actual return shape.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.