Support for other search types about bloodhound HOT 15 CLOSED

You can volleyball a PR but I'm hesitant to recommend doing that as some people find review on something they think should be done exhausting. I don't mind using it as a method of directing design as it can be easier to just "show" what you mean than to try to describe it in words for some people. What you propose sounds like it'll be fine. :)

It does look like a low-impact way of doing this would be another field in Search, probably also a Maybe. To make sure we don't do something silly, while I am not prepared to add full streaming support in, it might be worth thinking about a streaming-library agnostic means of presenting the Scroll & Scan functionality so that it can be readily plugged into Pipes | Conduit | io-streams. The best example I'm aware of off-hand is the changes Snoyman made to WAI which are described here. This can provide some design guidance as "paginating through results produced by effectful computations and processing them linearly" seems a dead ringer for streaming to me.

I'm very glad the library has helped. I know I am definitely happier not having to create "templated" stringly queries that I interpolate variables into anymore.

from bloodhound.

@bitemyapp
I totally didn't understand what streaming had to do with anything; I didn't understand how elastic's scroll worked. Now I think I get it, and I understand what you're saying. Yeah, how to do that will take some thought. I guess the most basic way would be one lower-level function which returns some Maybe Lazy.ByteString (or something) which can be plugged into something streaming, and a higher-level helper function to convert the whole thing into a strict for direct usage. Make sense?

In the meanwhile, you can look over the more trivial work done:
bermanjosh@c2a362e

Thanks

from bloodhound.

@bermanjosh The guideline/default I've operated on is a lazy spine of strict bytestring chunks or in this case, pages. I can't say precisely what this should look like or how it should work in order to be amenable to integration with streaming libraries which is why I referenced Snoyman's post on this.

I'll take a look at your commit later today.

from bloodhound.

@bitemyapp
Hi,
Is it ok if I submit that PR now? The functionality in the branch works. I don't know if it's streaming-ready, though.

from bloodhound.

@bermanjosh I'd rather review the code via PR anyway - go ahead.

from bloodhound.

@bitemyapp Hi - have you have a chance to review the PR? Thanks.

from bloodhound.

@bermanjosh oh sorry, I was waiting for you to fix the merge conflict. I try to avoid reviewing stuff waiting for a cleanup. I'll start looking now and do a final scan when you fix the conflict.

from bloodhound.

@bermanjosh took a look, left two comments. Resolve the conflict and I'll try to get this wrapped up.

from bloodhound.

@bitemyapp sorry about the delay, merge is clear

from bloodhound.

@bermanjosh bouncing the CI tests.

from bloodhound.

@bermanjosh bounced, tests still broken.

#53

from bloodhound.

@bitemyapp sorry about that, taken care of

from bloodhound.

@bitemyapp
I missed an encoding issue:
#63

from bloodhound.

@bermanjosh just catching up to #63 having been merged? Need anything else?

from bloodhound.

@bitemyapp Yeah, I was just looking around for something else and realized I never closed it. (Now you see why I'm not the right guy for PIC status... :-) )

from bloodhound.