fix: correctly retry device read when device is unstable

[richm]

Cause: The retry loop for reading from a device would bail out of the loop
before retrying because the loop should have used a greater-than-or-equal
to test the retry condition rather than a less-than.

Consequence: If the device is not stable, the loop would bail and issue
an exception rather than trying to retry the read.

Fix: Use the correct loop condition.

Result: The network role can correctly retry the read when the device
is unstable.

Signed-off-by: Rich Megginson rmeggins@redhat.com

Summary by CodeRabbit

• Bug Fixes

  • Improved reliability of network interface reads so the system stops retrying at the intended limit and surfaces errors predictably when results remain unstable.

• Tests

  • Added unit tests to verify retry behavior: convergence to stable results and proper failure after the maximum attempts.
  • Added test setup to clear cached link-info state between tests to prevent cross-test interference.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR fixes an off-by-one error in SysUtil.link_infos() retry logic by changing the exception-path check to try_count >= 50, and adds TestSysUtils.setUp plus two tests validating retry-success and retry-failure behaviors.

Changes

Retry-limit fix and test coverage

Layer / File(s)	Summary
Retry-limit boundary condition fix library/network_connections.py	The exception handler's retry-limit condition is updated to try_count >= 50 instead of try_count < 50, correcting the off-by-one logic for when the function stops retrying and raises an error.
Test setup and retry semantics coverage tests/unit/test_network_connections.py	TestSysUtils.setUp() clears the cached _link_infos before each test. New tests verify that unstable-then-stable fetch results return the final stable mapping with expected fetch counts, and that perpetually unstable results raise an exception and exercise the 51-call retry path.

🚥 Pre-merge checks | ✅ 3 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description Format	⚠️ Warning	PR description uses "Cause:", "Consequence:", "Fix:" headers instead of required "Enhancement:"/"Feature:" and "Reason:" sections from template.	Rewrite description to follow template: "Enhancement: [what changed]", "Reason: [why needed]", "Result: [outcome]"
Description check	❓ Inconclusive	The pull request description covers the cause, consequence, fix, and result. However, it does not follow the required template structure with the specified section headings (Enhancement, Reason, Result, Issue Tracker Tickets).	Reorganize the description to match the required template format with section headings: Enhancement, Reason, Result, and Issue Tracker Tickets sections.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title follows Conventional Commits format with type 'fix' and clearly describes the main change: correcting the retry condition for device reads.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.19%. Comparing base (1b57520) to head (7f7acc1).
⚠️ Report is 100 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #874      +/-   ##
==========================================
+ Coverage   43.11%   43.19%   +0.07%     
==========================================
  Files          12       13       +1     
  Lines        3124     3172      +48     
==========================================
+ Hits         1347     1370      +23     
- Misses       1777     1802      +25     

☔ 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.

[richm]

[citest]

[coderabbitai]

🧹 Nitpick comments (1)

tests/unit/test_network_connections.py (1)

5650-5655: ⚡ Quick win

Use assertRaisesRegex instead of catching broad exceptions

At Line 5652, except Exception makes this test pass on unrelated failures. Prefer asserting the expected exception message directly.

Proposed change

         fetch_mock = mock.Mock(side_effect=unstable_fetch)
         with mock.patch.object(SysUtil, "_link_infos_fetch", fetch_mock):
-            try:
-                SysUtil.link_infos(refresh=True)
-            except Exception as exc:
-                self.assertIn("stable link-infos", str(exc))
-            else:
-                self.fail("link_infos did not raise after max retries")
+            with self.assertRaisesRegex(Exception, "stable link-infos"):
+                SysUtil.link_infos(refresh=True)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_network_connections.py` around lines 5650 - 5655, Replace the
broad try/except pattern around SysUtil.link_infos(refresh=True) with unittest's
assertRaisesRegex to specifically assert the expected error and message; use
self.assertRaisesRegex(<ExceptionType>, "stable link-infos") as cm: and call
SysUtil.link_infos(refresh=True) inside that context so the test fails for
unrelated exceptions and verifies the error text from SysUtil.link_infos.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/unit/test_network_connections.py`:
- Around line 5650-5655: Replace the broad try/except pattern around
SysUtil.link_infos(refresh=True) with unittest's assertRaisesRegex to
specifically assert the expected error and message; use
self.assertRaisesRegex(<ExceptionType>, "stable link-infos") as cm: and call
SysUtil.link_infos(refresh=True) inside that context so the test fails for
unrelated exceptions and verifies the error text from SysUtil.link_infos.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bbd8a414-f68e-4a85-b3ec-232d02eb9eb9

📥 Commits

Reviewing files that changed from the base of the PR and between 8b81fbb and d1fabd3.

📒 Files selected for processing (2)

• library/network_connections.py
• tests/unit/test_network_connections.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/test_network_connections.py`:
- Around line 5650-5660: Replace the broad try/except block that calls
SysUtil.link_infos(refresh=True) with an assert that the call raises the
expected error and message using self.assertRaisesRegex; specifically, use
self.assertRaisesRegex(Exception, r"stable link-infos") as ctx:
SysUtil.link_infos(refresh=True) so the test asserts the exact exception message
rather than catching all Exception subclasses and masking unrelated errors.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e4d8a2a1-3b02-44e0-9a93-a0c73db4a05f

📥 Commits

Reviewing files that changed from the base of the PR and between d1fabd3 and 6fbb721.

📒 Files selected for processing (2)

• library/network_connections.py
• tests/unit/test_network_connections.py

[richm]

[citest]

[richm]

[citest]

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.19%. Comparing base (1b57520) to head (7f7acc1).
⚠️ Report is 100 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #874      +/-   ##
==========================================
+ Coverage   43.11%   43.19%   +0.07%     
==========================================
  Files          12       13       +1     
  Lines        3124     3172      +48     
==========================================
+ Hits         1347     1370      +23     
- Misses       1777     1802      +25     

☔ 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.