Add some types to coverstore#11593
Conversation
404b7d6 to
1accf59
Compare
There was a problem hiding this comment.
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
CoverDbDetailsandPartialCoverDetailsclasses to type cover detail objects - Refactored variable naming in the
cover.GETmethod for better clarity (e.g.,value→cover_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.
RayBB
left a comment
There was a problem hiding this comment.
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): |
There was a problem hiding this comment.
This is a web storage because that's what the db returns.
RayBB
left a comment
There was a problem hiding this comment.
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
* Add some types to coverstore * Update openlibrary/coverstore/code.py
Small refactor while trying to grok some of the coverstore code in response to coverstore investigation during high traffic event.
Technical
Testing
Screenshot
Stakeholders