Updated to handle download large files. #14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When downloading files, the current code reads the response in memory before write the file to disk. Large files could cause the client to crash with OutOfMemoryException. Updated the client to stream the response directly to disk.
Black Duck Security ReportMerging #14 into master will not change security risk. Added ComponentsClean: 101 |
risdenk
requested changes
May 25, 2018
| { | ||
| return await ReadStream(fileStream, path, offset, length, buffersize); | ||
| } | ||
| return await ReadStream(filePath, path, offset, length, buffersize); |
There was a problem hiding this comment.
Please don't make this change. This is designed to stream to the file. You basically do the same thing with File.Open later for the stream to write to.
| } | ||
|
|
||
| /// <summary> | ||
| /// https://hadoop.apache.org/docs/r2.8.0/hadoop-project-dist/hadoop-hdfs/WebHDFS.html#Open_and_Read_a_File | ||
| /// Note: You are responsible for closing the stream you send in. | ||
| /// </summary> | ||
| public async Task<bool> ReadStream( | ||
| Stream stream, | ||
| string filePath, |
There was a problem hiding this comment.
Please leave this as Stream stream. This breaks the public contract of the method.
| var response2 = await _httpClient.GetAsync(response.Headers.Location); | ||
| response2.EnsureSuccessStatusCode(); | ||
| if (response2.IsSuccessStatusCode) | ||
| var response2 = await _httpClient.GetAsync(response.Headers.Location, HttpCompletionOption.ResponseHeadersRead); |
There was a problem hiding this comment.
I think the logic here can be cleaned up to accomplish the same thing without moving the File stream around.
|
This PR should fix issue #13 |
|
@zhusulai, are you still working on this PR? Looks like good feature to have. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When downloading files, the current code reads the response in memory before write the file to disk. Large files could cause the client to crash with OutOfMemoryException. Updated the client to stream the response directly to disk.