Skip to content

refactor: replace @web.memoize with @functools.cache#12057

Merged
cdrini merged 1 commit intointernetarchive:masterfrom
bhardwajparth51:12017/refactor/use-cache-instead-of-web-memoize
Mar 17, 2026
Merged

refactor: replace @web.memoize with @functools.cache#12057
cdrini merged 1 commit intointernetarchive:masterfrom
bhardwajparth51:12017/refactor/use-cache-instead-of-web-memoize

Conversation

@bhardwajparth51
Copy link
Copy Markdown
Contributor

@bhardwajparth51 bhardwajparth51 commented Mar 9, 2026

Closes #12017

Technical

Replaced parameterless @web.memoize instances with Python's built-in @functools.cache across 14 files. Explicitly excluded vendor/infogami files and the 3 instances with expiry/background parameters., following @cdrini's guidance:

"Anywhere that doesn't specify any parameters should be safe to switch."

Design Decision: @functools.cache I intentionally used import functools and the fully qualified @functools.cache decorator to avoid namespace collisions. Many modified files already contain from openlibrary.core import cache, and using from functools import cache would have shadowed that module, leading to runtime TypeError: 'module' object is not callable errors.

Excluded — require async-friendly background caching solution (tracked in #12017):

  • get_ol_dumps in openlibrary/plugins/upstream/data.py
  • _get_recent_changes in openlibrary/plugins/upstream/utils.py
  • _get_recent_changes2 in openlibrary/plugins/upstream/utils.py

Excluded — vendored submodule:

  • vendor/infogami files require a separate PR against internetarchive/infogami

Testing

  1. Run Python tests:
docker compose run --rm home make test-py
  1. Run linting:
pre-commit run --all-files
  1. Start dev server via docker compose up and verify the application loads with no circular import or unhashable type errors on startup.

Screenshot

N/A — no UI changes.

Stakeholders

@cdrini @RayBB

Fixes internetarchive#12017. Replaced parameterless @web.memoize instances with Python's built-in @functools.cache across 14 files. Explicitly avoided vendor/infogami files and functions with expiration/background parameters.
@bhardwajparth51
Copy link
Copy Markdown
Contributor Author

@RayBB when you have bandwidth, could you please take a look at this PR? Thanks!

@RayBB
Copy link
Copy Markdown
Collaborator

RayBB commented Mar 12, 2026

@cdrini is the lead so he'll have to look at this. Since there's a lot going on now it probably won't be merged soon.

Copy link
Copy Markdown
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Lgtm! Tested and the site seems to be performing normally.

@cdrini cdrini merged commit 58f44bc into internetarchive:master Mar 17, 2026
4 checks passed
@bhardwajparth51 bhardwajparth51 deleted the 12017/refactor/use-cache-instead-of-web-memoize branch March 17, 2026 19:41
@RayBB RayBB removed the On Testing label Mar 23, 2026
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.

use @cache instead of @web.memoize for most of the codebase

3 participants