Skip to content

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

Merged
JarryShaw merged 1 commit into
mainfrom
feat/1726-config-module
Oct 14, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
feat/1726-config-module

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Refs #1726. This PR covers rulings 1–6. Ruling 7, one home per duplicated RFC constant, follows in #1739. It also closes #1708, by fixing that issue's last three agreement lines and dropping its guard's PENDING allowlist.

  • New pcapkit/utilities/config.py: every tunable is read once from a PCAPKIT_* variable. That covers the frame-layer and extension-header limits, the IPv6-TCP warned cache, the dumpkit fallback, the chardet cache, the read chunk, the three zero-pad guards, and the vendor retry and CI settings. The proxies are read per call, as before. config imports only utilities.warnings.
  • Readers look up config.<NAME> at call time, so one patch reaches every engine.
  • Bad or risky values: a bad value warns with ConfigWarning, naming the variable, the value and the importing module, then falls back to the default. -1 lifts a limit; that, or a value above the default, warns of its own risk. Bad DEVMODE/VERBOSE/SPHINX values now warn too.
  • Breaking: the old homes are removed with no re-export. Each carries a versionremoved:: 1.5.0, and the docs extra now needs Sphinx>=7.3.
  • Docs: docs/source/pcapkit/utilities/config.rst gives every variable's default, range and risk.
  • Tests: a literal declaration table covers all 13 variables, with a patched case and a -1 case per reader. Across five Sonnet rounds, every mutant is killed except one proven equivalent.

Closes #1708

Every tunable limit and the vendor crawlers' retry and CI settings move to
the new pcapkit.utilities.config, each read once from a PCAPKIT_* variable.
Readers look them up as config.<NAME> at call time, so one patch reaches
every engine. A bad value warns with the new ConfigWarning, naming the
variable and the value, and falls back; -1 lifts a limit, and that or a
value above the default warns of the risk. The proxy variables are still
read on every call, through config.http_proxy() and https_proxy().

A bad PCAPKIT_DEVMODE, PCAPKIT_VERBOSE or PCAPKIT_SPHINX is recorded by
utilities.logging and reported by utilities.warnings once it has loaded.
Both flag readers strip whitespace, and a ConfigWarning given at import
names the importing module rather than <sys>:0. The docs extra now needs
Sphinx>=7.3, for versionremoved.

BREAKING CHANGE: the old homes are removed, with no re-export:
protocols.protocol.FRAME_LAYER_LIMIT,
protocols.internet.internet.EXTENSION_HEADER_LIMIT,
toolkit.pypcapfile.IPV6_TCP_WARNED_LIMIT, dumpkit.common.FALLBACK_LIMIT,
utilities.chardet.DETECT_CACHE_SIZE, corekit.io._READ_CHUNK, the
corekit.fields.field zero-pad guards, and vendor.default.MAX_RETRY and
CI_MODE.

Refs #1726
@JarryShaw JarryShaw added feat Pull requests that add a new capability (feat: subject prefix) breaking Breaks public-facing behaviour or API (apply alongside the type label) labels Oct 14, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 14, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at d74af19ad. A Sonnet cross-review checked it over five rounds (author: Opus). Rounds 1–3 each returned NEEDS CHANGES, all fixed in later rounds.

  • Rulings:
  • Mutation testing:
    • every reader, -1 branch, budget edge, env-var name, default, minimum and risk phrase has a killing test;
    • of 107 + 33 + 3 mutants, only one survives, and it is proven equivalent: no value config accepts behaves differently;
    • stale import-time copies, the bug this issue is about, are killed in every reader.
  • Suites: tests/protocols/internet 500 passed, tests/utilities 144, tests/vendor 129, tests/dumpkit 73. Sphinx has 63 warnings on both trees.
  • My own re-run in the PR's tree: test_config_unit, test_logging, isort, the 1735 boundary, the 1708 guard, documentation_claims, public_api and all_exports all pass.

@JarryShaw JarryShaw added the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 14, 2026
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.69% (unit tier, Python 3.15, d74af19ad, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18860 954 2368 868 91.06%
pcapkit/corekit 2457 69 780 35 96.54%
pcapkit/dumpkit 276 3 100 3 98.40%
pcapkit/foundation 2906 164 1016 34 93.57%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17174 225 4588 190 98.07%
pcapkit/toolkit 864 182 298 9 76.33%
pcapkit/utilities 518 2 158 4 99.11%
pcapkit/vendor 4415 2293 1010 160 44.57%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

The red here is GraalPy alone (#1740). Its runner was lost (The runner has received a shutdown signal), so Required checks passed failed. Every other leg passed, and the KTLO legs fail 0 tests, the same as main. This branch predates #1742, so its Unit Tests run still waits on GraalPy. I have re-run the failed jobs. Once #1742 merges, bringing this branch up to date takes GraalPy out of the required path. The verdict above stands.

@JarryShaw
JarryShaw merged commit 0cdd3cd into main Oct 14, 2026
89 of 91 checks passed
@JarryShaw
JarryShaw deleted the feat/1726-config-module branch October 14, 2026 22:27
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 14, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 14, 2026
JarryShaw added a commit that referenced this pull request Oct 14, 2026
The crawlers' variables move back out of pcapkit.utilities.config, which
now holds the tunable limits only.

- Restore pcapkit.vendor.default.MAX_RETRY (PCAPKIT_VENDOR_RETRY,
  default 5, minimum 1) and CI_MODE (PCAPKIT_CI_MODE) under their 1.4.x
  names, and drop #1743's versionremoved notes for them.
- Read them with config.read_limit and config.read_flag, made public
  from _read_limit and _read_flag, so a bad value still warns with
  ConfigWarning, naming the variable and the value, and falls back.
- get_proxies() reads PCAPKIT_HTTP_PROXY and PCAPKIT_HTTPS_PROXY itself
  on every call. config.VENDOR_RETRY, CI_MODE, http_proxy and
  https_proxy never shipped, so they are removed outright.
- Document the four variables on the pcapkit.vendor.default page, which
  config.rst and index.rst link to.
- Move #1743's vendor tests to tests/vendor/test_vendor_variables_1726_unit.py,
  and run it in project-tests' docs-only step, since it reads the docs.

Refs #1726
JarryShaw added a commit that referenced this pull request Oct 14, 2026
…#1745)

The crawlers' variables move back out of pcapkit.utilities.config, which
now holds the tunable limits only.

- Restore pcapkit.vendor.default.MAX_RETRY (PCAPKIT_VENDOR_RETRY,
  default 5, minimum 1) and CI_MODE (PCAPKIT_CI_MODE) under their 1.4.x
  names, and drop #1743's versionremoved notes for them.
- Read them with config.read_limit and config.read_flag, made public
  from _read_limit and _read_flag, so a bad value still warns with
  ConfigWarning, naming the variable and the value, and falls back.
- get_proxies() reads PCAPKIT_HTTP_PROXY and PCAPKIT_HTTPS_PROXY itself
  on every call. config.VENDOR_RETRY, CI_MODE, http_proxy and
  https_proxy never shipped, so they are removed outright.
- Document the four variables on the pcapkit.vendor.default page, which
  config.rst and index.rst link to.
- Move #1743's vendor tests to tests/vendor/test_vendor_variables_1726_unit.py,
  and run it in project-tests' docs-only step, since it reads the docs.

Refs #1726
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

refactor(toolkit): revert the cross-engine agreement tests and the adapter code that only mirrors the default engine

1 participant