Skip to content

Add some types to coverstore#11593

Merged
RayBB merged 2 commits intointernetarchive:masterfrom
cdrini:refactor/some-types-coverstore
Mar 1, 2026
Merged

Add some types to coverstore#11593
RayBB merged 2 commits intointernetarchive:masterfrom
cdrini:refactor/some-types-coverstore

Conversation

@cdrini
Copy link
Collaborator

@cdrini cdrini commented Dec 14, 2025

Small refactor while trying to grok some of the coverstore code in response to coverstore investigation during high traffic event.

Technical

Testing

Screenshot

Stakeholders

@cdrini cdrini force-pushed the refactor/some-types-coverstore branch from 404b7d6 to 1accf59 Compare December 14, 2025 21:52
@cdrini cdrini marked this pull request as ready for review December 16, 2025 16:55
Copilot AI review requested due to automatic review settings December 16, 2025 16:55
@github-project-automation github-project-automation bot moved this to Waiting Review/Merge from Staff in Ray's Project Dec 16, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds type annotations to several functions in the coverstore codebase to improve code clarity and type safety. The changes introduce type hints for function parameters and return types, and define two TypedDict-style classes (CoverDbDetails and PartialCoverDetails) to provide structure for cover detail objects.

  • Added type annotations to utility functions (ol_things, ol_get) and core cover handling functions
  • Introduced CoverDbDetails and PartialCoverDetails classes to type cover detail objects
  • Refactored variable naming in the cover.GET method for better clarity (e.g., valuecover_id)

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
openlibrary/coverstore/utils.py Added type hints to ol_things and ol_get functions
openlibrary/coverstore/db.py Introduced CoverDbDetails class and added type annotation to details function
openlibrary/coverstore/coverlib.py Added TYPE_CHECKING import and type hint to read_image function
openlibrary/coverstore/code.py Added PartialCoverDetails class, type annotations to multiple functions, and refactored variable naming in cover.GET

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@RayBB RayBB left a comment

Choose a reason for hiding this comment

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

I didn't test this locally but we need to fix the key string issue.
Otherwise the types themselves look right.

Also today I learned about the _ support for large numbers.



def details(id):
class CoverDbDetails(web.storage):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a web storage because that's what the db returns.

@RayBB RayBB self-requested a review March 1, 2026 07:43
Copy link
Collaborator

@RayBB RayBB left a comment

Choose a reason for hiding this comment

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

Finally got around to putting this on testing the other day and it's been going strong Gave it another review and I think this one is safe to merge

@RayBB RayBB merged commit 71dd632 into internetarchive:master Mar 1, 2026
4 checks passed
@github-project-automation github-project-automation bot moved this from Waiting Review/Merge from Staff to Done in Ray's Project Mar 1, 2026
bhardwajparth51 pushed a commit to bhardwajparth51/openlibrary that referenced this pull request Mar 3, 2026
* Add some types to coverstore

* Update openlibrary/coverstore/code.py
@RayBB RayBB removed the On Testing label Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants