fix(api): redirect benign pnpm postinstall warning to log file #1290
Conversation
WalkthroughA new function, Changes
Sequence Diagram(s)sequenceDiagram
participant API as pnpm_install_unraid_api()
participant Runner as run_pnpm_install()
participant Cmd as pnpm install Command
participant Log as Build Scripts Log (/var/log/unraid-api/build-scripts.log)
API->>Runner: Invoke run_pnpm_install()
Runner->>Cmd: Execute "pnpm install --production --offline"
Cmd-->>Runner: Return results & warnings
Runner->>Log: Append warning messages
Runner-->>API: Return final result
Poem
📜 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)
🪧 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 (
|
This reverts commit da342af.
|
since there isn't a native way to customize the pnpm reporter or suppress the 'ignored build scripts' warning message, i had chatgpt come up with a bash solution in the format of: stdbuf -oL pnpm install --prod --ignore-workspace --store-dir=../.pnpm-store 2>&1 | sed $'/^╭ Warning/,/^╰/c\\\nSuppressed third-party scripts'but we decided not to add it bc this is only a nice-to-have. adding a log message instead |
|
Wait @pujitm didn't the build break on unraid anyway, any reason to add the deps in that case ? |
![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 (2)
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api (2)
15-15: Style improvement: removing unnecessary semicolonGood cleanup of syntax by removing the unnecessary semicolon after
true. In Bash, semicolons at the end of a line are optional when it's the last statement on the line.
18-18: Style improvement: removing unnecessary semicolonGood cleanup of syntax by removing the unnecessary semicolon after
true. This matches the change in line 15 and improves consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api (2)
105-121: New function looks good with helpful logging for build scriptsThe new
run_pnpm_install()function effectively handles the pnpm install process while properly capturing and logging build script warnings. The implementation:
- Uses
stdbuf -oLto ensure line-buffered output- Creates log directories as needed
- Provides clear messaging about ignored build scripts
This addresses the PR objective of handling the warning message related to 'ignored build scripts' by logging them appropriately.
One suggestion to consider: The current implementation overwrites the log file each time. You might want to append the initial note rather than overwrite it to preserve historical logs:
- echo "Note: The warning above is expected. Build scripts are intentionally ignored for security and performance reasons." > "$log_file" + echo "Note: The warning above is expected. Build scripts are intentionally ignored for security and performance reasons." >> "$log_file"
144-144: Good refactoring to use the new functionThe code now properly calls the new
run_pnpm_installfunction instead of directly running pnpm install. This maintains the same functionality while adding the enhanced logging capabilities.
pujitm
force-pushed
the
chore/install-warning
branch
from
0471f34 to
38239ac
Compare
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
pujitm
changed the title
🤖 I have created a release *beep* *boop* --- ## [4.5.0](v4.4.1...v4.5.0) (2025-04-02) ### Features * add webgui theme switcher component ([#1304](#1304)) ([e2d00dc](e2d00dc)) * api plugin system & offline versioned dependency vendoring ([#1252](#1252)) ([9f492bf](9f492bf)) * **api:** add `unraid-api --delete` command ([#1289](#1289)) ([2f09445](2f09445)) * basic array controls ([#1291](#1291)) ([61fe696](61fe696)) * basic docker controls ([#1292](#1292)) ([12eddf8](12eddf8)) * copy to webgui repo script docs + wc build options ([#1285](#1285)) ([e54f189](e54f189)) ### Bug Fixes * additional url fixes ([4b2763c](4b2763c)) * **api:** redirect benign pnpm postinstall warning to log file ([#1290](#1290)) ([7fb7849](7fb7849)) * **deps:** update dependency chalk to v5 ([#1296](#1296)) ([6bed638](6bed638)) * **deps:** update dependency diff to v7 ([#1297](#1297)) ([3c6683c](3c6683c)) * disable all config watchers ([#1306](#1306)) ([5c1b435](5c1b435)) * extract callbacks to library ([#1280](#1280)) ([2266139](2266139)) * OEM plugin issues ([#1288](#1288)) ([d5a3d0d](d5a3d0d)) * replace files lost during pruning ([d0d2ff6](d0d2ff6)) --- 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>
Summary by CodeRabbit
New Features
Refactor