Skip to content

fix(foundation): warn once when an extraction engine is unavailable (#1045) - #1046

Merged
JarryShaw merged 4 commits into
mainfrom
fix/1045-engine-warn-once
Oct 5, 2026
Merged

JarryShaw merged 4 commits into
mainfrom
fix/1045-engine-warn-once

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat
  • perf
  • refactor
  • test
  • docs
  • ci
  • chore

Description

Closes #1045

A missing engine package raised two EngineWarnings: Extractor.import_test warned and returned None, then Extractor.run()'s else branch warned again.

Site chosen: import_test warns, run() is silent. The engines' docstrings (pypcap.py, pcap_ct.py, pyshark.py) say the import test reports absence, and import_test is public API documented as warning.
Message: import_test now uses the more informative wording naming the module (engine PyPCAP (`pcap`) is not installed). Fallback to the default engine and the logger.debug lines are unchanged.

Tests (tests/foundation/test_extraction.py): exactly one warning for an absent module and for an engine blocked by unsupported_reason, and the default engine is selected. The absent-module test fails on main (2 warnings); the blocked test is a regression guard (already one warning on main).

…1045)

Extractor.import_test warned that the engine package was missing and returned
None; run()'s else branch then warned again, so one problem raised two
EngineWarnings. run() now stays silent there, and import_test's message names
the module (engine PyPCAP (`pcap`) is not installed).
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 5, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on f44d0bc53: NEEDS CHANGES (ran on Opus).

  1. pcapkit/foundation/extraction.py:737 — with no name, the module prints twice: Extractor.import_test('definitely_missing_mod') now warns engine definitely_missing_mod (definitely_missing_mod) is not installed; …. Add the (`{engine}`) suffix only when name is given.
  2. Add a Fixed entry to docs/source/changelog/1.5.0.rst and regenerate CHANGELOG.md with util/changelog_md.py. The double warning shipped in v1.4.1 (extraction.py:439 and :480 there), and public import_test text changes. The sibling foundation fixes fix(foundation): name the base class the registrar guard actually checks #1020, fix(foundation): declare the engine-typed slots as EngineBase, not Engine #1023, fix(foundation): raise RegistryError, not a leaked TypeError, for a non-class #1025 and Seven register/issubclass guards in the protocol classes still leak a bare TypeError for a non-class #1026 each added an entry. 1.5.0.rst:762-767 already says an engine that cannot run gives one warning; that was false for a missing package until this PR.

Confirmed: per-engine warning counts in fresh processes — pypcap and pcap_ct 2→1; default, pcapkit, dpkt, scapy 0; pyshark, pypcapfile, unknown 1 on both trees, all falling back cleanly. run() at :684 is the only caller. The new test fails on main with 2 warnings. No stale quotes of the old text anywhere. tests/foundation/test_extraction.py 21 passed, all tests/foundation/engines/* pass, tests/project 275 passed. Pylint is 9.77 on both trees with the repo's flags, so the 8.88 in my brief came from a mis-split invocation, not this diff.

Non-blocking: a subclass import_test that returns None without warning now falls back silently, since the else no longer warns.

@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 5, 2026
… name (#1045)

import_test printed engine foo (`foo`) when called without a display name. The (`module`) suffix is now emitted only when name is given. Adds a regression test (failed before the fix: 2 != 1), a Fixed changelog entry, regenerated CHANGELOG.md, and moves the entry-count pin 162 -> 163.
@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 5, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on cb3a4fd70: NEEDS CHANGES (ran on Opus).

The code, the new test, CHANGELOG.md (changelog_md.py --check in step) and the process.rst:104 pin (163, measured) are all correct. The changelog entry at docs/source/changelog/1.5.0.rst:1039 needs two corrections:

  1. Drop "An import_test call with no display name names the module once, not twice." On main the unnamed call already named the module once (extraction engine 'x' not available; …). The double naming existed only in this PR's f44d0bc53, so it never shipped and there is nothing to release-note.
  2. "Code that matches the old wording … must be updated" overstates the change. The surviving text, engine <name> (`<module>`) is not installed; using default engine instead, is byte-identical to the second warning that run() printed on main. Only a match on the import_test wording breaks, and so does anything expecting two warnings.

Non-blocking: the entry writes (<module>) because the generator rejects nested backticks. Saying the module name appears in backticks would keep an exact-match reader right.

Confirmed by probe: named and unnamed text on all three trees; pypcap and pcap_ct 2→1 warnings; the new test fails on f44d0bc53 (2 != 1) and passes on head. test_extraction.py 22 passed; tests/project 275 passed.

@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 5, 2026
Drop the claim that an unnamed import_test call named the module twice (only true in this PR's first commit, never shipped), and narrow who breaks: the surviving text equals the warning run() already emitted on main.
@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 5, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 0b0bf7ab1: GOOD TO GO (ran on Opus).

The changelog entry no longer claims an unnamed-case fix. Its "only code that matched import_test's old wording, or expects two warnings, breaks" is accurate. run always passes name=eng.name, so the surviving message is byte-identical to origin/main's run() warning at extraction.py:693. The backtick note is true; the generator rejects nested backticks. changelog_md.py --check is in step, and the pin is 163 against a measured 163. The commit touches no code or tests, so round 2's probes and test runs on cb3a4fd70 still stand.

Nit, not blocking: a direct import_test call with no name gives engine <module> is not installed; …, without the parenthesised module.

@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 5, 2026
@JarryShaw
JarryShaw merged commit 4873066 into main Oct 5, 2026
11 checks passed
@JarryShaw
JarryShaw deleted the fix/1045-engine-warn-once branch October 5, 2026 20:18
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 5, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

extraction: a missing engine package raises two EngineWarnings

1 participant