github-api icon indicating copy to clipboard operation
github-api copied to clipboard

Please provide streaming for upload large content

Open Vampire opened this issue 3 years ago • 7 comments

I have a Gradle build where I build a Docker image tar file which has a size of 166 MiB. I use the GitHub publish Gradle plugin and tried to upload this Docker image as asset to the release. This consistently resulted in an OutOfMemoryError.

The GitHub publish Gradle plugin under the hood uses this library. And I also easily reproduced it stand-alone. In version 1.1xx, you use an HttpsUrlConnection without setting it to fixed-length streaming or chunking mode and thus the JRE uses a byte-array output stream to colllect the whole body in RAM to determine the size up-front. In version 1.3xx many things changed, but now you use your GitHubRequest.Builder#with(java.io.InputStream) method to load the whole file into RAM using IOUtils.toByteArray.

So at different places both load the whole file into RAM instead of properly streaming it. Please support at least giving the size to methods like uploadAsset so that the size can be set as HTTP header but the content be streamed, or if GitHub supports it maybe even using chunked mode additionally if no explicit size was given.

The problem can pretty easily be reproduced using:

github
    .getRepository("my/repository")
    .listReleases()
    .iterator()
    .next()
    .uploadAsset("foo.tar.gz", new FileInputStream("file/that/is/too/large/for/RAM"), "application/octet-stream")

Vampire avatar May 09 '22 10:05 Vampire

see #1405.

bitwiseman avatar May 15 '22 07:05 bitwiseman

But that's a different issue, isn't it? There it is about receiving something and according to you it needs significant complexity to be fixed. Here it is about sending an artifact where you (at least in the 1.1xx version) just need to enable streaming mode and eventually let the user specify the file size or the file instead of just a stream. Should be pretty trivial to fix this one, shouldn't it?

Vampire avatar May 15 '22 11:05 Vampire

@Vampire Sorry for not responding sooner. If you are able to create a PR enabling the behavior you want, I'm open to discussing.

bitwiseman avatar Aug 18 '22 17:08 bitwiseman

I can see how they are different path but the underlying framework currently treats them the same. I'll keep this open separately as I can see ways that the upload issue could be resolved separately from the download.

bitwiseman avatar Aug 18 '22 18:08 bitwiseman