[fix] Remove invalid escape sequence warnings in OpenWrt strings#403
[fix] Remove invalid escape sequence warnings in OpenWrt strings#403yassinekolsi wants to merge 1 commit into
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR removes Python 3.12+ SyntaxWarning: invalid escape sequence warnings by updating OpenWrt upgrader false-positive regex patterns and related test fixture strings, while keeping the false-positive suppression behavior compatible with both escaped and unescaped log formats.
Changes:
- Converted OpenWrt upgrader false-positive regex patterns to raw strings to eliminate invalid escape sequence warnings.
- Adjusted the regex patterns to match both escaped (
\/tmp) and unescaped (/tmp) path variants. - Simplified the OpenWrt upgrader test fixture error strings by removing unnecessary escaped forward slashes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| openwisp_firmware_upgrader/upgraders/openwrt.py | Updates false-positive regex patterns (raw strings + optional escaped slash handling) used to suppress known sysupgrade “false failure” errors. |
| openwisp_firmware_upgrader/tests/test_openwrt_upgrader.py | Updates the mocked sysupgrade failure message fixture to avoid invalid escape warnings by using unescaped paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Command failed: ubus call system sysupgrade " | ||
| '{ "prefix": "\/tmp\/root", ' | ||
| f'"path": "\/tmp\/{filename}", ' | ||
| '"backup": "\/tmp\/sysupgrade.tgz", ' | ||
| '"command": "\/lib\/upgrade\/do_stage2", ' | ||
| '{ "prefix": "/tmp/root", ' | ||
| f'"path": "/tmp/{filename}", ' | ||
| '"backup": "/tmp/sysupgrade.tgz", ' | ||
| '"command": "/lib/upgrade/do_stage2", ' | ||
| '"options": { "save_partitions": 1 } } ' | ||
| "(Connection failed)" |
There was a problem hiding this comment.
The new false-positive suppression regex is intended to match both escaped (e.g. \/tmp) and unescaped (/tmp) path variants, but this test fixture now only exercises the unescaped form. Add a second test case (or parameterize this helper/test) that raises a CommandFailedException message containing literal backslashes before slashes so the escaped-log variant is covered and the new regex behavior is locked in.
nemesifier
left a comment
There was a problem hiding this comment.
Thanks @yassinekolsi, this is a correct cleanup. Converting the _false_positives regex patterns to raw strings removes the Python 3.12 SyntaxWarnings, and using \?/tmp so the patterns match both escaped and plain slashes is a nice touch that keeps the suppression working regardless of how the device emits the path. Simplifying the test fixture to plain slashes is consistent with that.
One small suggestion:
- Since the regex now intentionally handles both escaped and unescaped slashes, consider keeping (or adding) one test case for each form so the
\?optionality is actually exercised. Right now the fixture only covers the plain variant.
Otherwise good to merge.
Summary
This PR removes Python 3.12+ invalid escape sequence SyntaxWarnings in OpenWrt upgrader/test strings.
Changes
Behavior impact
No intended functional behavior change beyond keeping false-positive matching robust across escaped/unescaped log formats.
Validation
Checklist