[jclouds/jclouds] JCLOUDS-769: putBlob OutputStream straw man (#1129)

[Andrew Gaul <no...@github.com>]

This allows clients to write their payloads via an OutputStream
instead of providing an InputStream during construction.  This can
avoid buffering in some situations although one must still provide the
Content-Length.
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/1129

-- Commit Summary --

  * JCLOUDS-769: putBlob OutputStream straw man

-- File Changes --

    M core/src/main/java/org/jclouds/http/HttpRequest.java (19)
    M core/src/main/java/org/jclouds/http/HttpResponse.java (19)
    A core/src/main/java/org/jclouds/http/functions/ReturnOutputStream.java (36)
    M core/src/main/java/org/jclouds/http/internal/BaseHttpCommandExecutorService.java (7)
    M core/src/main/java/org/jclouds/http/internal/JavaUrlHttpCommandExecutorService.java (38)
    M core/src/main/java/org/jclouds/rest/internal/GeneratedHttpRequest.java (6)
    M core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java (5)
    M core/src/main/java/org/jclouds/rest/internal/TransformerForRequest.java (4)
    M providers/azureblob/src/main/java/org/jclouds/azureblob/AzureBlobClient.java (10)
    M providers/azureblob/src/test/java/org/jclouds/azureblob/AzureBlobClientLiveTest.java (37)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/1129.patch
https://github.com/jclouds/jclouds/pull/1129.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1129

Re: [jclouds/jclouds] JCLOUDS-769: allow for streaming PUTs (#1129)

[Andrew Gaul <no...@github.com>]

I have started to implement the other providers; if you have feedback on the approach I would appreciate it sooner rather than later.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1129#issuecomment-329390295

Re: [jclouds/jclouds] JCLOUDS-769: streaming putBlob (#1129)

[stevegomez17 <no...@github.com>]

thanks

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1129#issuecomment-330142532

Re: [jclouds/jclouds] JCLOUDS-769: streaming putBlob (#1129)

[Ignasi Barrera <no...@github.com>]

I've finally had some time to look at this. Thanks for starting this proposal!

I like the idea of having the API methods return a stream clients can write to, but I'm not sure about several things that are needed to have that.

It's weird for me to have an OutputStream in the HttpResponse. If I get it right it's there just to be accessible from the response transformers, a convenient way to propagate it to the API return value. But it looks weird; I understood it better if it was in the request. We should also take into account that, although not so widely used, there is a generic Http utils class that allows using the HttpRequest and HttpResponse directly, and there are also binders and transformers for those objects. If we modify the request/response objects we need to make sure the features we already leverage that use them directly can still be used with the new additions.

This said, could an alternate approach be payload-based? I mean, instead of changing the return type of a method, could we play with specialized Payloads instead? For example, with the [OkHttp driver we're already streaming data](https://github.com/jclouds/jclouds/blob/master/drivers/okhttp/src/main/java/org/jclouds/http/okhttp/OkHttpCommandExecutorService.java#L120-L146). Could we modify that to let clients provide a specialized payload that leveraged the streaming PUT? I think a PoC with this driver and specific payloads would also be great.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1129#issuecomment-330141894

Re: [jclouds/jclouds] JCLOUDS-769: streaming putBlob (#1129)

[Andrew Gaul <no...@github.com>]

@nacx Sorry for my delayed response; this recently became interesting again.  I follow that OkHttp allows returning `BufferedSink.outputStream` but I have a problem at the jclouds `HttpRequest` and `HttpResponse` abstraction.  My callers generate dynamic content and want to interact with the underlying `OutputStream` which the current system does not provide.  I agree that abusing `HttpResponse` for something that is not actually a response is an ugly hack and did something slightly more palatable with `Object`.  Reviewing my own code, `EtagOutputStream` was stupid and should be `HttpRequestOutputStream` whose `close` method returns the `HttpResponse`.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1129#issuecomment-407926023

Re: [jclouds/jclouds] JCLOUDS-769: allow for streaming PUTs (#1129)

[Andrew Gaul <no...@github.com>]

This demonstrates one approach to allowing streaming PUTs of data.  This is useful when clients want to drive the interaction themselves, e.g., interacting with async frameworks, constructing/transforming input on the fly.  This functionality maps poorly onto the existing request-response model.  Presently I fake a bunch of the response data to show what I want to do.  Any better suggestions?

Do not commit; straw man to provoke feedback.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1129#issuecomment-321055041

Re: [jclouds/jclouds] JCLOUDS-769: streaming putBlob (#1129)

[Ignasi Barrera <no...@github.com>]

Hmmmmm, I see where you're going.

The main issue I see with this approach is that a client won't know actually when a streaming request has finished, and won't have access to the response code, which might be important...
How are we handling streaming requests to endpoints return a redirect (3xx) we should follow before posting the content?

Access to the response status is important, and that was one of my reasons to suggest the specialized payload approach. My thinking was something like having a specialised binding for, say, `Writer` parameters in API methods, and handle that at the writer level by blocking until the writer is done, then completing the method call and returning following the existing flows and response and error handling mechanisms.

Something like:

```java
@Named("PutObject")
@PUT
@Path("/{key}")
@Headers(keys = EXPECT, values = "100-continue")
@ResponseParser(ReturnOutputStream.class)
ETagOutputStream putObjectStreaming(
      @Bucket @EndpointParam(parser = AssignCorrectHostnameForBucket.class) @BinderParam(BindAsHostPrefixIfConfigured.class) @ParamValidators(BucketNameValidator.class) String bucketName,
      @PathParam("key") @ParamParser(ObjectKey.class) @BinderParam(BindS3ObjectMetadataToRequest.class) S3Object object,
      Writer streamingContent,   // This is an object the client controlls that should be piped to the request outputstream
      PutObjectOptions... options);
```

Does this make sense?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1129#issuecomment-455596882

Re: [jclouds/jclouds] JCLOUDS-769: streaming putBlob (#1129)

[Andrew Gaul <no...@github.com>]

Please reopen against apache/jclouds if this is still relevant.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1129#issuecomment-663855769

Re: [jclouds/jclouds] JCLOUDS-769: streaming putBlob (#1129)

[Andrew Gaul <no...@github.com>]

Closed #1129.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1129#event-3586273675