Skip to content
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

/search/inside incorrectly including aggregates/facets #10439

Closed
cdrini opened this issue Feb 11, 2025 · 1 comment
Closed

/search/inside incorrectly including aggregates/facets #10439

cdrini opened this issue Feb 11, 2025 · 1 comment
Labels
Needs: Breakdown This big issue needs a checklist or subissues to describe a breakdown of work. [managed] Needs: Lead Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Theme: Performance Issues related to UI or Server performance. [managed] Type: Bug Something isn't working. [managed]

Comments

@cdrini
Copy link
Collaborator

cdrini commented Feb 11, 2025

Problem

Our search inside endpoint is including aggregates in the response: https://openlibrary.org/search/inside.json?q=gandalf

These should be excluded by the nofacets: true call here:

**({'nofacets': 'true'} if not facets else {}),

But it seems like the API has changed. Facets are expensive and we shouldn't request them if we're not using them, so let's see how we can remove them.

Reproducing the bug

  1. Go to https://openlibrary.org/search/inside.json?q=gandalf
  • Expected behavior: No aggregates in response
  • Actual behavior: Aggregates in response

Context

  • Browser (Chrome, Safari, Firefox, etc): FF
  • OS (Windows, Mac, etc): Win11
  • Logged in (Y/N): Y
  • Environment (prod, dev, local): prod

Breakdown

Requirements Checklist

  • [ ]

Related files

Stakeholders


Instructions for Contributors

  • Please run these commands to ensure your repository is up to date before creating a new branch to work on this issue and each time after pushing code to Github, because the pre-commit bot may add commits to your PRs upstream.
@cdrini cdrini added Needs: Breakdown This big issue needs a checklist or subissues to describe a breakdown of work. [managed] Needs: Lead Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Theme: Performance Issues related to UI or Server performance. [managed] Type: Bug Something isn't working. [managed] labels Feb 11, 2025
@cdrini
Copy link
Collaborator Author

cdrini commented Feb 11, 2025

Ah never mind, non-issue! This appears by design, the JSON endpoint includes the facets, the HTML one does not, so this is working correctly

@cdrini cdrini closed this as not planned Won't fix, can't repro, duplicate, stale Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Breakdown This big issue needs a checklist or subissues to describe a breakdown of work. [managed] Needs: Lead Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Theme: Performance Issues related to UI or Server performance. [managed] Type: Bug Something isn't working. [managed]
Projects
None yet
Development

No branches or pull requests

1 participant