Skip to content

feat(utilities): one config module for the tunable limits (PCAPKIT_* environment variables), and one home per duplicated protocol constant #1726

Closed
Closed
feat(utilities): one config module for the tunable limits (PCAPKIT_* environment variables), and one home per duplicated protocol constant#1726
Labels
breakingBreaks public-facing behaviour or API (apply alongside the type label)featPull requests that add a new capability (feat: subject prefix)

Description

@JarryShaw
JarryShaw
opened on Oct 14, 2026

Is your feature request related to a problem? Please describe.
The last four weeks added eight tunable limits and about 27 protocol constants, many duplicated across modules (asked in chat). Measured on main (6aeba80e8):

  • Tunables: FRAME_LAYER_LIMIT (64), EXTENSION_HEADER_LIMIT (32), IPV6_TCP_WARNED_LIMIT (32), dumpkit.common.FALLBACK_LIMIT, chardet.DETECT_CACHE_SIZE, corekit.io._READ_CHUNK, and the zero-pad guards in corekit/fields/field.py. Only PCAPKIT_DEVMODE, _VERBOSE and _SPHINX are environment variables today. PCAPKIT_VENDOR_RETRY (which crashes the import on a non-integer) and PCAPKIT_CI_MODE are undocumented.
  • Overrides don't propagate: ipv6.py:38-40, toolkit/dpkt.py, scapy.py and pypcapfile.py copy the two layer limits with from ... import. Changing the defining module therefore leaves those readers on the old value.
  • Duplicates: about 25 bare µs/ns resolution literals across four packages; the IPv4 (20), IPv6 (40) and TCP (20) header lengths in up to four places each, including a bare 20 in toolkit/pcap.py:178; PCAP and PCAP-NG magics; and two overlapping PCAP-NG minimum-block tables.

Describe the solution you'd like

  1. New module: a stdlib-only pcapkit/utilities/config.py. It reads every PCAPKIT_* variable once at import, validates each one, and holds the tunables. utilities is the only cycle-free home: corekit.fields.strings imports utilities.chardet, so a corekit module would loop.
  2. Readers: consumers read config.<NAME> at call time, with no from ... import copies, so one value reaches every engine.
  3. Naming and docs: variables are named PCAPKIT_<NAME>, with one .. envvar:: list in the docs.
  4. Protocol constants: they stay fixed. Each moves to its owning protocol module (IPv4/IPv6/TCP lengths, PCAP and PCAP-NG magics), or to a single resolution pair.
  5. Sequencing: this comes after the refactor(toolkit): revert the cross-engine agreement tests and the adapter code that only mirrors the default engine #1708 dpkt and scapy reverts, which delete about ten of the duplicates.

Describe alternatives you've considered

  • A runtime config.configure(), like logging.configure, as a follow-up.
  • Per-Extractor arguments. context.py:20-29 avoided protocol-specific keyword arguments.

