When passed a filename, download a report in chunks

[toofishes]

Description

Rather than pulling an entire 6-8 GB report into a string in memory to
immediately write it out to a file, use the built-in feature of Net::HTTP to
retrieve the result in chunks and write them out as they are read from the
remote host. This prevents receiving NoMemoryError issues when downloading
huge reports.

While here, also add Travis CI testing for Ruby 2.5.

Motivation and Context

We're hitting out of memory errors trying to download huge reports using this gem.

How Has This Been Tested?

We have a set of VCR-based tests that invoke this gem, and specifically the download method. I was able to use this version of the download method and see our test suite pass successfully.

Unfortunately, we don't have tests for this method in this gem itself. It probably makes sense to do some sort of VCR recording or cassette, which I could attempt if you think that makes sense. EDIT: tests now included.

Types of changes

• New feature (non-breaking change which adds functionality)

Checklist:

• I have updated the documentation accordingly (if changes are required).
• I have added tests to cover my changes.
• All new and existing tests passed.

[gschneider-r7]

Looks like you have to specify a basename for backwards compatibility: https://blog.bigbinary.com/2017/04/04/ruby-2-4-has-default-basename-for-tempfile-create.html

[toofishes]

Just pushed that change as I noticed the build had failed. All ruby versions green now.

[gschneider-r7]

Should there be some dummy data here to ensure there are enough chunks to iterate? Does vcr actually simulate that properly?

This might be getting into "are you just testing the standard library now" territory, and if the answer to that is yes it's not worth worrying about too much as we would catch problems here in our internal automated testing anyway.

[toofishes]

My gut is the latter; I also don't know if VCR would even emulate that the same way an actual Net::HTTP call would work, or if it is always going to return the data in the yaml file here as a single chunk.

[toofishes]

I just spent a few minutes diving into the VCR and WebMock source code. I didn't see any obvious way to get this to be a multipart or chunked response in the VCR cassette, so I think we're just going to have to rely on the standard library and/or internal testing here.

[toofishes]

I should point out that this is very similar to #248, but I attempted to leave the non-filename path alone as much as possible, and not add any block_given? logic.

[gschneider-r7]

This looks good so once I get a chance to test this myself I'll probably merge and release. Thanks for doing this since I completely forgot about the past attempt. 😓

[gschneider-r7]

Worked as expected with a 400 MB PDF report.

[gschneider-r7]

Version 7.2.1 is published to RubyGems.

[toofishes]

Thank you, appreciate you getting this reviewed and merged so quickly!

[gschneider-r7]

Just a note for anyone coming here later to find out if this change broke their scripts:

It seems like tempfile handling changed between Ruby 2.1.x and 2.2.x versions. If you use a tempfile and do not keep the file reference open after the report is downloaded it seems to get garbage collected immediately after the report completes (likely due to the scope change on the file block) which deletes the temp file from disk. As far as I can tell, when this happens, it means the tempfile is being used incorrectly (e.g. passing the path around instead of the file object). To avoid this just make sure any tempfile usage keeps the file object in scope until it's no longer needed.

For example, this won't work because the file won't exist when you try to open it:

report_file = Tempfile.new('report').path
nsc.download(report.uri, report_file)
File.open(report_file, 'rb') do |f|
  # do stuff with the file
end

Correct usage:

report_file = Tempfile.new('report')
nsc.download(report.uri, report_file) # you can also use report_file.path but it's redundant
File.open(report_file, 'rb') do |f|
  # do stuff with the file
end