feat!: role bases declare their methods instead of stubbing them
A role base used to fill its methods with `raise NotImplementedError`. That is
not neutral under multiple inheritance: the placeholder wins the MRO against a
sibling base's working implementation and silently replaces it. Adding one stub
to a base was therefore a breaking change for every driver mixing that base with
another, and it broke three of them — OpenWrt grew seven forwarding methods,
QNAP one, and OpenMediaVault avoided inheriting StorageDriver at all.
Role bases now declare their surface under `if TYPE_CHECKING` and implement
nothing. There is no longer anything to shadow, so a device can finally say what
it is:
class QnapQtsDriver(StorageDriver, HypervisorDriver, LinuxDriver):
The order of those bases is the ranking, read back by roles_of(),
role_keys_of() and primary_role_of() in the new roles module. Nothing restates
it: no precedence table, no attribute to override.
Two consequences, both wanted. `hasattr` is a truthful capability probe again,
because a method exists exactly when a driver provided it. And a method that was
never implemented now raises AttributeError rather than NotImplementedError, so
callers should ask before calling.
Shared behaviour moves out of the roles and into function classes, each holding
it once: PackageManagementMixin (was five byte-identical copies),
HealthMetricsMixin (five), ServiceControlMixin, UpdateMixin, NatVpnMixin,
MacAclMixin, FirewallRuleMixin, InterfaceFilterMixin.
BREAKING CHANGE: methods whose contract genuinely differed were renamed apart —
StorageDriver.get_services -> get_storage_services, the storage and hypervisor
snapshot writers -> create/delete/rollback_{volume,vm}_snapshot,
HypervisorDriver.get_storage -> get_vm_storage_pools, get_snapshots ->
get_vm_snapshots, SwitchDriver.get_dot1x_config -> get_dot1x_ports. Two
duplicate names collapsed onto the one already in use: get_pending_updates ->
get_available_updates and remove_package -> uninstall_package.
Also fixes __doc__ being None on all seven role bases: TYPE_LABEL was assigned
above the triple-quoted string, which made it a bare expression rather than a
docstring.
This commit is contained in:
@@ -22,6 +22,66 @@ NAPALM's `NetworkDriver` defines a common interface for all network devices. In
|
||||
|
||||
`napalm-device-types` sits in between: it adds one well-typed layer of abstract methods per device category, so every driver for the same category exposes the same interface.
|
||||
|
||||
## Roles: what a device *is*
|
||||
|
||||
A device is often several things at once. A QNAP NAS runs VMs on a Linux userland; an
|
||||
OpenMediaVault box is a NAS built on Debian. So a driver inherits **one role base per
|
||||
role its device fills**, and **the order it lists them in is the ranking**:
|
||||
|
||||
```python
|
||||
class QnapQtsDriver(StorageDriver, HypervisorDriver, LinuxDriver):
|
||||
... # primary_role_of(...) == "storage"
|
||||
```
|
||||
|
||||
`roles_of(cls)`, `role_keys_of(cls)` and `primary_role_of(cls)` read that back. Nothing
|
||||
restates the ranking: there is no precedence table and no attribute to override.
|
||||
|
||||
### Role bases declare; they never implement
|
||||
|
||||
**A role base must not contain a single runtime method — not even a
|
||||
`NotImplementedError` placeholder.** Its methods are declared under `if TYPE_CHECKING`:
|
||||
|
||||
```python
|
||||
class StorageDriver(DeviceTypeDriver):
|
||||
"""Contract, not code."""
|
||||
ROLE: str = "storage"
|
||||
TYPE_LABEL: str = "Storage"
|
||||
|
||||
if TYPE_CHECKING: # nothing exists at runtime
|
||||
def get_disks(self) -> List[PhysicalDiskDict]: ...
|
||||
```
|
||||
|
||||
This is not a style preference. A placeholder on a base class is not neutral under
|
||||
multiple inheritance: it wins the MRO against a sibling base's *working* implementation
|
||||
and silently replaces it. Adding one stub to a base is therefore a breaking change for
|
||||
every driver that mixes that base with another. It happened three times here before the
|
||||
rule existed, and each time the fix was hand-written forwarding methods in the driver.
|
||||
|
||||
Two things follow, and both are improvements:
|
||||
|
||||
- **`hasattr` is truthful again.** A method exists on a driver class exactly when that
|
||||
driver implemented it, which is how netOrk asks "can this driver list disks".
|
||||
- **A method that was never implemented raises `AttributeError`, not
|
||||
`NotImplementedError`.** Ask before calling.
|
||||
|
||||
## Function classes: what a device *can do*
|
||||
|
||||
Behaviour shared across roles lives in a function class, exactly once, and a role base
|
||||
is a thin bundle over them — `PackageManagementMixin`, `HealthMetricsMixin`,
|
||||
`ServiceControlMixin`, `UpdateMixin`, `NatVpnMixin`, `MacAclMixin`, `FirewallRuleMixin`,
|
||||
`DhcpServerMixin`, `PingSweepMixin`, `ConfigLifecycleMixin`, `InterfaceFilterMixin`.
|
||||
|
||||
A function class may use the **template form** — public method concrete, the
|
||||
device-specific part a `_hook` declared under `if TYPE_CHECKING` — *when the base
|
||||
genuinely does work* on the result: normalising, sorting, validating, or orchestrating
|
||||
several hooks. `ConfigLifecycleMixin.compare_config` over `_get_running_config` is the
|
||||
model. Where the base would only pass the call through, declare the method directly;
|
||||
two names for one pass-through is ceremony, not design.
|
||||
|
||||
NAPALM's own getters (`get_facts`, `get_interfaces`, `ping`, `get_config`) are never
|
||||
wrapped in a template — they belong to NAPALM, and code outside this repo relies on
|
||||
their contract.
|
||||
|
||||
## Design principle: generic vs. device-specific logic
|
||||
|
||||
When adding behavior to a device-type base class, split it along one line: **would
|
||||
|
||||
Reference in New Issue
Block a user