fix(plugin): flaky masking of benign warning during pnpm install #1313
Conversation
WalkthroughThe changes refactor the Changes
Sequence Diagram(s)sequenceDiagram
participant API as API Script
participant PNPM as pnpm install
participant SED as sed Processor
participant Log as Log File
API->>PNPM: Execute pnpm install --reporter=append-only
PNPM->>SED: Stream output
SED->>Log: Write warning messages
API->>Log: Append completion message after sed finishes
Possibly related PRs
Suggested reviewers
Poem
🪧 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)
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api (1)
111-112: Excellent refactoring that addresses the flaky warning issue.This change streamlines the
run_pnpm_installfunction by usingsedto efficiently filter and redirect pnpm warnings. The approach is more reliable than the previous line-by-line processing, which was likely the source of the flakiness mentioned in the PR title.One potential issue to consider: the code assumes the
/var/log/unraid-api/directory exists. If this directory doesn't exist when the function runs, thesedcommand might fail.Consider adding a directory check to ensure the log directory exists:
run_pnpm_install() { local log_file="/var/log/unraid-api/build-scripts.log" + mkdir -p "$(dirname "$log_file")" stdbuf -oL pnpm install --prod --prefer-offline --reporter=append-only 2>&1 | sed -e "/^╭ Warning/,/^╰/w $log_file" -e "/^╭ Warning/,/^╰/c\Build scripts completed. See $log_file for details." echo "Note: This warning is expected. Build scripts are intentionally ignored for security and performance reasons." >> "$log_file" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Cloudflare Pages
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.6.0](v4.5.0...v4.6.0) (2025-04-03) ### Features * add gui settings field for sso users ([#1310](#1310)) ([5ba3fa6](5ba3fa6)) ### Bug Fixes * build ([ed67af9](ed67af9)) * **plugin:** flaky masking of benign warning during pnpm install ([#1313](#1313)) ([1f10b63](1f10b63)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
The masking logic has a different effect in TTY and non-TTY environments bc pnpm will change the
installreporter. Now, we:--reporter=append-onlyfor consistencysedto mask and redirect the benign "Ignored build scripts" warningSummary by CodeRabbit