Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 45 additions & 40 deletions pcapkit/protocols/internet/mh.py
Original file line number Diff line number Diff line change
Expand Up @@ -581,25 +581,28 @@ class FastBindingAcknowledgmentStatus(EnumLookup, IntEnum):
This class carried its own hand-rolled ``get()`` override through
#930, and briefly again through GitHub issue #935's first attempt,
which widened the override to accept ``default`` rather than delete
it outright. A ruling given in review of the work for #935 went the
other way, verbatim -- asked *"why must we have the two overrides
tho? cant they directly fall back to the base class's?"*, the answer
was *"I prefer (2) directly"*, ``(2)`` naming deletion among the
ruling's own options. Measured before acting on it: the override's own
docstring called it a "Backport support for original codes", but
this class mints no alias -- ``__members__`` and ``list(cls)``
agree at 6 -- so what the override actually did was resolve an
:class:`int` by direct construction and a name by subscript,
exactly the dual resolution
it outright. An earlier lean on issue #935 had preferred that
widening; a later ruling in review of the attempt went the other
way: delete both ``get`` overrides in this module rather than widen
them, so :class:`FastBindingAcknowledgmentStatus` and
:class:`IPv6AddressPrefixCode` inherit
:meth:`~pcapkit.corekit.enum.EnumLookup.get` outright. That also
removes the ``# type: ignore[override]`` suppressions the overrides
needed, and makes all seven re-parented classes behave alike on
``get``, which had not been literally true. Measured before acting
on it: the override's own docstring called it a "Backport support
for original codes", but this class mints no alias --
``__members__`` and ``list(cls)`` agree at 6 -- so what the override
actually did was resolve an :class:`int` by direct construction and
a name by subscript, exactly the dual resolution
:meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for
every other :class:`int`-valued registry in this tree. There was
nothing left to backport. ``get``/``get_all`` now come from the
base alone, the same as the five other re-parents #930 finished
alongside this one -- including :class:`LocalizedRoutingStatus`
and :class:`LMAAddressCode` below, whose own hand-rolled ``get()``
GitHub issue #880 had already deleted outright, for the same
reason: zero callers depended on anything the base does not
already do.
nothing left to backport. ``get``/``get_all`` now come from the base
alone, the same as the five other re-parents #930 finished alongside
this one -- including :class:`LocalizedRoutingStatus` and
:class:`LMAAddressCode` below, whose own hand-rolled ``get()``
GitHub issue #880 had already deleted outright, for the same reason:
zero callers depended on anything the base does not already do.

A behaviour change comes with the deletion, deliberately: the
override branched on ``isinstance(key, int)`` and routed every
Expand Down Expand Up @@ -698,26 +701,30 @@ class IPv6AddressPrefixCode(EnumLookup, IntEnum):
This class carried its own hand-rolled ``get()`` override through
#930, and briefly again through GitHub issue #935's first attempt,
which widened the override to accept ``default`` rather than delete
it outright. A ruling given in review of the work for #935 went the
other way, verbatim -- asked *"why must we have the two overrides
tho? cant they directly fall back to the base class's?"*, the answer
was *"I prefer (2) directly"*, ``(2)`` naming deletion among the
ruling's own options. Measured before acting on it: the override's own
docstring called it a "Backport support for original codes", but
this class mints no alias -- ``__members__`` and ``list(cls)``
agree at 4 -- so what the override actually did was resolve an
:class:`int` by direct construction and a name by subscript,
exactly the dual resolution
it outright. An earlier lean on issue #935 had preferred that
widening; a later ruling in review of the attempt went the other
way: delete both ``get`` overrides in this module rather than widen
them, so :class:`FastBindingAcknowledgmentStatus` and
:class:`IPv6AddressPrefixCode` inherit
:meth:`~pcapkit.corekit.enum.EnumLookup.get` outright. That also
removes the ``# type: ignore[override]`` suppressions the overrides
needed, and makes all seven re-parented classes behave alike on
``get``, which had not been literally true. Measured before acting
on it: the override's own docstring called it a "Backport support
for original codes", but this class mints no alias --
``__members__`` and ``list(cls)`` agree at 4 -- so what the override
actually did was resolve an :class:`int` by direct construction and
a name by subscript, exactly the dual resolution
:meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for
every other :class:`int`-valued registry in this tree. There was
nothing left to backport. ``get``/``get_all`` now come from the
base alone, the same as the five other re-parents #930 finished
alongside this one -- including
nothing left to backport. ``get``/``get_all`` now come from the base
alone, the same as the five other re-parents #930 finished alongside
this one -- including
:class:`~pcapkit.protocols.internet.mh.LocalizedRoutingStatus` and
:class:`~pcapkit.protocols.internet.mh.LMAAddressCode` below, whose
own hand-rolled ``get()`` GitHub issue #880 had already deleted
outright, for the same reason: zero callers depended on anything
the base does not already do.
outright, for the same reason: zero callers depended on anything the
base does not already do.

