Skip to content

Upload progress recipe#1528

Closed
aried3r wants to merge 1 commit into
lysine-dev:masterfrom
aried3r:ar/upload_progress_recipe
Closed

Upload progress recipe#1528
aried3r wants to merge 1 commit into
lysine-dev:masterfrom
aried3r:ar/upload_progress_recipe

Conversation

@aried3r

@aried3r aried3r commented Mar 23, 2015

Copy link
Copy Markdown
Contributor

Let's have it, @swankjesse! ;)

There's a few things I'm not quite sure about how to handle (basically the whole writeTo() method), so I'm awaiting your comments. But I hope I'll stay below 10 this time :p

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you change this so that it wraps another RequestBody? That way the caller can do whatever they want: upload data being generated, a file, or whatever.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to, but from what I've gathered I can't get the upload data out of the request body, so it would have to look like this, file being passed to the RequestBody and again to the ProgressRequestBody so it can be accessed for contentLength() etc.

File file = new File("website/static/logo-square.png");
RequestBody requestBody = RequestBody.create(MEDIA_TYPE_PNG, file);
Request request = new Request.Builder()
  .header("Authorization", "Client-ID " + IMGUR_CLIENT_ID)
  .url("https://api.imgur.com/3/image")
  .post(new ProgressRequestBody(requestBody, file, progressListener)) // Here file is passed a second time
  .build();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can, but you need to be a bit sneaky. You'd just need to call delegate.writeTo(buffer(progressSink(sink))) in your own writeTo method.

@swankjesse

Copy link
Copy Markdown
Collaborator

I think this is pretty awesome. Consider building on the PostFile endpoint instead. It's simpler & so there's less to distract from the main content of the example. With this one it's unclear whether you're showing me about multipart vs. progress monitor.

@swankjesse

Copy link
Copy Markdown
Collaborator

This is great!

@aried3r

aried3r commented Mar 30, 2015

Copy link
Copy Markdown
Contributor Author

With this one it's unclear whether you're showing me about multipart vs. progress monitor.

The only difference is .header("Authorization", "Client-ID " + IMGUR_CLIENT_ID), I'm not even explicitly using a multipart builder, I'm not passing a name or a description or anything, but I can change it, if you want.

The reason why I used the imgur example was that the README is below 2KB which means there's no real progress to watch, but I guess it should just serve as an example?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(You can look at the PostStreaming example to see an unknown content length in practice.)

@swankjesse

Copy link
Copy Markdown
Collaborator

@aried3r follow me on the PostStreaming thing?

@aried3r

aried3r commented Apr 15, 2015

Copy link
Copy Markdown
Contributor Author

Sorry, been busy. I'll try to get this done during the weekend. Stay tuned! ;)

@swankjesse

Copy link
Copy Markdown
Collaborator

I'm going to close this for now. If you'd like to revisit, please do!

@LouisCAD

Copy link
Copy Markdown

Does the code found here sticks to your recommendations @swankjesse ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants