Fork me on GitHub
#onyx
<
2017-11-14
>
asolovyov10:11:29

@lucasbradstreet should I maybe ping you for onyx-http? 🙂

lmergen10:11:47

@asolovyov perhaps something i can help with ?

asolovyov10:11:29

I'd love to see this merged 🙂 Any comments/anything to do?

asolovyov10:11:52

I'm also not sure if there needs to be batch output support, and how it should work

asolovyov10:11:44

I'd prefer to do it later or something, never saw a need for that 🙂

lmergen15:11:22

@asolovyov looks pretty solid to me! i think batch output support is not really necessary

lmergen15:11:37

you're already doing manifold / aleph http requests

lmergen15:11:55

which means everything is being done in the background anyway

lmergen15:11:16

the only advantage that actual request batching might have would be the ability to do http request pipelining

lmergen15:11:07

i like this PR, looks like very good quality

lucasbradstreet19:11:02

@asolovyov batch output was handy for some APIs that needed to collect the results together first. With windowing, it’s no longer really necessary as you can perform those actions very easily via windows.

lucasbradstreet19:11:15

@asolovyov feel free to drop the batch output code and README

lucasbradstreet19:11:03

If you’re happy with it, I’m happy to merge it.

asolovyov19:11:14

ah one moment, I'll drop it from readme

lucasbradstreet19:11:19

(once you’ve changed that)

asolovyov20:11:01

@lucasbradstreet done! The only thing I don't really like is post-process-fn, but we really need it so that we can communicate back various data from response - we track emails and messages we've sent to user. I think we discussed that at some point in time but we never came around to making changes to Onyx which would allow us to drop it...

lucasbradstreet20:11:54

Right. I think there is something we could do in Onyx for that, but it’s kinda hard to know what the user facing API should look like. If you want to create an issue with the overall goal there (e.g. we want to do some post processing after a plugin write succeeds), that would be good.

lucasbradstreet20:11:14

It might be a while before we get around to it, but if we can figure out what the design should look like that would be a start.

lucasbradstreet20:11:23

Thanks for getting this together.

lucasbradstreet20:11:03

Btw, did you look into whether Thread/sleep was the right approach for manifold? I could see ending up with a lot of stuck threads.

lucasbradstreet23:11:10

@asolovyov I’ve merged it. I’m getting it updated for 0.12. One thing that might be worth improving is allowing it to backpressure when there are multiple requests outstanding.