ring-clojure / ring-json

Ring middleware for handling JSON
313 stars 47 forks source link

Add stream support for response body - StreamableResponseBody impl #60

Closed pete23 closed 5 years ago

pete23 commented 5 years ago

As previously trailed in issue #58 and discussed at length in PR #59, here is a version of the JSON streaming support patch reworked to use StreamableResponseBody rather than piped-io-stream.

Should all be pretty much as expected; had to add a helper fn to the tests to get the content out of the more opaque body.

Testing seems slightly faster with my simple large-scale harness but probably within the bounds of variance - certainly removing the copy thread of piped-io-stream can't have hurt.

Let me know if you need any more changes/polish!

weavejester commented 5 years ago

This PR should be fine to merge, once the streamable->string function is made private, and the commits are squashed.

pete23 commented 5 years ago

Yeah, sorry had other things on. Will take a look once I get the kids into bed.

pete23 commented 5 years ago

Abandoning this trail due to git puzzlement. Next PR has @weavejester's requested changes.