Canonicalize registry keys to uppercase platform names - #295
Merged
Conversation
The registry keyed entries by Platform | str, so listing semantics
were accidental and, because Platform is a str-Enum whose members
hash as their auto() counter values, register_driver("3", X)
silently overwrote the built-in CISCO_IOS entry and "3" resolved as
a platform lookup.
Key everything on canonical uppercase names: Platform members
convert via .name at the boundary, making a member and its name
interchangeable, and get_registered_platforms() converts enum-known
names back to members while custom names stay uppercase strings.
Error messages now echo the caller's spelling.
Closes #284
Reuse the module-level _CustomDriver instead of a copy-pasted nested IOS subclass, name the value-string constant once, and keep cleanup in finally with the assertion outside the try. The not-overridden error now formats the canonical name because pre-3.11 f-strings render a str-Enum member as its meaningless value string.
5 tasks
jtdub
added a commit
that referenced
this pull request
Aug 5, 2026
* Align agent instruction files with repo reality AGENTS.md claimed model fields must be immutable collections, but HConfigDriverRules deliberately uses list fields so built-in rules and callbacks can be removed by identity (#286); an agent following the rule literally would break that API. Document the carve-out everywhere the rule is stated (AGENTS.md, copilot-instructions, code-style.md, review skill, new-driver skill). The review skill diffed against master, which on any next-based branch pulls in all of v4 and makes the report meaningless; switch to merge-base against the actual base branch. Add the branching strategy to AGENTS.md (previously only CLAUDE.md had it, so non-Claude agents and Copilot would target master for v4 work) and drop the now-duplicate section from CLAUDE.md. Also surface facts agents had no way to learn: the CI Python matrix (3.10-3.14), the unconditional docs --strict job and its separate docs/requirements.txt, the formats layer (JSON/XML/NETCONF/gNMI), future_with_report(), registry key canonicalization (#284/#295), and per-driver unit test expectations. Fix CONTRIBUTING.md stale content (nonexistent test file, missing yamllint/flynt, missing changelog requirement, no branch-base guidance). * Fix broken doc examples and stale user-facing docs Several documented examples did not run or showed wrong output: getting-started referenced a fixture that does not exist, tags.md filtered on an 'ntp' tag its fixture never defines (actual output was empty), remediation-workflows called a nonexistent delete_child() method, config-views mixed switchport and ip address config and claimed outputs the view never produces, and the hierarchical JunOS example was missing six trailing set lines. All replacement outputs were verified by running the snippets against the real fixtures. Remove the hardcoded prerelease pin from install.md (it had already drifted from pyproject) and add --pre to the README install so its Quick Start, which uses the v4-only API, can actually run. Propagate Aruba AOS-CX into the architecture driver table and config-views lists, correct the HConfigViewBase abstract-member lists (dot1q_mode_from_vlans is a concrete static helper), and align the interface-view example with the in-tree base-class pattern. Fill reference gaps: formats module, future_with_report/FutureReport, resolve_driver, view data models, and the full built-in post-load callback table in api-reference/rule-reference; glossary entries for the registry, future reports, list_keys, callbacks, and RemediationPlugin; rules-from-files loader constraints (one criterion per lineage entry, Platform-only). Point the legacy utilities.md redirects at admin/rules-from-files.md where that content actually lives. * Restructure CHANGELOG Unreleased section per Keep a Changelog The Unreleased block had accumulated duplicate Added/Changed/Fixed headings from successive merges, Added-type entries filed under Fixed, a non-standard 'v4 design decisions' heading, and a stale claim that JSON/XML ingestion was post-4.0 roadmap work when the same section documents it as shipped. Merge to one heading per category, re-file the misfiled entries, drop the contradictory sentence, and keep the design decisions as an intro note. All 59 entries and every (#NNN) reference are preserved. Point the migration guide at the Unreleased section instead of a 4.0.0 CHANGELOG section that does not exist yet. Correct the per-file ignore path for the benchmarks file, which moved to tests/benchmarks/ without the lint config following; with the ignore active again, the inline print suppressions it replaces became unused and were removed. * Fix stale WorkflowRemediation docstring; document public members The WorkflowRemediation class docstring, which renders into the public API reference, still showed a v3-era example importing the removed get_hconfig from the nonexistent hier_config.model module, and claimed __init__ raises ValueError when it raises IncompatibleDriverError. Add attribute docs to the FutureReport fields (the user-facing contract of future_with_report) and docstrings to 33 previously undocumented public members on autodoc'd classes — driver extension points (idempotent_for, sectional_exit, prefixes, config_preprocessor), the HConfigViewBase contract, and the HConfigBase/HConfigChild/HConfig/ HConfigChildren members that doc examples tell users to call. Also add this PR's changelog entry (#297).
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.
Summary
Implements #284. Targets
nextfor v4.0.0 — an API-shape decision that is free to make now and breaking to make later.The driver registry keyed entries by
Platform | str, which leaked into the public API two ways:get_registered_platforms()returned an accidentally-mixed tuple, and custom names were silently uppercased. Exploration also surfaced a verified latent bug:Platformis astr-Enum withauto()values, so members hash/compare as stringified counters (Platform.CISCO_IOS == "3") —register_driver("3", X)silently overwrote the built-in CISCO_IOS entry andget_hconfig_driver("3")resolved to the IOS driver._BUILTIN_DRIVERS/_registryare nowdict[str, type[HConfigDriverBase]]keyed on canonical uppercase names;_normalize()convertsPlatformmembers via.nameand uppercases strings — a member and its name are fully interchangeable inregister_driver/unregister_driver/get_hconfig_driver.get_registered_platforms()keeps itstuple[Platform | str, ...]signature with now-deterministic, documented semantics:Platformmembers for enum-known names (via exact-namePlatform.__members__lookup — never value lookup), canonical uppercase strings for custom names."3"no longer collide with or resolve as platforms — they raiseDriverNotFoundError.DriverNotFoundErrormessages now echo the caller's spelling rather than the normalized form.No public signatures changed. Out of scope per the issue:
register_drivervalidation, thread-safety, exportingresolve_driver.Testing
TDD: five new tests in
tests/unit/test_registry.py; the collision test and the value-lookup test were observed failing on the pre-change code (custom driver returned forPlatform.CISCO_IOS;"3"resolving instead of raising). The other three pin the listing semantics (custom names appear as uppercase strings, builtins as enum members) and member/name interchangeability with builtin restore. All existing registry tests pass unmodified.poetry run ./scripts/build.py lint-and-testexits 0 (762 passed, coverage ≥95%)poetry run mkdocs build --strictpassesDocs & changelog
docs/dev/architecture.md— registry section describes canonical uppercase-name keying and the listing semanticsdocs/admin/custom-drivers.md— member/name interchangeability and listing-return noteshier_config/registry.pydocstrings (rendered intodocs/dev/api-reference.mdvia mkdocstrings) document the canonicalizationCHANGELOG.md— Changed (key canonicalization) + Fixed (value-string collision) entries (v4: Canonicalize registry platform keys before 4.0.0 final #284)Closes #284