cognitect-labs / aws-api

AWS, data driven
Apache License 2.0
723 stars 98 forks source link

Support response streaming #14

Open dchelimsky opened 5 years ago

dchelimsky commented 5 years ago

A couple of people have asked me if/when we'll support streaming for blob types. We do accept and return InputStreams, but the underlying http-client does not.

SevereOverfl0w commented 5 years ago

The thing I really care about is being able to do chunked uploads to S3. Not just of files (where I can perform (.length f)) but also on unknowns where I'm streaming generation:

potetm commented 5 years ago

I think a recent change to how you coerce to ByteBuffer broke my usage of InputStream. It looks like you started requiring .markSupported InputStreams. This is understandable since you were fully consuming the InputStream into memory. However, my take is either:

  1. InputStreams should be supported without reading them fully into memory.
  2. InputStream should not supported at all. I know how to read into a byte array or a ByteBuffer, and not supporting streams at all would make the behavior clear.

An implementation note: It looks like jetty has an InputStreamContentProvider. I would assume the cognitect.http-client library could dispatch to use that instead of assuming a ByteBuffer body.

dchelimsky commented 5 years ago

@potetm would you kindly make a separate issue for that? Thanks!

potetm commented 5 years ago

Sure! https://github.com/cognitect-labs/aws-api/issues/65

DerGuteMoritz commented 3 years ago

I'd be willing to contribute a patch to implement streaming request and response bodies. @dchelimsky would that suit you? If so, I noticed that the code repository for the com.cognitect/http-client library doesn't seem to be available anywhere on the public internet. Could this be an oversight?

dchelimsky commented 3 years ago

I'd be willing to contribute a patch to implement streaming request and response bodies. @dchelimsky would that suit you? If so, I noticed that the code repository for the com.cognitect/http-client library doesn't seem to be available anywhere on the public internet. Could this be an oversight?

Thanks for the offer, @DerGuteMoritz , but we don't accept patches for aws-api. As it happens, we are in the middle of some redesign work that opens the door to supporting this. It's just slow-going since resources are limited.

re: the http-client, the code is available in a freely distributed jar, but falls under the same policy of not accepting contributions.

DerGuteMoritz commented 3 years ago

Thanks for the offer, @DerGuteMoritz , but we don't accept patches for aws-api.

Alright - glad I asked first :smile:

As it happens, we are in the middle of some redesign work that opens the door to supporting this. It's just slow-going since resources are limited.

That's great to hear, patiently looking forward to that then!

re: the http-client, the code is available in a freely distributed jar, but falls under the same policy of not accepting contributions.

Aye, was just wondering about the lack of a public repository anyhow. But your call to keep it private, of course :+1:

SevereOverfl0w commented 3 years ago

I notice the title has been reworded, but I'd still be highly interested in using aws-api for streaming uploads to S3, should I make a new issue?