Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upload progress recipe #1528

Closed
wants to merge 1 commit into from

Conversation

aried3r
Copy link
Contributor

@aried3r aried3r commented Mar 23, 2015

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

private final MediaType contentType;
private final File file;

public ProgressRequestBody(MediaType contentType, File file,
Copy link
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
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
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
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
Collaborator

This is great!

@aried3r
Copy link
Contributor Author

aried3r commented Mar 30, 2015

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?

super.write(source, byteCount);
totalBytesWritten += byteCount;
progressListener.update(totalBytesWritten, contentLength(),
totalBytesWritten == contentLength());
Copy link
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
Collaborator

@aried3r follow me on the PostStreaming thing?

@aried3r
Copy link
Contributor Author

aried3r commented Apr 15, 2015

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

@swankjesse
Copy link
Collaborator

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

@LouisCAD
Copy link

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants