Skip to content

RegistryWarning: nine sibling registrars share #710's repr collision, and guard on presence rather than difference #718

Description

@JarryShaw

#711 fixes the "overwriting X with X" message for register_protocol (#710). The same defect exists in nine sibling registrars, and in them it is worse.

The nine sites

All on main at 3904c025a, all rendering {incumbent!r} with {protocol!r}:

File Line Keyed by
pcapkit/protocols/protocol.py 805 code
pcapkit/protocols/internet/internet.py 166 code
pcapkit/protocols/link/link.py 146 code
pcapkit/protocols/transport/transport.py 117 port
pcapkit/protocols/transport/sctp.py 630 payload protocol identifier
pcapkit/protocols/misc/pcap/frame.py 153 code
pcapkit/protocols/misc/pcapng.py 887 code
pcapkit/protocols/schema/schema.py 1127 _code
pcapkit/protocols/schema/schema.py 1180 code

These reprs were added by #695. Before it, these sites interpolated only the key and could not exhibit #710.

Why it is worse here than in register_protocol

The guards differ. register_protocol tests difference:

incumbent = protocol_registry.get(name)
if incumbent is not None and incumbent is not protocol:

All nine siblings test presence:

if code in cls.__proto__:
    warn(f'protocol {code} already registered, overwriting '
         f'{cls.__proto__[code]!r} with {protocol!r}', RegistryWarning)

So a sibling emits "overwriting X with X" in two distinct situations that the message cannot tell apart:

  1. Two genuinely different classes sharing a repr — the RegistryWarning claims a protocol was overwritten with itself when two distinct classes share a qualname #710 shape, where the warning is correct but unreadable.
  2. Re-registering the same object, which is a no-op. Here the warning is not merely unreadable, it is misleading: nothing was displaced.

register_protocol's identity guard (added in #681) already suppresses case 2. The siblings have no equivalent.

Suggested fix

Two independent changes, in this order:

  1. Add the identity guard to all nine, so a same-object re-registration stays silent. This is the behavioural half and needs its own test per site.
  2. Apply fix(registry): disambiguate register_protocol's overwrite warning when two classes share a repr #711's disambiguation to the message, so case 1 stays readable once case 2 is gone.

Splitting them matters: (1) alone removes most of the confusing output, and (2) alone leaves the misleading no-op warning in place.

Provenance

Found by the cross-review of #711 at #711 (comment), which correctly identified nine siblings but cited them under pcapkit/foundation/registry/ paths that do not exist, with every line number off by one. The table above is re-derived from origin/main directly.

Activity

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

    bugwipWork in flight - a covering PR is open or an agent is actively on it

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions