Zero-risk cleanup pass for envoy.distribution.release review findings

[Copilot]

This PR applies the mechanical, behavior-preserving cleanup items from py/envoy.distribution.release/REVIEW.md (findings 1.2, 1.3, 1.4, 6.2, 9.2, 11.1, 12.1, 12.2, 12.3). Scope is limited to dead code removal, comment cruft removal, redundant delegation cleanup, and fixing no-op assertions in existing tests.

• Dead module removal

  • Deleted:
    • py/envoy.distribution.release/envoy/distribution/release/abstract.py (empty/unreferenced)
    • py/envoy.distribution.release/envoy/distribution/release/exceptions.py (PackagesConfigurationError unused in this package)

• Commented-out cruft cleanup

  • Removed stale commented import from:
    • py/envoy.distribution.release/envoy/distribution/release/__init__.py
  • Removed commented profiler artifacts from:
    • py/envoy.distribution.release/envoy/distribution/release/cmd.py

• ReleaseRunner redundancy cleanup

  • In py/envoy.distribution.release/envoy/distribution/release/runner.py:
    • Removed redundant command, commands, and release_manager overrides that only delegated to super()
    • Removed redundant add_arguments override that only delegated to super()
    • Removed now-unused cached_property import
    • Kept release_manager_class and decorated run intact

• Test correctness fixes (no-op expressions -> assertions)

  • py/envoy.distribution.release/tests/test_runner.py
    • isinstance(...) expression converted to assert isinstance(...)
    • Updated delegation expectations to match inherited behavior after override removal
  • py/envoy.github.release/tests/test_release.py
    • Bare equality expression converted to assert ... == ...

• Reference cleanup confirmation

  • No remaining imports/references to envoy.distribution.release.abstract or envoy.distribution.release.exceptions in repository code/tests.

# before (no-op)
isinstance(run, AGithubReleaseRunner)
release.version_name == _manager.format_version.return_value

# after
assert isinstance(run, AGithubReleaseRunner)
assert release.version_name == _manager.format_version.return_value

Original prompt

Zero-risk cleanup pass on py/envoy.distribution.release following the code review in #4479 and the initial cleanup in #4483. Bundles findings 1.2, 1.3, 1.4, 6.2, 9.2, 11.1, 12.1, 12.2, 12.3 from py/envoy.distribution.release/REVIEW.md.

Scope

All changes are mechanical and behaviour-preserving. No functional changes. No new tests required (one existing test is fixed).

1. Delete dead modules

• Delete py/envoy.distribution.release/envoy/distribution/release/abstract.py — empty 0-byte file, unimported anywhere (finding 1.4).
• Delete py/envoy.distribution.release/envoy/distribution/release/exceptions.py — defines PackagesConfigurationError which is never raised, never exported, and the import in __init__.py is already commented out (finding 6.2).

2. Remove commented-out cruft

• In py/envoy.distribution.release/envoy/distribution/release/__init__.py: remove the commented line # from .exceptions import PackagesConfigurationError (finding 12.1). After removing, verify the file isn't left empty/awkward — if it only contained that one comment, an empty file is fine.

• In py/envoy.distribution.release/envoy/distribution/release/cmd.py: remove the two commented lines around L26-L28:

  • # from memory_profiler import profile
  • # @profile

  (findings 9.2 / 12.2).

3. Drop redundant ReleaseRunner overrides

In py/envoy.distribution.release/envoy/distribution/release/runner.py (findings 1.2 and 1.3):

• Remove the three @cached_property overrides whose bodies are literally return super().<property> — command, commands, and release_manager. They add caching but no behaviour; if caching is needed it belongs in the abstract base. For now, just remove the overrides and let them inherit. The cached_property import becomes unused — remove it too.
• Remove the add_arguments override that contains only a super().add_arguments(parser) call (no extra arguments registered).
• Keep release_manager_class and the decorated run method — those are doing real work.

After this change, runner.py should still define ReleaseRunner with release_manager_class and the run method (with its @runner.catches decorator).

4. Fix no-op isinstance checks in tests

These are bare isinstance(...) calls evaluated and discarded — no assertion is made, so the test always passes regardless of result.

• py/envoy.distribution.release/tests/test_runner.py:L19 — change isinstance(run, AGithubReleaseRunner) to assert isinstance(run, AGithubReleaseRunner) (finding 11.1).
• py/envoy.github.release/tests/test_release.py:L231 — same pattern, an equality check == used as an expression statement rather than assert ... == .... Convert to a proper assert (finding 12.3).

Verification

• Run the test suites for both py/envoy.distribution.release and py/envoy.github.release — they should still pass.
• Confirm no remaining imports reference envoy.distribution.release.abstract or envoy.distribution.release.exceptions (use grep across the repo, including tests).
• Confirm cached_property and any other now-unused imports are removed from runner.py.

Out of scope

• Anything in the report's "Recommended follow-up PRs" table other than items libs: Add aio.functional #3 and libs: Add aio.subprocess #7. In particular, do not touch commands.py push/fetch error-surfacing (that is a separate PR with tests).
• Do not modify the version regex in envoy.github.release/manager.py — finding 8.3 in the report is a false positive (the regex is used with re.sub, not re.match/fullmatch, so the unmatched tail is preserved correctly).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: school report is in - wdyt?

@envoyproxy/toolshed/pull/4483 might address some

Assistant: ## School report verdict

It's a solid second-pass review — well-structured, accurate, and the prioritisation at the end is sensible. A few observations:

Strongest findings (worth acting on):

• 8.3 version regex r"v(\w+)" — genuinely a bug; \w doesn't match . so v1.19.0 collapses to Version("1"). High-risk and correct.
• 2.1 PushCommand.run() / FetchCommand.run() discard the errors dict — real correctness issue; CI gets green on partial failure. The annotation mismatch (10.2) is a nice corollary.
• 3.1 dead ConcurrentIteratorError handler + 6.1 raise e.args[0] losing chain — both legit.
• 4.1 / 4.2 (no session timeouts, response written to disk before status check) — straightforward, mid-risk.
• 3.3 / 5.1 missing cache=True on manager.releases / latest — concrete and trivially fixable.

Padding / borderline:

• 1.5 (FetchCommand.path shadowing runner.path) — theoretical; nothing actually accesses the wrong one.
• 2.3 draft-vs-published support — that's a feature request dressed as a finding.
• 10.3 `Rel...

This pull request was created from Copilot chat.

[netlify]

✅ Deploy Preview for nifty-bassi-e26446 ready!

Name	Link
🔨 Latest commit	6654e31
🔍 Latest deploy log	https://app.netlify.com/projects/nifty-bassi-e26446/deploys/6a01c71841a0d00008e3d50f
😎 Deploy Preview	https://deploy-preview-4486--nifty-bassi-e26446.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.