Additional context, the decisions I need from you:

  1. No re-export: does the refactor(corekit)!: move the byte-order field helpers into corekit (#1711) #1715 no-re-export rule apply? Moving a constant would remove names such as pcapkit.protocols.protocol.FRAME_LAYER_LIMIT. That is a breaking change with a !. The alternative is to keep each definition where it is and centralise only the environment read.
  2. Raising a limit: raising FRAME_LAYER_LIMIT past 64 reopens fix(protocols): deep layer nesting of any protocol exhausts the recursion limit and loses TCP silently #1610's RecursionError. Should a higher value be refused, capped, or allowed with a documented risk?
  3. Bad values: should an invalid value warn and fall back to the default, or raise at import?
  4. Vendor variables: should PCAPKIT_VENDOR_RETRY, _CI_MODE and _HTTP(S)_PROXY move into the module too?

Activity

  1. JarryShaw
    added
    featPull requests that add a new capability (feat: subject prefix)
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 14, 2026
  2. JarryShaw
    added this to PyPCAPKiton Oct 14, 2026
  3. JarryShaw
    moved this to Needs decision in PyPCAPKiton Oct 14, 2026

  4. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    JarryShaw
    on Oct 14, 2026
    OwnerAuthor
    1. no re-export
    2. allow raising the limit and allow -1 as no limit. add document for the risks.
    3. bad values warn and fallback to default.
    4. moves as well.

  5. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    JarryShaw
    on Oct 14, 2026
    OwnerAuthor

    Recorded:

    1. No re-export. Each constant moves to its one home, and every old path goes. That makes it a breaking change, marked with a !.
    2. Limits can be raised. -1 means no limit. The docs state the risk: past the default, a deep frame can hit Python's recursion limit, as in fix(protocols): deep layer nesting of any protocol exhausts the recursion limit and loses TCP silently #1610.
    3. A bad value warns and falls back to the default. The warning has to go through the stdlib warnings module, because pcapkit's own warn imports logging and would cycle back into the config module.
    4. The vendor variables move too. PCAPKIT_VENDOR_RETRY, _CI_MODE and _HTTP(S)_PROXY move, and each one gets documented.

    This is blocked until the #1708 dpkt and scapy reverts merge: #1724, and the dpkt PR that follows it. Both rewrite the limit readers in toolkit/dpkt.py and scapy.py that this change has to switch to config.<NAME>.

  6. JarryShaw
    added
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    and removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 14, 2026
  7. JarryShaw
    moved this from Needs decision to Blocked in PyPCAPKiton Oct 14, 2026

  8. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    JarryShaw
    on Oct 14, 2026
    OwnerAuthor
    1. maybe warn on -1 and values over 64 as well
    2. is it possible we still use pcapkit's warning?

  9. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    JarryShaw
    on Oct 14, 2026
    OwnerAuthor

    Recorded for 2: setting a limit to -1, or above its default (64 for the frame-layer limit), also warns, and the warning names the RecursionError risk.

    For 3, yes, with one layout constraint. pcapkit.utilities.warnings imports only exceptions and logging (warnings.py:64-65), so config can import it as long as logging does not import config. So:

    • PCAPKIT_DEVMODE, _VERBOSE and _SPHINX stay in utilities/logging.py. warnings needs them while it is still loading.
      • A bad value for one of them is recorded during import and warned through pcapkit's warn once warnings has finished loading.
      • Today such a value falls back to False silently, and test_logging.py:60-69 pins that. That test would change.
    • Every other variable lives in utilities/config.py, which imports pcapkit.utilities.warnings directly: the limits, the caches and the vendor variables. Each bad or risky value warns through a new ConfigWarning(BaseWarning, RuntimeWarning), so devmode and the existing filters apply as usual.

    Say if the three logging flags should move into config as well. That works too, but only if every warning is deferred until pcapkit/__init__.py finishes importing.

  10. JarryShaw
    added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Oct 14, 2026
  11. JarryShaw
    moved this from Blocked to Needs decision in PyPCAPKiton Oct 14, 2026

  12. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    JarryShaw
    on Oct 14, 2026
    OwnerAuthor

    whats your recommendation?

  13. 14 remaining items

  14. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    JarryShaw
    on Oct 14, 2026
    OwnerAuthor

    take all defaults.

  15. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    JarryShaw
    on Oct 14, 2026
    OwnerAuthor

    Recorded: all three defaults stand. Scapy's _READ_CHUNK stays unconfigurable. The proxy variables are still read on every call. Every limit raised above its default, or set to -1, warns about its own risk. Round 2, already running, implements exactly these.

  16. JarryShaw
    removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 14, 2026
  17. JarryShaw
    moved this from Needs decision to WIP in PyPCAPKiton Oct 14, 2026
  18. JarryShaw
    added 2 commits that reference this issue on Oct 14, 2026

    docs(toolkit): drop agreement claims from adapter docstrings (#1708) (#…

    87d9645

    feat(utilities)!: one config module for the tunable limits (#1726) (#…

    0cdd3cd

  19. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    JarryShaw
    on Oct 14, 2026
    OwnerAuthor

    actually, on second thoughts, vendor's envvars keep in vendor, they dont move to utilities.config

  20. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    JarryShaw
    on Oct 14, 2026
    OwnerAuthor

    Recorded: the vendor variables stay in pcapkit.vendor. #1743 has already merged with them in config, so a follow-up PR moves them back. Defaults below, unless you say otherwise:

    • pcapkit.vendor.default.MAX_RETRY (PCAPKIT_VENDOR_RETRY) and CI_MODE (PCAPKIT_CI_MODE) return under their original names. This undoes feat(utilities)!: one config module for the tunable limits (#1726) #1743's removal of those two names, along with its versionremoved notes.
    • get_proxies() reads PCAPKIT_HTTP_PROXY and PCAPKIT_HTTPS_PROXY itself, on every call, as before.
    • feat(utilities)!: one config module for the tunable limits (#1726) #1743's handling stays: a bad value warns with ConfigWarning and falls back. The parsing helpers become public in config so vendor can reuse them without reaching into private names.
    • config.VENDOR_RETRY, config.CI_MODE, config.http_proxy and config.https_proxy are removed outright. They never shipped in a release, so they get no versionremoved.
    • The vendor variables are documented on the vendor page, and config.rst links there.

    Ruling 7 (#1739) is unaffected and is in progress.

  21. JarryShaw
    added 2 commits that reference this issue on Oct 14, 2026

    refactor(vendor): keep the vendor variables in pcapkit.vendor (#1726)

    e41eaf0

    refactor(vendor): keep the vendor variables in pcapkit.vendor (#1726) (…

    d651060

  22. JarryShaw commented on Oct 15, 2026

    @JarryShaw
    JarryShaw
    on Oct 15, 2026
    OwnerAuthor

    All of this has now landed:

    Closing.

  23. JarryShaw
    closed this as completedon Oct 15, 2026
  24. JarryShaw
    removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 15, 2026
  25. github-project-automation
    moved this from WIP to Done in PyPCAPKiton Oct 15, 2026
  26. JarryShaw
    added a commit that references this issue on Oct 15, 2026

    docs(foundation): second pass over engines, toolkit and reassembly (#…

    c25c28c
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

    breakingBreaks public-facing behaviour or API (apply alongside the type label)featPull requests that add a new capability (feat: subject prefix)

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions