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

Add Storage.create() method which takes a restartable input stream #201

Closed
frankyn opened this issue Mar 21, 2020 · 2 comments
Closed

Add Storage.create() method which takes a restartable input stream #201

frankyn opened this issue Mar 21, 2020 · 2 comments
Assignees
Labels
api: storage Issues related to the googleapis/java-storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@frankyn
Copy link
Member

frankyn commented Mar 21, 2020

Filing feature request that was raised in:
googleapis/google-cloud-java#2620

So it doesn't get lost.

@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/java-storage API. label Mar 21, 2020
@frankyn frankyn added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed api: storage Issues related to the googleapis/java-storage API. labels Mar 21, 2020
@dmitry-fa
Copy link
Contributor

dmitry-fa commented Mar 23, 2020

@frankyn
Will it be enough to implement upload based on WriterChannel?

@dmitry-fa dmitry-fa self-assigned this Apr 29, 2020
@dmitry-fa
Copy link
Contributor

dmitry-fa commented May 6, 2020

Hi @frankyn,

Is this functionality really needed?
Justification for that request (googleapis/google-cloud-java#2620) sounds like: so that it is easier for users to create blobs without loading the entire content into memory.
But Storage.create(IntputStream) doesn't load the entire content into memory. I successfully uploaded 1.5Gb on the JVM with the heap of 50M (-Xmx50m). Upload is implemented in:

com.google.api.client.http.AbstractInputStreamContent {
    public void writeTo(OutputStream out) throws IOException {
        IOUtils.copy(this.getInputStream(), out, this.closeInputStream);
        out.flush();
    }
}

IOUtils.copy() uses buffer of 4096 bytes to read from and write to the given streams, so no problem with memory.

As you probably remember, #179 will introduce a number of upload() methods which read the content portion by portion and applies resumable approach.

Even if upload with a restartable input stream is still needed for some reason, it could be implemented without introducing a new class representing a restartable input stream (as it implemented in googleapis/google-cloud-java#2682), no need to expose internal details to public. Upload details can be reflected in the method name:

  private class RestorableInputStream  {...}
  public Blob createBlobUsingRestorableInputStream(blobInfo, inputStream, options) {
     RestorableInputStream  restorable = new RestorableInputStream (inputStream);
   ...
  }

I personally do not see any benefits from adding such method. Do you?

@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/java-storage API. label Jul 10, 2020
@frankyn frankyn closed this as completed Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants