Skip to content

NC | Fixing two issues in entropy.utils#9279

Merged
jackyalbo merged 1 commit into
noobaa:masterfrom
jackyalbo:jacky-nc-fixes2
Nov 12, 2025
Merged

NC | Fixing two issues in entropy.utils#9279
jackyalbo merged 1 commit into
noobaa:masterfrom
jackyalbo:jacky-nc-fixes2

Conversation

@jackyalbo

@jackyalbo jackyalbo commented Nov 12, 2025

Copy link
Copy Markdown
Contributor

Describe the Problem

Fixing 2 issues in entropy utils

Explain the Changes

Issues: Fixed #xxx / Gap #xxx

Testing Instructions:

  • Doc added/updated
  • Tests added

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed entropy generation termination logic to exit properly
    • Corrected device path format in disk selection whitelist for consistent device matching

Signed-off-by: jackyalbo <jacky.albo@gmail.com>
@coderabbitai

coderabbitai Bot commented Nov 12, 2025

Copy link
Copy Markdown

Walkthrough

This change corrects two issues in entropy utilities: inverts the loop termination condition in generate_entropy to break when the condition becomes false instead of true, and fixes a missing leading slash in the disk selection whitelist entry for DASD devices.

Changes

Cohort / File(s) Summary
Entropy Utilities Fixes
src/util/entropy_utils.js
Loop termination condition inverted in generate_entropy function to break when condition is falsy instead of truthy; disk selection whitelist entry corrected by adding missing leading slash to /dev/dasd device path

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Loop condition inversion: Verify that inverting the break condition from truthy to falsy produces the intended control flow and doesn't introduce unintended early or delayed loop exits.
  • Whitelist entry correction: Confirm the leading slash addition aligns with the intended device path format and existing whitelist patterns.

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 'NC | Fixing two issues in entropy.utils' directly corresponds to the changeset which fixes two issues in entropy_utils.js, making it clear and specific.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36c21ce and 7a79ac9.

📒 Files selected for processing (1)
  • src/util/entropy_utils.js (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Noobaa Image
  • GitHub Check: run-package-lock-validation
  • GitHub Check: run-jest-unit-tests
🔇 Additional comments (2)
src/util/entropy_utils.js (2)

31-31: LGTM! Critical logic error fixed.

The inverted condition is now correct—breaking when loop_cond() becomes false aligns with the documented behavior ("run as long as the callback is true") and prevents unnecessary disk I/O after the caller has finished initializing.


101-101: LGTM! Typo fixed for DASD device matching.

The missing leading slash would have prevented DASD devices from matching since disk names are constructed with /dev/ prefix (line 76) and matched using startsWith() (line 107). This fix ensures consistency with the other device prefixes.


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.

@shirady shirady 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.

LGTM

@jackyalbo jackyalbo merged commit ed6b491 into noobaa:master Nov 12, 2025
18 checks passed
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