Support for other search types

[bermanjosh]

Firstly - awesome work, thanks. We're starting to use your package in code moving towards production, and it's made a lot of my SQL -> Elastic migration much, much easier.

I was wondering if we'd be able to open up other potential search-types; I specifically am looking for scan. I'm happy to do do a PR; if that's ok with you, do you have an approach you'd prefer?

My first (very rough) thought is to add a new ADT as a field in the Search type, which would be used to trigger some new logic in dispatchSearch to add some query parameters. This way it wouldn't be a breaking change for any of the search functions, or any smart constructors (once I updated them, obviously). Since you have a manual toJSON, it can be safely ignored for the body.

Thoughts?

Thanks!

[bitemyapp]

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.

[bermanjosh]

@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

[bitemyapp]

@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.

[bermanjosh]

@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.

[bitemyapp]

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

[bermanjosh]

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

[bitemyapp]

@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.

[bitemyapp]

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

[bermanjosh]

@bitemyapp sorry about the delay, merge is clear

[bitemyapp]

@bermanjosh bouncing the CI tests.

[bitemyapp]

@bermanjosh bounced, tests still broken.

#53

[bermanjosh]

@bitemyapp sorry about that, taken care of

[bermanjosh]

@bitemyapp
I missed an encoding issue:
#63

[bitemyapp]

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

[bermanjosh]

@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... :-) )