Skip to content

Fixing retries of Storage.create #2359

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

Merged
merged 2 commits into from
Aug 18, 2017

Conversation

garrettjonesgoogle
Copy link
Member

  • For Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... options): Turning off retries and marking @Deprecated, because once the InputStream has been consumed, it can't be consumed again.
  • For Blob create(BlobInfo blobInfo, byte[] content, BlobTargetOption... options): Re-creating the InputStream on every call, so that the same data is resent to the server.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 18, 2017
Copy link
Contributor

@vam-google vam-google left a comment

Choose a reason for hiding this comment

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

LGTM, but maybe mark()/reset() is worth doing it.

Map<StorageRpc.Option, ?> optionsMap = optionMap(targetOptions.x(), targetOptions.y());
InputStream inputStreamParam = firstNonNull(content, new ByteArrayInputStream(EMPTY_BYTE_ARRAY));
// retries are not safe when the input is an InputStream, so we can't retry.
return Blob.fromPb(this, storageRpc.create(blobPb, inputStreamParam, optionsMap));

This comment was marked as spam.

@@ -1460,6 +1463,7 @@ public static Builder newBuilder() {
* @return a [@code Blob} with complete information
* @throws StorageException upon failure
*/
@Deprecated

This comment was marked as spam.

This comment was marked as spam.

@nafg
Copy link

nafg commented Oct 22, 2017

So for now we're not supposed to use it and are required to either load everything into RAM or write a lot of boilerplate?

@frankyn
Copy link
Contributor

frankyn commented Nov 6, 2017

Ping @garrettjonesgoogle and @andreamlin for user feedback on this PR. Is there a plan to introduce RestartableInputStream as stated?

@garrettjonesgoogle
Copy link
Member Author

I have filed #2620 to track the work of introducing RestartableInputStream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants