Skip to content

refactor(corekit): move stream proxy classes such as _PeekableStream into corekit.io #1516

Closed

Description

@JarryShaw

The owner asked in chat for _PeekableStream to move into pcapkit.corekit.io, and for any similar proxy or wrapper classes to move to their proper homes in corekit. #1509 added _PeekableStream.

Known candidates on main 6af87f608:

Scope:

  1. Inventory every proxy, wrapper or adapter class outside corekit, and propose a corekit home for each one. Leave the lazy-attribute __getattr__ data classes in foundation/reassembly/data and foundation/traceflow/data in place unless they are genuinely generic.
  2. Move the classes, update the imports and the docs pages, and keep behaviour byte-identical.
  3. Add tests that pin each moved class from its new module.

Open question: whether the classes stay underscore-private once they live in corekit. The default is to make them public, like SeekableReader, because they would no longer be file-local.

Activity

  1. added
    refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)
    pendingAccepted and unstarted - nobody is on it and nothing blocks it
    on Oct 9, 2026
  2. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    A worker is on this: inventory first, then move the classes into corekit.io, then tests. Default taken: the moved classes become public, like SeekableReader.

  3. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    pendingAccepted and unstarted - nobody is on it and nothing blocks it
    on Oct 9, 2026
  4. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    Scope widened by the owner in chat: every helper or proxy class outside corekit is in scope, not just the stream wrappers. Shared helper functions are tracked in #1519. The worker is extending its inventory to both. This PR moves the stream wrappers plus any class that doesn't touch an in-flight PR's files; the rest follow in separate PRs.

  5. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    Inventory (classes). #1521 moves PeekableStream and NamedStream into corekit.io. Everything else either waits for an in-flight PR or needs a call:

    Class Where Proposed Waits on
    PortEnumField ×3, differing only in the enum schema/transport/{sctp,tcp,udp}.py one field in corekit/fields/numbers.py, with a proto parameter #1497
    _EnumRegistry protocols/schema/schema.py:1187 corekit.multidict #1484
    DeferredPacket ×2, near-identical foundation/{reassembly,traceflow}/data/data.py one corekit.infoclass mixin #1484, and a call
    PCAPNGReader foundation/engines/dpkt.py:46 stays: it imports foundation/protocols, which corekit must not —

    Stay where they are: protocol schema fields, the ESP and MH value types, the utilities.compat backports, metaclasses and dumpers.

    Question for the owner: should the two DeferredPacket classes merge into one corekit.infoclass mixin? Each docstring names the other as its counterpart, and they differ only in what they resolve. My recommendation is to merge them after #1484.

  6. added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 9, 2026
  7. moved this from WIP to Needs decision in PyPCAPKiton Oct 9, 2026
  8. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    PortEnumField - good
    _EnumRegistry - rename as EnumRegistry and maybe put in corekit.enum? or a new module like registry or if other better choice (never touch corekit.multidict, keep it a tailored clean copy from Werkzeug)
    DeferredPacket - merge, and maybe create a new module for it, like packet or if other better choice
    PCAPNGReader - stays

  9. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    Recorded owner rulings:

    • PortEnumField: the three copies become one field in corekit/fields/numbers.py.
    • DeferredPacket: the two copies merge into a new module. I'm taking pcapkit.corekit.packet.
    • PCAPNGReader: stays.
    • _EnumRegistry: made public and moved into corekit. corekit.multidict must stay a clean Werkzeug copy and is not touched.

    Workers are starting on PortEnumField and DeferredPacket now. #1497 has merged, and no open PR touches their files.

    One question on _EnumRegistry. It can't be called EnumRegistry in corekit.enum, because that name is taken. pcapkit.corekit.enum.EnumRegistry (enum.py:436) is the public base of every open pcapkit.const enumeration, and pcapkit.all exports it. What _EnumRegistry actually is: a defaultdict holding EnumSchema subclasses by code, which never inserts on a lookup miss.

    My recommendation is SchemaRegistry in a new pcapkit.corekit.registry module. pcapkit.foundation.registry is a different module, the public register_* API, so corekit.registry sits near that name but doesn't clash with it. Okay, or would you prefer another name?

  10. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    what exactly does this _EnumRegistry do?

  11. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    What _EnumRegistry does (protocols/schema/schema.py:1187):

    It is a collections.defaultdict subclass that overrides only __missing__: on a miss it returns default_factory() without storing it.

    • Every EnumSchema family keeps a class-level registry, __enum__, mapping each code to its schema class. EnumSchema.__init_subclass__ creates it as _EnumRegistry(cls.__default__) (:1366), and swaps a plain defaultdict declared in a class body for one (:1376).
    • Call sites read it with a bare registry[code], e.g. Option.registry[type]. An unknown code then falls back to the Unknown* or Unassigned* schema, by design.
    • A plain defaultdict would insert that fallback on the first miss. Because the registry lives on the class, one packet with an unrecognised code would then permanently grow a process-wide registry. A later, legitimate register(code, ...) would also report an overwrite that never happened.

    So the class is not tied to enums at all. It is a generic "default-on-miss, never insert" dict, used only by the schema registries. The two names that fit are:

    • SchemaRegistry, named for its role, in a new corekit.registry. This is my recommendation, since it is only used there.
    • FallbackDict, named for its behaviour, if you would rather keep it generic.
  12. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    im thinkin put still in schema since it's only used there, especially it's a subclass of defaultdict. if we wanna move it to corekit (maybe collections? to match defaultdict), i would consider moving it to one of the collections.abc subclasses and implement the default_factory() by hand.

  13. 3 remaining items

  14. moved this from Needs decision to WIP in PyPCAPKiton Oct 9, 2026
  15. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    Reopened: #1521's Closes keyword closed this, but the DeferredPacket slice is still open in #1532. This closes when #1532 merges.

  16. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    Closing: #1532 has merged (de72d0154). Both DeferredPacket classes are now pcapkit.corekit.packet.DeferredPacket, and #1521 already moved the stream proxies into corekit.io, so every slice of this issue is on main.

  17. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 10, 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

    refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions