Skip to content

Fix: Don't report dynamix.cfg as corrupt if 0 byte file#2386

Merged
limetech merged 2 commits into
unraid:masterfrom
Squidly271:patch-6
Sep 19, 2025
Merged

Fix: Don't report dynamix.cfg as corrupt if 0 byte file#2386
limetech merged 2 commits into
unraid:masterfrom
Squidly271:patch-6

Conversation

@Squidly271

@Squidly271 Squidly271 commented Sep 19, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Prevents false “missing/invalid” plugin configuration alerts by tolerating temporarily empty plugin config files during processing.
    • Reduces unnecessary delays and intermittent pauses during plugin checks, leading to a smoother experience during system startup and plugin install/update cycles.
    • Improves reliability of plugin status reporting, minimizing spurious warnings and retries when plugin configurations are in transient states.

@Squidly271

Copy link
Copy Markdown
Contributor Author

@coderabbitai review me

@github-actions

github-actions Bot commented Sep 19, 2025

Copy link
Copy Markdown

🔧 PR Test Plugin Available

A test plugin has been generated for this PR that includes the modified files.

Version: 2025.09.19.1740
Build: View Workflow Run

📥 Installation Instructions:

Install via Unraid Web UI:

  1. Go to Plugins → Install Plugin
  2. Copy and paste this URL:
https://preview.dl.unraid.net/pr-plugins/pr-2386/webgui-pr-2386.plg
  1. Click Install

Alternative: Direct Download

⚠️ Important Notes:

  • Testing only: This plugin is for testing PR changes
  • Backup included: Original files are automatically backed up
  • Easy removal: Files are restored when plugin is removed
  • Conflicts: Remove this plugin before installing production updates

📝 Modified Files:

Click to expand file list
emhttp/plugins/dynamix/include/Report.php

🔄 To Remove:

Navigate to Plugins → Installed Plugins and remove webgui-pr-2025.09.19.1740, or run:

plugin remove webgui-pr-2025.09.19.1740

🤖 This comment is automatically generated and will be updated with each new push to this PR.

@Squidly271

Copy link
Copy Markdown
Contributor Author

@coderabbitai Are you alive or just giving me the silent treatment?

@ljm42 ljm42 added the 7.2 label Sep 19, 2025
@elibosley

Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 19, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Sep 19, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Adds a conditional guard in the plugin config validation loop to skip zero-length, non-required plugin config files, altering the inner control flow to continue rather than flagging them as missing/invalid. Other existence, parsing, retry, and sleep behaviors remain unchanged.

Changes

Cohort / File(s) Summary
Plugin config check logic
emhttp/plugins/dynamix/include/Report.php
Inserted a guard: when need=false, plugin=true, and file size == 0, continue the loop. Preserves existing checks (existence, INI parsing), retry count, break/sleep behavior; only adjusts inner loop flow to tolerate empty plugin config files.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant R as Report.php
  participant FS as File System
  participant INI as INI Parser
  participant T as Timer/Sleep

  rect rgb(235, 245, 255)
    Note over R: For each plugin config (inner loop with limited retries)
    R->>FS: Check file exists
    alt File missing
      R-->>R: Mark as missing (existing logic)
      R->>T: sleep()
      R-->>R: Next retry / break (existing logic)
    else File exists
      R->>FS: Stat file size
      alt New guard (not required AND plugin AND size==0)
        Note right of R: New behavior: continue without marking invalid
        R-->>R: continue (retry loop proceeds)
      else Non-empty or required
        R->>INI: Parse INI
        alt Parse ok
          R-->>R: proceed as valid (existing logic)
        else Parse fail
          R-->>R: mark invalid (existing logic)
          R->>T: sleep()
          R-->>R: Next retry / break (existing logic)
        end
      end
    end
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nose through configs, hop-hop, inspect—
A zero-byte burrow? I pause, deflect.
Not needed, I skip, with whiskered grace,
Then parse the rest at a steady pace.
Two hops to retry, no fright, no fog—
A tidy warren for this diligent log. 🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately describes the primary change: preventing dynamix.cfg from being reported as corrupt when it is a 0-byte file, which aligns with the Report.php change that skips empty plugin config files; it is specific, concise, and relevant for scanning PR history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
emhttp/plugins/dynamix/include/Report.php (1)

24-26: Initialize $flag per file to avoid future undefined-variable paths

Defensive init clarifies intent and prevents undefined $flag if new continue paths are added later.

   [$name,$need,$plugin] = explode(':',$file);
   $filename = $plugin ? "$config/plugins/$name/$name.cfg" : "$config/$name.cfg";
+  $flag = 0;
   for ( $i=0;$i<2;$i++) {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80a7550 and c0cc157.

📒 Files selected for processing (1)
  • emhttp/plugins/dynamix/include/Report.php (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.

Comment thread emhttp/plugins/dynamix/include/Report.php Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@limetech limetech merged commit 7b6bf0c into unraid:master Sep 19, 2025
1 check passed
@Squidly271 Squidly271 deleted the patch-6 branch November 5, 2025 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants