Skip to content

fix(plugin): add fallback for unraid-api stop in deprecation cleanup - #1668

Merged
Pujit Mehrotra (pujitm) merged 1 commit into
mainfrom
fix/min-version
Sep 8, 2025
Merged

fix(plugin): add fallback for unraid-api stop in deprecation cleanup#1668
Pujit Mehrotra (pujitm) merged 1 commit into
mainfrom
fix/min-version

Conversation

@pujitm

@pujitmPujit Mehrotra (pujitm) commented Sep 8, 2025

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability of service shutdown during account disconnect/cleanup, handling environments where the service script or CLI may be missing.
    • Removed unnecessary delays during shutdown, resulting in a faster and smoother disconnect process.
    • Added clearer warning messaging when the service cannot be found.
  • Chores

    • Minor whitespace cleanup.

@coderabbitai

coderabbitaiBot commented Sep 8, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Reworked unraid-api stop logic in plugin and cleanup script to branch between rc script, CLI, or warning, removing output-capture, fixed delay, and status echoes. Surrounding conditions and subsequent cleanup/sign-out logic remain unchanged. Minor whitespace adjustments.

Changes

Cohort / File(s)Summary
Stop logic refactor
plugin/plugins/dynamix.unraid.net.plg, plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/cleanup.sh
Replace output-capturing stop with branching: prefer /etc/rc.d/rc.unraid-api stop, else unraid-api stop, else warn; remove --delete, sleeps, and echo of captured output; preserve existing guards and follow-up cleanup flow.

Sequence Diagram(s)

sequenceDiagram
autonumber
participant Cleanup as Cleanup Caller
participant Shell as Shell Script
participant RC as /etc/rc.d/rc.unraid-api
participant CLI as unraid-api (CLI)
Cleanup->>Shell: perform_connect_cleanup / cleanup.sh
alt rc.unraid-api exists
Shell->>RC: stop 2>&1
RC-->>Shell: exit status
else unraid-api CLI available
Shell->>CLI: stop 2>&1
CLI-->>Shell: exit status
else neither available
Shell-->>Cleanup: warn "Neither rc.unraid-api nor unraid-api found"
end
note right of Shell: Removed output-capture and fixed sleep
Shell-->>Cleanup: proceed to sign-out / subsequent steps (unchanged)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paws—no sleeps, no wait,
Two paths to stop, or warn the state.
RC? CLI? I’ll check and hop—
If none are found, I’ll gently stop.
Burrow tidy, scripts aligned—
Carrot logs: all reassigned. 🥕🐇


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0cffbd and 4758049.

📒 Files selected for processing (2)
  • plugin/plugins/dynamix.unraid.net.plg (2 hunks)
  • plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/cleanup.sh (1 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/min-version

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude

claudeBot commented Sep 8, 2025

Copy link
Copy Markdown

Claude finished Pujit Mehrotra (@pujitm)'s task —— View job


Code Review Analysis

Analyzing the diff/changeset for critical issues in the changed lines only.

Todo List:

  • Read the modified plugin files to understand the changes
  • Analyze only the diff/changeset for critical issues
  • Provide review feedback focusing on high-priority issues

No critical issues found in changes

The PR replaces a single command execution pattern with a more robust fallback approach for stopping the unraid-api service. The changes:

  1. Replace output=$(/etc/rc.d/rc.unraid-api stop --delete 2>&1) with proper file existence checks
  2. Add fallback to unraid-api stop command if the rc script doesn't exist
  3. Add warning message when neither method is available
  4. Remove the output capture and echo logic that depended on empty output

The changes are defensive programming improvements that handle missing files gracefully and don't introduce any critical bugs, security issues, breaking changes, or data loss risks.


@pujitm
Pujit Mehrotra (pujitm) merged commit 797bf50 into mainSep 8, 2025
10 of 11 checks passed
@pujitm
Pujit Mehrotra (pujitm) deleted the fix/min-version branch September 8, 2025 14:06
@github-actionsgithub-actionsBot mentioned this pull request Sep 8, 2025
@codecov

codecovBot commented Sep 8, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.56%. Comparing base (f0cffbd) to head (4758049).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #1668 +/- ##
=======================================
Coverage 54.56% 54.56% =======================================
Files 809 809 Lines 45457 45457 Branches 4675 4675 =======================================
Hits 24804 24804 Misses 20616 20616 Partials 37 37 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR1668/dynamix.unraid.net.plg

Eli Bosley (elibosley) pushed a commit that referenced this pull request Sep 8, 2025
🤖 I have created a release *beep* *boop*
---
## [4.20.0](v4.19.1...v4.20.0)
(2025-09-08)
### Features
* **disks:** add isSpinning field to Disk type
([#1527](#1527))
([193be3d](193be3d))
### Bug Fixes
* better component loading to prevent per-page strange behavior
([095c222](095c222))
* **deps:** pin dependencies
([#1669](#1669))
([413db4b](413db4b))
* **plugin:** add fallback for unraid-api stop in deprecation cleanup
([#1668](#1668))
([797bf50](797bf50))
* prepend 'v' to API version in workflow dispatch inputs
([f0cffbd](f0cffbd))
* progress frame background color fix
([#1672](#1672))
([785f1f5](785f1f5))
* properly override header values
([#1673](#1673))
([aecf70f](aecf70f))
---
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>
@github-actionsgithub-actionsBot mentioned this pull request Oct 27, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@pujitm@elibosley