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