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 algolia indexing capabilities #1

Merged
merged 2 commits into from
Aug 18, 2017

Conversation

redox
Copy link
Contributor

@redox redox commented Aug 11, 2017

Add a simple algoliasearch-based concern indexing big articles in
multiple smaller records.

Note: for the sake of the MVP, the split function is currently very stupid.

Add a simple `algoliasearch`-based concern indexing big articles in
multiple smaller records.

Note: for the sake of the MVP, the split function is currently very stupid.
@hannesfostie
Copy link
Owner

This is great @redox ! Certainly something I can work with.

I'll port this over to our app some time next week and keep you posted if I run into any issues or questions.

@redox
Copy link
Contributor Author

redox commented Aug 11, 2017

Awesome @hannesfostie!

I would recommend reading https://blog.algolia.com/how-to-build-a-helpful-search-for-technical-documentation-the-laravel-example/ in order to understand how our DocSearch crawler is building the Algolia records splitting documentation pages in multiple objects with title hierarchy.

Let us know if that makes sense, we would love to help!

@hannesfostie
Copy link
Owner

@redox while I'm working on porting your PR over to our app and filling in the missing pieces, I've found 2 bits of feedback I wanted to share and get your thoughts on:

  • The webmock library that you provide does not work quite like "advertised" in that api client article. I've tried a number of things, required the file in different places, but no matter what I tried the stubs weren't being loaded. I ended up copy and pasting all those stubs into our test config
  • The above library doesn't correctly stub all requests. The way you're deleting all records for an Article object (delete_by_query) causes an infinite loop because of this code and the way the stubs are set up - the stub should return hits in the body I imagine to prevent this.

@redox
Copy link
Contributor Author

redox commented Aug 16, 2017

The webmock library that you provide does not work quite like "advertised" in that api client article. I've tried a number of things, required the file in different places, but no matter what I tried the stubs weren't being loaded. I ended up copy and pasting all those stubs into our test config

Oh that's weird; just including the file & calling WebMock.enable! should make it :/

The above library doesn't correctly stub all requests. The way you're deleting all records for an Article object (delete_by_query) causes an infinite loop because of this code and the way the stubs are set up - the stub should return hits in the body I imagine to prevent this.

Arg, that's correct -> will add a GH issue.

…nt to

leverage the fine-tuned delete_by_query
@redox
Copy link
Contributor Author

redox commented Aug 17, 2017

Just released the 1.15.0 version that will fix the delete_by_query.

@hannesfostie
Copy link
Owner

Thanks @redox ! That was incredibly fast :-) The previously failing tests now seem to work.

As for the webmock issue, I'm going to try to replicate in this repo after merging this PR.

@hannesfostie hannesfostie merged commit cd42d4e into hannesfostie:master Aug 18, 2017
@redox
Copy link
Contributor Author

redox commented Aug 18, 2017

Thanks @redox ! That was incredibly fast :-) The previously failing tests now seem to work.

You're welcome! Maybe force usage of 1.15.1, there was a small perf regression in 1.15.0.

@hannesfostie
Copy link
Owner

@redox is it possible that version has not been released? It can't be found on Rubygems

@redox
Copy link
Contributor Author

redox commented Aug 19, 2017

@redox is it possible that version has not been released? It can't be found on Rubygems

Arg /o\ should be good now /o\ @hannesfostie

@redox redox deleted the algolia-concern-mvp branch August 19, 2017 08:13
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