-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Temporarily disable author page facets to see perf impact #11403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Temporarily disable author page facets to see perf impact #11403
Conversation
There was a problem hiding this 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 facet fetching for author book pages by setting facet=False in the Solr query and updates the template to check for the existence of facet_counts before rendering subject sections.
- Disabled facet fetching in the
get_books()method to improve query performance - Added defensive check in the template to prevent errors when facet data is not available
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| openlibrary/plugins/upstream/models.py | Changed facet parameter from True to False in the works_by_author call within Author.get_books() |
| openlibrary/templates/type/author/view.html | Updated condition to check both books_count and books.facet_counts before rendering subjects |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $if books_count and books.facet_counts: | ||
| $:render_subjects(_("Subjects"), books.facet_counts.get('subject_facet'), '') |
Copilot
AI
Nov 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition checks for books.facet_counts but this is redundant with the backend change that sets facet=False. Since facets are now disabled in the backend, this entire subjects display section (lines 169-173) will never render. Consider removing this code block entirely or documenting why it's being kept if there are plans to re-enable facets in the future.
e003dcf to
31ea664
Compare
31ea664 to
05fe72a
Compare
|
@jimchamp notes this is likely causing this error on testing: |

We were seeing high bot traffic to this page, resulting in solr strain across the whole site. Turning off the facets (i.e. the subjects/places/etc) in the sidebar of the authors page, and we seem to be doing much better. They're still hitting us and traffic is 1.5x normal, but we now seem to be able to weather it.
We want to turn these back on, but make them lazy load similarly to the facets on the search page.
Technical
Testing
Screenshot
Stakeholders