search

sneako / finch

Elixir HTTP client, focused on performance
MIT License
1.23k stars 114 forks source link

Finch.stream: Return `{:cont, acc} | {:halt, acc}` #237

Closed wojtekmach closed 10 months ago

wojtekmach commented 11 months ago

Currently the given function returns possibly updated accumulator. If we want to cancel the request we need to exit/throw/raise. Should we change the contract so that we return {:cont, acc} | {:halt, acc}, the latter cancelling the request under the hood?

I don't have a particular use case in mind at the moment, just wondering cause we wouldn't be able to change it after v1.0. If we were to move forward with this, I think we could assist users with a deprecation if their callback does not return the shape. (It would be a breakage if they happen to return the exact tuple in their accumulator.)

cc @josevalim

josevalim commented 11 months ago

Yeah, it is probably worth moving to this style, even though it is a bit verbose. Alternatively we can call it Finch.stream_while, to mirror Enum.reduce_while.

zachallaun commented 11 months ago

Finch.stream_while sounds great and fixes the backwards compatibility issue. Could also potentially deprecate Finch.stream/5 and encourage rewriting to Finch.stream_while.

sneako commented 11 months ago

Finch.stream_while sounds good to me!

dlindenkreuz commented 6 months ago

@sneako Can you create a release for this?

sneako commented 6 months ago

@dlindenkreuz I have just released 0.17.0 :tada: