Skip to content

Conversation

cdrini
Copy link
Collaborator

@cdrini cdrini commented Sep 24, 2025

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 /isbn point up for finding books.

Technical

Testing

Screenshot

Stakeholders

@cdrini cdrini changed the title Don't hit affiliate server on import Don't hit affiliate server on /isbn Sep 24, 2025
@cdrini cdrini added the Patch Deployed This PR has been deployed to production independently, outside of the regular deploy cycle. label Sep 24, 2025
@cdrini cdrini marked this pull request as ready for review September 25, 2025 17:33
@Copilot Copilot AI review requested due to automatic review settings September 25, 2025 17:33
@cdrini cdrini added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Sep 25, 2025
Copy link
Contributor

@Copilot 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 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_import parameter to the Edition.from_isbn() method
  • Sets allow_import=False when calling from the /isbn endpoint 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,
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
allow_import: bool = False,
allow_import: bool = True,

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

@mekarpeles mekarpeles self-assigned this Sep 26, 2025
@cdrini cdrini assigned jimchamp and unassigned mekarpeles Sep 29, 2025
@cdrini
Copy link
Collaborator Author

cdrini commented Sep 29, 2025

Reviewed on call with @jimchamp

@cdrini cdrini merged commit 844c537 into internetarchive:master Sep 29, 2025
4 checks passed
@cdrini cdrini deleted the hotfix/skip-affiliate-server-isbn branch September 29, 2025 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Patch Deployed This PR has been deployed to production independently, outside of the regular deploy cycle. Priority: 1 Do this week, receiving emails, time sensitive, . [managed]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/isbn endpoint crushing Open Library haproxy queue

3 participants