Skip to content

refactor: move shared helper functions into utilities or corekit by use case #1519

Closed

Description

@JarryShaw

The owner, in chat, extended #1516: shared helper functions should move to pcapkit.utilities or pcapkit.corekit, depending on what they are used for. Helper and proxy classes are #1516's scope.

Rule of thumb:

  • utilities: generic, protocol-agnostic code, such as decorators, validation and warnings.
  • corekit: the library's core data model and I/O primitives.

Scope:

  1. Inventory the module-level helper functions that are duplicated across modules, or defined privately in one module and imported by others. For each, record where it lives, who calls it, and the proposed home.
  2. Move them in slices that avoid in-flight PRs' files. Keep behaviour byte-identical, and update imports and docs.
  3. Add tests that pin each moved helper at its new location.

The #1516 worker is producing the inventory. The moves follow in separate PRs.

Activity

  1. added
    refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 9, 2026
  2. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    Inventory (functions). Nothing has moved yet.

    Helper Where Proposed Waits on
    _same_entry, _restore_shipped, _restore_shipped_dumper (private, imported by traceflow.py) foundation/extraction.py:98,129,156 corekit.module #1511
    byteorder_callback, same role in two places schema/misc/pcap/frame.py:21, schema/misc/pcapng.py:192 corekit.fields #1497
    packet2chain and packet2dict, identical (AST-checked) toolkit/pypcap.py:59,79 = toolkit/pcap_ct.py:60,80 a shared libpcap toolkit module —
    9 schema helpers, 3 identical and the rest differing only in the error prefix schema/internet/hopopt.py and ipv6_opts.py, lines 132–351 a shared IPv6-options schema module —
    _test_start_line (private, imported by http.py) protocols/application/httpv1.py:85 protocols/application/http.py, the shared base —

    Question for the owner: the last three helpers are shared, but they are engine- or protocol-specific, so neither utilities nor corekit fits them. Is it OK to put each in a shared module inside its own subpackage? I recommend yes, and those three can start right away.

  3. added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    and removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 9, 2026
  4. moved this from WIP to Needs decision in PyPCAPKiton Oct 9, 2026
  5. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    _same_entry, _restore_shipped, _restore_shipped_dumper - promote to public and move to maybe interface instead? can create a new module under it

    byteorder_callback - yes, maybe these schema helpers can live in schema module still or create a corekit.schema for them

    packet2chain and packet2dict - keep them as is, in case they diverge in the future updates of pcap-ct

    9 schema helpers - yes, maybe these schema helpers can live in schema module still or create a corekit.schema for them

    _test_start_line - yes, move to http base module

  6. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    Recorded owner rulings:

    • _same_entry, _restore_shipped, _restore_shipped_dumper become public and move into a new module under pcapkit.interface. I checked for an import cycle: with extraction.py importing a new pcapkit.interface submodule at top level, importing pcapkit, pcapkit.foundation.extraction, foundation.traceflow.traceflow and pcapkit.interface all succeed, since pcapkit/__init__.py:96 loads interface first. This slice is queued until refactor(corekit): move the stream proxy classes into corekit.io (#1516) #1521 merges, because both edit extraction.py.
    • byteorder_callback and the 9 HOPOPT/IPv6-Opts schema helpers are shared, and stay in the pcapkit.protocols.schema package rather than going to a new corekit.schema. The selectors return protocol schema classes, and corekit must not import protocols.
    • packet2chain and packet2dict stay duplicated, since pcap-ct may diverge.
    • _test_start_line moves to the http base module.

    Workers are starting on the schema helpers and on _test_start_line now.

  7. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 9, 2026
  8. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    _test_start_line maybe become test_start_line as well?

  9. JarryShaw commented on Oct 9, 2026

    @JarryShaw
    OwnerAuthor

    Recorded: _test_start_line becomes public as test_start_line and moves to the http base module.

    Its docstring requires it to accept exactly the start lines the HTTP/1 parser accepts, which is why it currently sits beside _RE_METHOD, _RE_VERSION, _RE_STATUS and _HTTP2_PREFACE_HEADER (httpv1.py:63-82). So those patterns move to http.py with it, and httpv1.py imports them from there.

    httpv1.py already imports http.py at top level (:37-38), so this adds no import cycle. It also removes the lazy import at http.py:476-477.

    A worker is starting on this now, and another on the schema helpers.

  10. 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