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

Add ability to batch search Rummager #855

Merged
merged 1 commit into from Nov 2, 2018
Merged

Conversation

karlbaker02
Copy link
Contributor

@karlbaker02 karlbaker02 commented Oct 24, 2018

This commit enables the new batch search endpoint to be called on Rummager, where each of the individual queries to be batched is encoded in a readable string as part of the query string.

Co-authored-by: Oscar Wyatt oscar.wyatt@digital.cabinet-office.gov.uk

Trello: https://trello.com/c/QlPhl3wp/122-add-batchsearch-method-to-gds-api-adaptors

searches.each_with_index do |search, index|
url_friendly_search = {}
url_friendly_search[index] = search
url_friendly_searches << url_friendly_search
Copy link
Contributor

Choose a reason for hiding this comment

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

It took me a while to understand this. Maybe clearer to use:

url_friendly_searches << { index => search }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would certainly work.

#
# # @see https://github.com/alphagov/rummager/blob/master/doc/search-api.md
def batch_search(searches, additional_headers = {})
url_friendly_searches = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be clearer to do

url_friendly_searches = searches.each_with_index.map do |search, index|
  { index => search }
end

@vanitabarrett vanitabarrett force-pushed the batch-search-rummager branch 2 times, most recently from 5d5b0e4 to 678e9f2 Compare October 29, 2018 10:47
Copy link
Contributor

@thomasleese thomasleese left a comment

Choose a reason for hiding this comment

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

Would be good to get an entry in the CHANGELOG too. I'm going to do a release of gds-api-adapters today, I can include this in if you want?

This commit enables the new batch search endpoint to be called on Rummager, where each of the individual queries to be batched is encoded in a readable string as part of the query string.

Co-authored-by: Oscar Wyatt <oscar.wyatt@digital.cabinet-office.gov.uk>
@vanitabarrett vanitabarrett changed the title Add ability to batch search Rummager [DO NOT MERGE] Add ability to batch search Rummager Nov 1, 2018
@vanitabarrett
Copy link
Contributor

Note: this is all ready to go, but we want to hold off merging this until the Rummager change has been merged.

@vanitabarrett vanitabarrett changed the title [DO NOT MERGE] Add ability to batch search Rummager Add ability to batch search Rummager Nov 2, 2018
@vanitabarrett vanitabarrett merged commit 8b912d9 into master Nov 2, 2018
@vanitabarrett vanitabarrett deleted the batch-search-rummager branch November 2, 2018 09:38
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.

None yet

4 participants