fix: close leaked DNS-probe socket, silence deprecation warning noise - #422
Conversation
is_connected_dns() opened a raw socket for the reachability probe and never closed it, leaking a file descriptor on every check (and on every failed probe, which is the common case on a flaky network). It's now closed in a finally block with the return semantics unchanged. The rest of the ~90 warnings were tests deliberately covering deprecated shims (standardize_lang_tag/get_language_dir, fakebus.Message, EventSchedulerInterface, the dialog module, expand_template) plus a couple of unrelated stdlib/test-fixture ResourceWarnings — now filtered per-test with specific message patterns, coverage kept. One warning remains, sourced from the still-installed (unpatched) ovos-bus-client dependency's own session.py, not from this repo; it clears once that fix ships. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
A new update is available for your PR checks! 📥I've aggregated the results of the automated checks for this PR below. 🏷️ Release PreviewThe release candidate is looking strong. 💪 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
🔍 LintI've got some results for you! 📝 ❌ ruff: issues found — see job log ⚖️ License CheckEnsuring our project remains legally compliant. ✅ ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 📋 Repo HealthEnsuring the codebase isn't suffering from 'technical debt' flu. 🤒 ✅ All required files present. Latest Version: ✅ 🔒 Security (pip-audit)Ensuring our encryption is top-notch. 🔐 ✅ No known vulnerabilities found (47 packages scanned). 📊 CoverageHow much of the logic is under the microscope? 🔬 ✅ 85.3% total coverage Files below 80% coverage (5 files)
Full report: download the 🔨 Build TestsThe compiler has spoken! Here is the verdict. 📜 ✅ All versions pass
Keeping the code clean, one PR at a time ✨ |
is_connected_dns() opened a raw socket to check reachability and never closed it, leaking a file descriptor on every check — including every failed probe, which is the common case on a flaky network. It's now closed in a finally block, with the return semantics unchanged.
The rest of the roughly 90 warnings were tests deliberately covering deprecated shims — standardize_lang_tag/get_language_dir, ovos_utils.fakebus.Message, EventSchedulerInterface, the dialog module, expand_template — plus a couple of unrelated stdlib/test-fixture ResourceWarnings from unclosed files in the dialog test fixtures. These are now silenced with per-test/class/module pytest.mark.filterwarnings using the specific message text, so the deprecated code paths stay covered but the summary is clean.
Full suite with -W default: ~90 warnings before, 1 after. The one remaining warning is emitted by the currently-installed (unpatched) ovos-bus-client dependency's own session.py at import time, not by anything in this repo — it will clear on its own once that fix (a companion PR already up in ovos-bus-client) is released. 949 passed, 1 skipped, unchanged.