Skip to content

fix wrongly assigned 404 bug - #276

Merged
LizzAlice merged 1 commit into
mainfrom
fix_wrong_404_bug
Sep 10, 2026
Merged

LizzAlice merged 1 commit into
mainfrom
fix_wrong_404_bug

Conversation

@LizzAlice

@LizzAlice LizzAlice commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

zbMath gives a 404 if an id gets requested that does not exist, which leads to a bug

Summary by CodeRabbit

  • Bug Fixes
    • Improved zbMATH data imports by recognizing a successful “no results” response as the normal end of a collection.
    • Other not-found responses now fail immediately instead of being retried unnecessarily.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 9f1e670a-2fd2-4653-bbb9-3958270ce2ef

📥 Commits

Reviewing files that changed from the base of the PR and between 0b4cf18 and 675b97f.

📒 Files selected for processing (1)
  • src/mardi_importer/zbmath/ZBMathSource.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The zbMATH pagination loop now treats a specific 404 response as the normal end of the collection. Other 404 responses raise a RuntimeError without retries.

Changes

zbMATH pagination

Layer / File(s) Summary
Handle 404 pagination responses
src/mardi_importer/zbmath/ZBMathSource.py
write_data_dump stops pagination when the response contains internal_code "successful access, but no result". Other 404 responses raise a RuntimeError immediately.

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

Suggested reviewers: eloiferrer

Merge Risk: ⚪ Minimal · up to 675b9

Expected end-of-collection responses now finish pagination without retries, while unexpected 404 responses fail immediately; no concrete current-head merge risk is identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title identifies the 404 bug addressed by the pull request. It is related to the main change, but its wording is imprecise.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
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.
✨ 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 fix_wrong_404_bug

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.

@LizzAlice
LizzAlice merged commit 17ea461 into main Sep 10, 2026
2 checks passed
@LizzAlice
LizzAlice deleted the fix_wrong_404_bug branch September 10, 2026 12:28
Sign up for free to 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.

1 participant