Skip to content

Skip argument conversion for disabled SSH log messages - #841

Merged
ronf merged 1 commit into
ronf:developfrom
Vmnebula:fix/skip-disabled-log-argument-conversion
Nov 7, 2026
Merged

ronf merged 1 commit into
ronf:developfrom
Vmnebula:fix/skip-disabled-log-argument-conversion

Conversation

@Vmnebula

@Vmnebula Vmnebula commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #839 using the early isEnabledFor() guard suggested in the report.

SSHLogger.log() currently decodes bytes and converts list elements even when the underlying logger will discard the message. Check the effective log level before doing that work, leaving enabled-message conversion and context handling unchanged.

Regression coverage

  • Disabled INFO messages do not call a list item's __str__, both when filtered by the log level and when the underlying logger is explicitly disabled. Both cases fail against the unchanged develop implementation.
  • Enabled messages still convert mixed byte/object lists, host/port tuples and non-printable bytes, and retain their context prefix.

Verification

  • Python 3.14.8: python -m pytest -q — 1,744 passed, 50 skipped, 4,249 subtests passed. Optional/platform-dependent cases remain skipped; existing PyOpenSSL deprecation warnings are reported.
  • Python 3.10.22: python -m pytest tests/test_logging.py -q — 11 passed, 5 subtests passed.
  • pre-commit run --all-files — all hooks passed.
  • Supplemental ruff check --isolated --select E,F asyncssh/logging.py tests/test_logging.py — passed.
  • mypy asyncssh/logging.py --follow-imports=silent — no issues in the changed source module.
  • git diff --check — passed.

This is an AI-assisted contribution, checked against the regression and full local suite above. These changes are contributed under the project's existing EPL-2.0 OR GPL-2.0-or-later license. Contributor credit: Muhammad Osama Amin.

Copilot AI balanced review requested due to automatic review settings November 4, 2026 03:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@codecov

codecov Bot commented Nov 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.86%. Comparing base (fab96ac) to head (ac65ba9).
⚠️ Report is 250 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #841      +/-   ##
===========================================
+ Coverage    99.85%   99.86%   +0.01%     
===========================================
  Files           95      102       +7     
  Lines        27784    32433    +4649     
  Branches      2908     3128     +220     
===========================================
+ Hits         27743    32390    +4647     
- Misses          38       39       +1     
- Partials         3        4       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ronf
ronf merged commit f1d1c75 into ronf:develop Nov 7, 2026
20 checks passed
@ronf

ronf commented Nov 7, 2026

Copy link
Copy Markdown
Owner

Thanks for the PR! This is now merged into "develop" and will be included in the next release.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSHLogger.log() formats its arguments before checking the log level

3 participants