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 API v2: Add adapter for autocomplete #1292

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Conversation

csutter
Copy link
Contributor

@csutter csutter commented Nov 13, 2024

This brings #1282 back around now that we have decided not to expose Search API v2's autocomplete endpoint publicly, and need to access it from Finder Frontend.

This adds a method for the new autocomplete functionality to Search API v2's API adapter.

  • Add SearchApiV2#autocomplete
  • Remove redundant error scenario tests to avoid tests bloating up too much

@csutter csutter force-pushed the sav2-autocomplete branch 2 times, most recently from 9de68b0 to ea15760 Compare November 13, 2024 15:38
This adds a method for the new autocomplete functionality to Search API
v2's API adapter.

- Add `SearchApiV2#autocomplete`
- Remove redundant error scenario tests to avoid tests bloating up too
  much
Copy link
Member

@kevindew kevindew left a comment

Choose a reason for hiding this comment

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

Looks good, though I imagine that this might box in for local dev where the public endpoints aren't accessible. Based on our previous chat about /api/search/autocomplete.json and /api/search.json, we'd find that a local dev could only have one of these. Is this a problem?

Also, only loosely related to this PR but a reminder before I forget. I guess autocomplete could push a lot more traffic through finder-frontend so we may want to up the number of pods.

@csutter
Copy link
Contributor Author

csutter commented Nov 14, 2024

Looks good, though I imagine that this might box in for local dev where the public endpoints aren't accessible. Based on our previous chat about /api/search/autocomplete.json and /api/search.json, we'd find that a local dev could only have one of these. Is this a problem?

So this should work in local dev if the developer has search-api-v2 running, which is now less of a pain than it used to be – and if they don't, I reckon autocomplete is far enough off the critical path for local development that it'll be fine (worst outcome is you'll get some fetch errors in the console).

Also, only loosely related to this PR but a reminder before I forget. I guess autocomplete could push a lot more traffic through finder-frontend so we may want to up the number of pods.

Agree, they're relatively basic requests but might block enough of the capacity that it's worth dialling up in anticipation 📈

@csutter csutter merged commit 03b3239 into main Nov 14, 2024
41 checks passed
@csutter csutter deleted the sav2-autocomplete branch November 14, 2024 12:05
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.

2 participants