Make sure the oldest boot ID is included in the boot list #5591
Conversation
📝 WalkthroughWalkthroughThe changes in the Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (1)
supervisor/host/logs.py (1)
129-135: Consider adding explicit error handling for JSON parsing.While the implementation correctly processes and deduplicates boot IDs, it could benefit from explicit error handling for JSON parsing to gracefully handle malformed entries.
Consider applying this diff:
self._boot_ids = [] for entry in text.split("\n"): - if ( - entry - and (boot_id := json.loads(entry)[PARAM_BOOT_ID]) not in self._boot_ids - ): - self._boot_ids.append(boot_id) + if not entry: + continue + try: + data = json.loads(entry) + if (boot_id := data.get(PARAM_BOOT_ID)) and boot_id not in self._boot_ids: + self._boot_ids.append(boot_id) + except json.JSONDecodeError as err: + _LOGGER.warning("Failed to parse journal entry: %s", err)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supervisor/host/logs.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build i386 supervisor
- GitHub Check: Build armv7 supervisor
- GitHub Check: Build armhf supervisor
- GitHub Check: Build aarch64 supervisor
- GitHub Check: Run tests Python 3.13.1
🔇 Additional comments (1)
supervisor/host/logs.py (1)
114-127: LGTM! Consider enhancing the comments and verifying the range header format.The implementation effectively ensures the oldest boot ID is included. Two minor suggestions:
- Consider expanding the comment to explain the range header format.
- Consider adding error handling for malformed JSON responses.
Let's verify the range header format is correct for systemd-journal-gatewayd:
✅ Verification successful
Range header format is correct and consistent with test cases
The format
entries=:0:1follows the established patternentries=:position:countused throughout the codebase, correctly requesting the oldest entry.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the range header format for systemd-journal-gatewayd # Expected: Documentation or examples showing the correct format for the Range header # Search for range header documentation in the systemd source code rg -A 5 -B 5 "entries=" --glob "*.{c,h,md,html}" # Search for examples of range header usage rg -A 5 "Range.*entries" --glob "*.{py,c,h}"Length of output: 879
|
The test failure is unrelated to this PR. |
sairon
force-pushed
the
host-logs-add-oldest-boot-id
branch
from
1ddad57 to
1dd90e6
Compare
If the system is running for a long time, or the logging is particularly chatty, the Systemd journal message we use to detect boot will be rotated out of the journal. Currently we only handled it if there was one boot, but we usually always missed the oldest boot if there were more boots. Adjust the method for getting boot IDs to always get the very first log line in the journal instead of the last one, and make sure its boot ID is included in the list.
Proposed change
If the system is running for a long time, or the logging is particularly chatty, the Systemd journal message we use to detect boot will be rotated out of the journal. Currently we only handled it if there was one boot, but we usually always missed the oldest boot if there were more boots.
Adjust the method for getting boot IDs to always get the very first log line in the journal instead of the last one, and make sure its boot ID is included in the list.
Current list of boots from Systemd:
Before:
After:
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit