Skip to content

Download the build named by the metadata - #192

Open
oschwald wants to merge 1 commit into
mainfrom
greg/download-by-build-epoch
Open

oschwald wants to merge 1 commit into
mainfrom
greg/download-by-build-epoch

Conversation

@oschwald

@oschwald oschwald commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

pygeoipupdate now identifies the database to download by its build epoch rather than its date. The download request sends the build_epoch from the metadata response. A request by date returns the latest build of that day, so a new build published during an update could cause a checksum mismatch.

The date request is gone. If the metadata has no build_epoch, the update fails with a DownloadError. This needs STF-295 deployed first. Today production does not send build_epoch.

This ports maxmind/geoipupdate#566. That PR also fixes a response-body leak in the Go client. The Python client reads each response inside async with, so it does not have that leak.

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings October 15, 2026 22:22

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 Oct 15, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: c8196046-0ee4-4b8a-be21-0ea63d0cb0e0
📥 Commits

Reviewing files that changed from the base of the PR and between e832a3b and 3d25812.

📒 Files selected for processing: 6
  • HISTORY.rst
  • src/pygeoipupdate/client.py
  • src/pygeoipupdate/models.py
  • tests/test_cli.py
  • tests/test_client.py
  • tests/test_updater.py

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

📝 Walkthrough

Metadata now identifies a database build with build_epoch, and downloads use that value instead of a date. Tests and the 0.2.0 changelog reflect the updated metadata and request parameter.

Changes: Build-Epoch Database Downloads

Layer / File(s) Summary
Metadata build identity
src/pygeoipupdate/models.py, src/pygeoipupdate/client.py, tests/test_client.py
Metadata replaces date with build_epoch. get_metadata reads the build epoch, and the metadata test checks the new field.
Build-specific download request
src/pygeoipupdate/client.py, tests/test_client.py, tests/test_cli.py, tests/test_updater.py, HISTORY.rst
download passes build_epoch to _fetch_database, which sends it as a query parameter. Test metadata fixtures and the download request expectation use build_epoch. The changelog describes the change.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 3d258

The build-specific download selector is propagated consistently, and a client test checks the resulting request parameter. No concrete merge-blocking risk is indicated.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 30.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 5 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
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.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly summarizes the main change: downloading the exact build identified by metadata instead of selecting a build by date.
Full details: Docstring Coverage

Explanation

Docstring coverage is 30.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 5 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit checks the build epoch bright,
Then follows its checksum through the night.
No date can send it down the wrong track,
The matching archive hops right back.
Test fixtures join the cheerful run,
The rabbit nibbles, work is done.

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

The metadata response gives the MD5 of one database build. Requesting
the download by date returns the latest build of that day, so a build
published between the two requests causes a checksum mismatch.

Request the download by the build_epoch from the metadata instead. The
server always sends build_epoch, so the date request is gone.

This ports maxmind/geoipupdate#566.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 15, 2026 22:38
@oschwald
oschwald force-pushed the greg/download-by-build-epoch branch from 3d25812 to 996f87a Compare October 15, 2026 22:38

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.

This branch has not been deployed

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants