Don't hit affiliate server on /isbn#11304
Conversation
cdrini
changed the title
cdrini
added
the
Patch Deployed
cdrini
added
the
Priority: 1
There was a problem hiding this comment.
Pull Request Overview
This PR disables automatic imports from the affiliate server when querying books by ISBN through the /isbn endpoint to resolve traffic issues. The change maintains the ISBN lookup functionality while preventing resource-intensive server hits.
- Adds an
allow_importparameter to theEdition.from_isbn()method - Sets
allow_import=Falsewhen calling from the/isbnendpoint to skip affiliate server calls - Preserves existing functionality for other callers of the method
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| openlibrary/plugins/openlibrary/code.py | Updated ISBN endpoint to disable imports by passing allow_import=False |
| openlibrary/core/models.py | Added allow_import parameter to control affiliate server calls in Edition.from_isbn() |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| cls, | ||
| isbn_or_asin: str, | ||
| high_priority: bool = False, | ||
| allow_import: bool = False, |
There was a problem hiding this comment.
The default value False for allow_import changes the existing behavior for all callers who don't explicitly pass this parameter. Consider defaulting to True to maintain backward compatibility, or ensure all existing callers are updated to explicitly pass allow_import=True where needed.
| allow_import: bool = False, | |
| allow_import: bool = True, |
There was a problem hiding this comment.
I decided to default to false as a result of the performance risks associated. Enabling it on a per-use case basis is I think a safer approach.
|
Reviewed on call with @jimchamp |
Closes #11280 . This remove the auto-import of the
/isbn. Note the greatest fix long term. We should create a separate issue to investigate how to re-enable. But this did appear to resolve our traffic issues, while still keeping the/isbnpoint up for finding books.Technical
Testing
Screenshot
Stakeholders