A behaviour change comes with the deletion, deliberately: the
override branched on ``isinstance(key, int)`` and routed every
Expand Down Expand Up @@ -839,10 +846,9 @@ class LocalizedRoutingStatus(EnumLookup, IntEnum):
-- tests included -- so GitHub issue #880 deleted it outright rather
than rebuilding it on the immutable contract, the same conclusion
#935 reached separately for the other two, on a ruling given in
review of that work, verbatim: *"I prefer (2) directly"* -- ``(2)``
being deletion of those two overrides rather than widening them to
match the base.
GitHub issue #930's re-parenting above gives this class
review of that work: delete those two overrides rather than widen
them to match the base, which an earlier lean on the issue had
preferred. GitHub issue #930's re-parenting above gives this class
``get``/``get_all`` again, but as the base's own bare lookup rather
than a bespoke override -- it still cannot mint, so an unassigned
value raises through ``get`` exactly as it does through the bare
Expand Down Expand Up @@ -919,10 +925,9 @@ class LMAAddressCode(EnumLookup, IntEnum):
-- tests included -- so GitHub issue #880 deleted it outright rather
than rebuilding it on the immutable contract, the same conclusion
#935 reached separately for the other two, on a ruling given in
review of that work, verbatim: *"I prefer (2) directly"* -- ``(2)``
being deletion of those two overrides rather than widening them to
match the base.
GitHub issue #930's re-parenting above gives this class
review of that work: delete those two overrides rather than widen
them to match the base, which an earlier lean on the issue had
preferred. GitHub issue #930's re-parenting above gives this class
``get``/``get_all`` again, but as the base's own bare lookup rather
than a bespoke override -- it still cannot mint, so an unassigned
value raises through ``get`` exactly as it does through the bare
Expand Down
8 changes: 4 additions & 4 deletions pcapkit/utilities/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -716,10 +716,10 @@ class EnumKeyError(BaseError, KeyError):
taste: ``E['nosuch']`` raises :exc:`KeyError` and ``E(999)`` raises
:exc:`ValueError`, so a lookup that misses by *name* is
:exc:`KeyError`-derived and one that misses by *value* is
:exc:`ValueError`-derived. A ruling recorded on GitHub issue #923,
verbatim: *"Either ``ValueError`` or ``KeyError``, that's depending on how
stdlib's ``Enum`` would raise on these circumstances. And we should raise
one from ``pcapkit.utilities.exceptions`` rather builtin exceptions."*
:exc:`ValueError`-derived. That is a ruling given in review of #877's
phase-2 re-parenting, carried out by GitHub issue #923: raise
whichever of the two stdlib :class:`~enum.Enum` would raise in the same
circumstances, and raise it from this module rather than as a builtin.

Deriving from :exc:`KeyError` is what makes that ruling cheap to carry out:
:meth:`~pcapkit.corekit.enum.EnumLookup.get` raised a bare builtin
Expand Down
36 changes: 18 additions & 18 deletions pcapkit/vendor/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,13 @@ def get_parser() -> 'ArgumentParser':
def _snapshot_and_restore(vendor: 'Type[Vendor]') -> 'Iterator[None]':
"""Copy a target's const file aside before it runs; restore it if it raises.

A ruling given in review of the work for #872, verbatim: *"an easier
path is simply keep a copy before running the sub-vendor and revert if
anything failed."* This is that -- at the per-target boundary
:func:`run` already owns, which is also exactly where the ruling's
``(b)``, "only discard changes made by a non-zero sub-vendor", wants
the discarding to happen.
A ruling given in review of the work for #872 settled how a failed target
is undone: keep a copy of its const file before running the sub-vendor and
revert if anything failed, rather than making the write itself atomic.
This is that -- at the per-target boundary :func:`run` already owns, which
is also exactly where the earlier ruling on the same work wants the
discarding to happen: only a non-zero sub-vendor's changes are discarded,
and a zero-exited one's are kept.

It is a *wider* guarantee than protecting the single
``open``/``print`` pair :meth:`~pcapkit.vendor.default.Vendor.__init__`
Expand Down Expand Up @@ -101,18 +102,17 @@ def _snapshot_and_restore(vendor: 'Type[Vendor]') -> 'Iterator[None]':

A symlinked destination has a related divergence from the pre-#872
behaviour -- documented against the write path by rounds 4-8 (deleted
along with :meth:`~pcapkit.vendor.default.Vendor._write_atomic`, though
the underlying behaviour persists here instead): ``open(const_file,
'w')`` writes *through* a symlink, into whatever file it points at,
leaving the link itself untouched. ``os.replace(backup, const_file)``
on restore instead replaces the *link itself* with a regular file.
Measured, for ``link.py`` symlinked to ``real.py``: after a failure and
restore, ``real.py`` is left however the crawler's failed write left
it (truncated, in this case -- nothing here restores the file a
symlink used to point at), ``link.py`` is now a regular file holding
the backup's content, and ``os.path.islink(link.py)`` is
:data:`False`. ``find pcapkit/const -type l`` is still empty, so this
remains latent.
along with ``_write_atomic``, though the underlying behaviour persists
here instead): ``open(const_file, 'w')`` writes *through* a symlink,
into whatever file it points at, leaving the link itself untouched.
``os.replace(backup, const_file)`` on restore instead replaces the *link
itself* with a regular file. Measured, for ``link.py`` symlinked to
``real.py``: after a failure and restore, ``real.py`` is left however
the crawler's failed write left it (truncated, in this case -- nothing
here restores the file a symlink used to point at), ``link.py`` is now a
regular file holding the backup's content, and
``os.path.islink(link.py)`` is :data:`False`. ``find pcapkit/const
-type l`` is still empty, so this remains latent.

Nothing is copied at all when the destination does not exist yet: there
is no previous file for a failure to discard, so there is nothing this
Expand Down
Loading