Add stream_response option to hackney adapter

[tim-smart]

Playing around with streaming responses (#271).

Only implemented for hackney here.

[teamon]

Amazing! Let's get the response streaming become a thing 👍

[maxmarcon]

👍 for this!

[cgarvis]

@tim-smart @teamon anything I can do to get this over the finish line? This would allow me to remove a dependency on Downstream from my app.

[Hajto]

Do you need any help with this? I would also want to help to get this through the finish line.

[byronalley]

I'll add my voice to the "I'd love to see this feature merged, can I help?" crowd!

[tim-smart]

Added some changes to improve the API surface.

• Added response to the Env struct. Defaults to :default, but also has a :stream option
• Added __pid__ to Env, for capturing the pid of the requestor process
• Added stream_owner to the hackney adapter options, which defaults to __pid__ from the Env

New API example:

{:ok, %Env{body: stream}} = Tesla.get("https://google.com", response: :stream)

# Use the response stream
data = Enum.join(stream)

[tim-smart]

Maybe leaving this as an adapter option is better, as it will not be implemented for every adapter.

[connorjacobsen]

Hey @teamon I used this as a base for a version that is adapter option driver: https://github.com/connorjacobsen/tesla/tree/hackney-response-streaming

I would love to be able to use hackney to stream responses like you and Tim have detailed here. Is there anything I can do to help make that happen? Happy to take a stab at some more code as needed, would just want some direction from you so it fits with where you want to guide the library.

[connorjacobsen]

@tim-smart @yordis is there anything I can do to help with this PR? Or the fork I linked above? tesla is a fantastic library and I would love to be able to contribute a little something back to it if it's helpful!

[yordis]

@connorjacobsen, by all means, please take the lead.

In the worst case, @tim-smart solved it differently, and we commit to one of the solutions, and you get to learn about Tesla internals; from my perspective, still a huge win. In the best case, we fix the issue.

I am unfamiliar with hackney, but I can help to some extent.

[connorjacobsen]

@yordis I am equally happy with either solution. I basically just took @tim-smart's comment and riffed on it in a way that kept mostly everything isolated to the Hackney adapter itself rather than adding the response field to the core Tesla.Env struct.

So I guess my question for you + others is: do you want to isolate this to the Hackney adapter or should we go down a more generalized path adding streaming for all adapters (all seem to support streaming responses). If the latter, do we want to pursue that one-by-one or all at once? Do you have a preference?

As a meta question: is this the right place to discuss this? Is there somewhere better?

[yordis]

Ideally, we add streaming to all adapters.

About one by one vs. all at once. If you are giving it a try, instead, you do a big PR where you experiment and find a path forward that works for the official adapters, and if we need to, we split the PR into smaller ones if they are too big.

I worry about merging something that prevents me from releasing a version or something broken or error-prune.

[teamon]

There is also this PR https://github.com/elixir-tesla/tesla/pull/540 that implements streaming for Finch.

I’d say we go with the response: :stream opt and implement each adapter separately.

[yordis]

Either way is fine by me

[noozo]

Any progress on this?