Skip to content

ci(util): fetch with requests and poll memory with psutil in the scripts (#1894) - #1896

Open
JarryShaw wants to merge 1 commit into
mainfrom
ci/1894-requests-psutil-scripts
Open

JarryShaw wants to merge 1 commit into
mainfrom
ci/1894-requests-psutil-scripts

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

These follow the owner's requests on #1885.

  • requests: used by util/ci_timing.py, util/update_python_versions.py and examples/generators/pcapng.py. The pcapng generator keeps its retry, Retry-After and offline fallback. A test against loopback servers shows ci_timing.py still keeps the token off a redirect to another host.
  • markdown-it-py: parse_images() reads the runner-images README table with it, so an escaped pipe stays in its cell.
  • lxml: parse_junit() and the PDML read in util/pyshark_encap_map.py use it, through a parser that resolves no entities and makes no network access. There is no ElementTree fallback, and the tests that parse skip on GraalPy, which has no lxml wheel.
  • psutil: util/vendor_test_gate.py samples the test run's process group with it, and falls back to ps on PyPy and GraalPy, which have no psutil wheel.
  • Installs: the test extra gains these three packages. ci-timing.yml installs requests and lxml, and the detect job installs requests and markdown-it-py. update_python_versions.py publish still runs on a bare python3.
  • Tests: each changed behaviour has a test that fails on the old code. tests/project and the tier guard pass.

Closes #1894

@JarryShaw JarryShaw added dependencies Pull requests that update a dependency file ci Pull requests that change CI or workflow configuration (ci: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 17, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: NEEDS CHANGES at fcbaf91c2. The one required change is a prose fix: a test docstring counts code items ("the same three errors"), and so does a module docstring ("the two binaries"). Both are being reworded. A Sonnet cross-review found no functional problems:

  • Every job that loads a changed script installs what it imports. publish ran to completion on a bare python3 with requests, markdown-it-py, lxml and psutil blocked.
  • The lxml parser resists every attack it tried: XXE in text or in an attribute, out-of-band parameter entities, a network DTD, billion laughs, malformed input and 5000-deep nesting. Nothing was read or fetched, and the JUnit output matches main's.
  • psutil and ps report the same group memory. An access-denied member makes the sample unknown, and the memory-cap kill works with and without psutil.
  • parse_images() returns the same images as before on the saved and the live runner-images README.
  • The retry, Retry-After, 404 and redirect-token behaviour is unchanged against real local servers.
  • The new tests fail on main. tests/project and the tier guard pass.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 17, 2026
@JarryShaw
JarryShaw force-pushed the ci/1894-requests-psutil-scripts branch from fcbaf91 to 3549e07 Compare October 17, 2026 15:50
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 17, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 17, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 3549e071d. The amend changes only docstrings. With docstrings stripped, the ASTs of the three files match the previous head's.

The counts are gone: "three errors" in test_python_versions_update_1660.py, "the two binaries" in test_pyshark_encap_map.py, and "both of which" in util/ci_timing.py. A Sonnet scan of every added docstring and comment line found no other count of code items.

The three touched test files pass. Every functional finding from the review of fcbaf91c2 still holds.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 17, 2026
@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 17, 2026
Comment thread tests/project/test_pyshark_encap_map.py Fixed
@JarryShaw JarryShaw removed the review: pending No verdict for the current head - never reviewed, or the head moved since the last one label Oct 17, 2026
@JarryShaw
JarryShaw force-pushed the ci/1894-requests-psutil-scripts branch from 3549e07 to efa8c21 Compare October 17, 2026 16:35
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 17, 2026
…pts (#1894)

- util/ci_timing.py: fetch through a requests session; an HTTPError's status
  decides the retry and the 404 note, the Link header is parsed as requests
  parses it, and the token staying off a cross-host redirect is tested against
  loopback servers. ci-timing.yml installs requests and lxml.
- util/update_python_versions.py: fetch() uses requests and parse_images()
  reads the runner-images table with markdown-it-py, so an escaped pipe stays
  in its cell; both import lazily, so `publish` still runs on a bare python3.
  The detect job installs both.
- examples/generators/pcapng.py: download with requests, keeping the retry,
  Retry-After and offline fallback; a timeout mid-body is still retried.
- util/vendor_test_gate.py: sample a run's process group with psutil, and
  with ps where psutil is not installed.
- util/ci_timing.py parse_junit() and util/pyshark_encap_map.py's PDML read:
  lxml with a hardened parser instead of ElementTree, no fallback; the tests
  that parse are gated on HAS_LXML.
- pyproject.toml: the test extra carries psutil (CPython only), markdown-it-py
  and lxml (not on GraalPy).
- Pipfile [dev-packages]: the same, with the test extra's markers, pinned by a test.

Closes #1894
@JarryShaw JarryShaw removed the review: pending No verdict for the current head - never reviewed, or the head moved since the last one label Oct 17, 2026
@JarryShaw
JarryShaw force-pushed the ci/1894-requests-psutil-scripts branch from efa8c21 to 3a7a1cd Compare October 17, 2026 16:56
@JarryShaw JarryShaw added the review: running A cross-review is in flight against the current head - no verdict yet label Oct 17, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 3a7a1cd51. Since 3549e071d, the PR has changed in four ways:

  • Pipfile: psutil, lxml and markdown-it-py are in [dev-packages], with the test extra's markers.
  • CodeQL: the XXE canary is renamed from secret to canary. CodeQL now passes, and the PR has no open alerts.
  • dev extra: the additions are reverted, since process.rst reserves dev for what the toolchain must resolve for pcapkit/.
  • New test: tests/project/test_script_dependencies_1894.py keeps the test extra and the Pipfile in step.

Sonnet re-reviews checked each amend:

  • the renamed tests' ASTs match apart from the identifier;
  • pyproject.toml now differs from main only in the test extra;
  • the new test fails against main's Pipfile;
  • the touched test files, tests/project and the tier guard pass.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 90.07% (unit tier, Python 3.15, 3a7a1cd51, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 2 20 2 96.77%
pcapkit/const 19595 881 2618 867 92.11%
pcapkit/corekit 2504 70 792 36 96.54%
pcapkit/dumpkit 276 3 100 3 98.40%
pcapkit/foundation 2916 164 1016 34 93.59%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17411 224 4580 189 98.10%
pcapkit/toolkit 872 177 298 8 77.18%
pcapkit/utilities 522 2 158 4 99.12%
pcapkit/vendor 4431 2341 1018 156 43.64%

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Pull requests that change CI or workflow configuration (ci: subject prefix) dependencies Pull requests that update a dependency file review: good-to-go Cross-review at the current head says ready; CI state is separate

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

ci(util): use requests for the scripts' HTTP fetches and psutil for vendor_test_gate's memory polling

2 participants