Skip to content

fix: report actual RSS fetch attempts - #48

Merged
IceCodeNew merged 1 commit into
masterfrom
codex/report-rss-attempt-count
Jul 17, 2026
Merged

fix: report actual RSS fetch attempts#48
IceCodeNew merged 1 commit into
masterfrom
codex/report-rss-attempt-count

Conversation

@IceCodeNew

@IceCodeNew IceCodeNew commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • track the number of RSS requests actually issued
  • report one attempt for non-retryable failures
  • preserve retry and backoff behavior while reporting exhausted retry counts accurately

Verification

  • mise exec -- uv run pytest tests/test_sources.py -q
  • mise exec -- uv run --with pytest --with pytest-cov -- pytest --cov --cov-branch --cov-report=xml (592 passed; 15 missed lines, 7 partial branches)
  • prek run --all-files

Summary by CodeRabbit

  • Bug Fixes
    • Improved RSS error messages to accurately report the number of fetch attempts made before failing.
    • Corrected singular and plural wording in retry failure messages.
  • Tests
    • Expanded coverage for exhausted transport retries and verified retry timing and attempt counts.

@IceCodeNew
IceCodeNew requested a review from Copilot July 16, 2026 18:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 88a9fb4a-79b8-49d9-9a20-fe02efc5fcd3

📥 Commits

Reviewing files that changed from the base of the PR and between 44248a3 and 66d6534.

📒 Files selected for processing (2)
  • tests/test_sources.py
  • weather_briefing/sources.py

📝 Walkthrough

Walkthrough

RSS fetch retries now track actual attempts and report that count in SourceFetchError messages, including correct singular/plural wording. Tests cover retry exhaustion, non-retry HTTP errors, transport failures, attempt counts, and backoff behavior.

Changes

RSS retry reporting

Layer / File(s) Summary
Attempt-accurate retry errors
weather_briefing/sources.py, tests/test_sources.py
The RSS retry loop counts entered attempts and formats failure messages accordingly. Tests verify one-attempt HTTP failures, three-attempt retries, and two-attempt transport-error exhaustion with matching backoff assertions.

Estimated code review effort: 2 (Simple) | ~10 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/report-rss-attempt-count

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.

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.64%. Comparing base (44248a3) to head (66d6534).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #48   +/-   ##
=======================================
  Coverage   99.64%   99.64%           
=======================================
  Files          38       38           
  Lines        6210     6226   +16     
  Branches      341      341           
=======================================
+ Hits         6188     6204   +16     
  Misses         15       15           
  Partials        7        7           

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

@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

1 similar comment
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 66d6534

@IceCodeNew
IceCodeNew marked this pull request as ready for review July 17, 2026 02:33
@IceCodeNew
IceCodeNew merged commit 42b3a55 into master Jul 17, 2026
20 checks passed
@IceCodeNew
IceCodeNew deleted the codex/report-rss-attempt-count branch July 17, 2026 02:33
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix RSS fetch error messaging to report actual attempt count

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Track the number of RSS HTTP requests actually issued during retry loops.
• Report a single attempt for non-retryable failures instead of max_attempts.
• Add regression tests for exhausted transport retries and error message accuracy.
Diagram

graph TD
  A["RSSSource.fetch"] --> B["RSSSource._fetch_with_retry"] --> C["httpx AsyncClient.get"] --> D{"Failure retryable?"}
  D -- "yes & attempts left" --> E["asyncio.sleep(backoff)"] --> B
  D -- "no / exhausted" --> F["SourceFetchError (attempts_made)"]
Loading
High-Level Assessment

The chosen approach (tracking a dedicated attempts_made counter incremented only when a request is actually attempted) is the clearest and most robust way to ensure error messages reflect real request issuance while keeping existing retry/backoff semantics unchanged.

Files changed (2) +27 / -3

Bug fix (1) +4 / -1
sources.pyReport actual RSS request attempts in final retry failure message +4/-1

Report actual RSS request attempts in final retry failure message

• Introduces an attempts_made counter incremented only when an HTTP request is attempted. Uses attempts_made (with proper singular/plural) in the terminal SourceFetchError message instead of always reporting max_attempts.

weather_briefing/sources.py

Tests (1) +23 / -2
test_sources.pyTighten RSS retry tests and add transport-attempt exhaustion coverage +23/-2

Tighten RSS retry tests and add transport-attempt exhaustion coverage

• Updates assertions to verify SourceFetchError messages include the correct attempt count (including singular '1 attempt'). Adds a new test ensuring transport-level failures (e.g., ConnectError) report exhausted attempts accurately and still back off the expected number of times.

tests/test_sources.py

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 66d6534

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants