Implement LZMA support check in platform.py #270
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces an import-time check in platform.py to detect Python’s lzma module. If unavailable, the module prints error messages and terminates the process with exit code 1 before proceeding with other imports. No other logic or public APIs are changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Importer as Importer
participant Platform as platform.py
participant OS as OS/Runtime
Importer->>Platform: import platform
rect rgba(220,235,245,0.6)
note right of Platform: New: LZMA availability check
Platform->>Platform: try import lzma
alt lzma available
Platform->>Platform: proceed with remaining imports
Platform-->>Importer: module ready
else lzma missing
Platform->>OS: print error messages
Platform->>OS: exit(1)
OS-->>Importer: process terminates
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
platform.py (3)
15-23: Fail-fast check is good; send errors to stderr, avoid F401, and use SystemExit.Printing to stdout at import time is noisy for tooling, and Ruff flags
lzmaas unused. Importing and deleting the symbol both proves availability and satisfies linters. Also prefer raisingSystemExitand slightly tighten messaging.Apply this diff:
# LZMA support check -try: - import lzma -except ImportError: - print("ERROR: LZMA module is not available in this Python installation.") - print("LZMA support is required for tool / toolchain installation.") - print("Please install Python with LZMA support.") - import sys - sys.exit(1) +try: + import lzma as _lzma +except ImportError: + import sys + print("ERROR: Python's lzma module is unavailable or broken in this interpreter.", file=sys.stderr) + print("LZMA (liblzma) support is required for tool/toolchain installation.", file=sys.stderr) + print("Please install Python built with LZMA support.", file=sys.stderr) + raise SystemExit(1) +else: + # Keep namespace clean and satisfy linters + del _lzma
15-23: Consider avoiding import-time hard exit if PlatformIO needs to import this module in contexts that don't install tools.Exiting during import can break benign commands (e.g., board listing, introspection) on systems lacking LZMA. If acceptable, keep as-is. Otherwise, gate the check behind the first path that actually needs LZMA (e.g., before installing/extracting tools) or raise a dedicated exception to be surfaced by the caller.
Would you like me to refactor this into a callable guard (e.g.,
ensure_lzma_available()), invoked right before any operation that downloads/extracts packages?
15-23: Deduplicatesysimports.You import
sysinside theexceptand again at Line 31. Safe but redundant. Either:
- Keep the inner
exceptimport and remove the globalimport syson Line 31, or- Import
sysonce above the check and reuse it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
platform.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
platform.py
17-17: lzma imported but unused; consider using importlib.util.find_spec to test for availability
(F401)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: build (macos-15, examples/espidf-coap-server)
- GitHub Check: build (macos-15, examples/arduino-matter-light)
- GitHub Check: build (macos-15, examples/arduino-wifiscan)
- GitHub Check: build (windows-latest, examples/espidf-peripherals-usb)
- GitHub Check: build (windows-latest, examples/espidf-exceptions)
- GitHub Check: build (windows-latest, examples/espidf-http-request)
- GitHub Check: build (windows-latest, examples/espidf-storage-sdcard)
- GitHub Check: build (windows-latest, examples/arduino-matter-light)
- GitHub Check: build (windows-latest, examples/espidf-arduino-littlefs)
- GitHub Check: build (windows-latest, examples/espidf-blink)
- GitHub Check: build (windows-latest, examples/tasmota)
- GitHub Check: build (ubuntu-latest, examples/espidf-blink)
- GitHub Check: build (ubuntu-latest, examples/tasmota)
- GitHub Check: build (ubuntu-latest, examples/espidf-ulp-lp)
- GitHub Check: build (windows-latest, examples/arduino-NimBLE-ext_client)
- GitHub Check: build (windows-latest, examples/arduino-blink)
- GitHub Check: build (windows-latest, examples/arduino-rmt-blink)
- GitHub Check: build (ubuntu-latest, examples/espidf-arduino-matter-light)
- GitHub Check: build (ubuntu-latest, examples/arduino-blink)
- GitHub Check: build (ubuntu-latest, examples/arduino-wifiscan)
Updated error messages for LZMA module import failure.
needed since there are defect Python distributions out there.
Checklist:
Summary by CodeRabbit