Skip to content

RegistryWarning: three Extractor registrars still warn on a same-object re-registration #739

Description

@JarryShaw

#718 converted ten registrar sites from a presence-only collision guard to an identity guard, so re-registering the same object no longer warns. Three more sit outside its criterion and still warn falsely.

site guard on origin/main
pcapkit/foundation/extraction.py:412 register_engine if name in cls.__engine__: → warn(f'engine {name} already registered, overwriting')
pcapkit/foundation/extraction.py:436 register_reassembly if protocol in cls.__reassembly__:
pcapkit/foundation/extraction.py:460 register_traceflow if protocol in cls.__traceflow__:

Each stores a class or ModuleDescriptor, and each is the funnel for an auto-registering __init_subclass__. So the ordinary sequence — declare a subclass, which registers it, then call the registrar for the same class — warns about a displacement that did not happen. Identical false positive to #718's, identical fix shape:

incumbent = cls.__engine__.get(name)
if incumbent is not None and incumbent is not engine:
    warn(...)

Why #718 missed them: its scope was registrars whose message renders {incumbent!r}. These three name only the key, so they fall outside that wording while sharing the defect. Worth deciding whether #718's criterion should have been "presence-only guard on a stored object" rather than the message shape — that phrasing would have caught all thirteen.

One caveat before anyone fixes it: check whether a ModuleDescriptor incumbent can meet a class replacement here. In the protocol-layer registries every seeded entry is a descriptor and zero are classes, so the first real registration legitimately warns and only a repeat is silent. If the same holds here, the identity guard changes less than it appears to — still correct, but the win is narrower than #718's.

Found while cross-reviewing #726. Not a blocker for it; #726 is scoped to the ten sites and discharges #718 fully.

Activity

  1. added
    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)
    fixPull requests that fix a defect (fix: subject prefix)
    on Sep 24, 2026
  2. JarryShaw commented on Sep 24, 2026

    @JarryShaw
    OwnerAuthor

    Correcting my own scope: it is five sites, not three. I missed both register_dumper guards. Verified on origin/main at 9b2d927c2:

    site guard registry
    foundation/extraction.py:386 register_dumper if format in cls.__output__: missed originally
    foundation/extraction.py:413 register_engine if name in cls.__engine__:
    foundation/extraction.py:437 register_reassembly if protocol in cls.__reassembly__:
    foundation/extraction.py:461 register_traceflow if protocol in cls.__traceflow__:
    foundation/traceflow/traceflow.py:223 register_dumper if format in cls.__output__: missed originally

    All five store a ModuleDescriptor | Type[...], which is what puts them in this class. My original line numbers were also off by one (:412/:436/:460 → :413/:437/:461).

    For context, found while re-reviewing #726: 42 already registered guards across 21 files package-wide. Most are keyed on an enum code and store a parser/constructor pair, which is a different shape; these five are the class-valued ones that share #718's exact defect.

    Which matters beyond tidiness: #726 tried to write "presence-only guards are no longer the case anywhere in the package" into register_protocol's docstring, and that clause is false precisely because of these. Fixing them makes that statement true.

  3. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 24, 2026
  4. added 2 commits that reference this issue on Sep 24, 2026
    f0a1f5b
    0071801
  5. added a commit that references this issue on Sep 24, 2026
    0a3abff
  6. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 24, 2026
  7. added this to the 1.5 milestone on Oct 6, 2026
  8. moved this to Done in PyPCAPKiton Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)fixPull requests that fix a defect (fix: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions