Skip to content

refactor(registry): NULL is a plain str compared by identity — use a real sentinel, defined once #833

Description

@JarryShaw

NULL is a plain string, and the guards that test it use identity, so an equal-but-distinct '(null)' takes a different branch from the sentinel itself. Measured on #815's head ee51366a6:

NULL is: '(null)'  (type str)
NULL == '(null)' -> True        NULL is '(null)' -> False

register_apptype(port, Unit, class_=NULL)       -> RegistryError: no transport protocol given …   (absent)
register_apptype(port, Unit, class_='(null)')   -> RegistryError: unknown transport protocol: '(null)'   (present)

Both raise, so nothing is broken today — but which branch runs depends on string interning, not on the caller's intent, and that is the kind of thing that turns into a real defect the moment a branch stops raising. pcapkit/foundation/registry/protocols.py:143 is NULL = '(null)' with 13 uses, and pcapkit/foundation/registry/foundation.py:48 defines the same sentinel independently with 7 more — two definitions of one concept, so a change to either silently diverges from the other.

Two further consequences visible today:

The fix is a real sentinel — a module-level singleton whose type is not str, e.g. an enum member or a bare object() subclass with a __repr__ of <NULL> — defined once and imported by both registry modules, with the is comparisons then meaning what they say. Typing becomes class_: 'str | NullType' = NULL, and the omitted case can be distinguished from any string a caller passes.

Raised at the maintainer's request, in his words: "For the NULL value in the foundation registry, can we actually use a sentinel rather than a pure string? This is not in scope of this PR tho. File an issue and we can talk further."

Blocked on #815 merging, for ordering rather than scope: #815 rewrites register_apptype's signature and its class_ is not NULL guard is one of the sites this would change, so doing it first would conflict directly. It also wants deciding alongside #832, which fixes the other end of the same leak.

Activity

  1. JarryShaw commented on Sep 26, 2026

    @JarryShaw
    OwnerAuthor

    Checkable blocker: #815 merged.

    gh pr view 815 -R JarryShaw/PyPCAPKit --json state,mergedAt
    

    #815 rewrites register_apptype's signature, and its class_ is not NULL guard at the top of the body is one of the sites this change would rewrite — so landing this first conflicts directly. It also wants deciding together with #832, which fixes the other end of the same leak ('(null)' reaching getattr and being reported as a missing attribute).

  2. added
    designA design or decision issue: a pattern being decided rather than a defect or a request
    refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 26, 2026
  3. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 26, 2026
  4. added this to the 1.5 milestone on Oct 6, 2026
  5. 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

    designA design or decision issue: a pattern being decided rather than a defect or a requestrefactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions