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

download! method doesn't close file #518

Open
roooneey opened this issue Jul 20, 2023 · 1 comment
Open

download! method doesn't close file #518

roooneey opened this issue Jul 20, 2023 · 1 comment

Comments

@roooneey
Copy link

which causes hard-to-debug errors when you download many files in a row and try to read the file contents immediately after downloading. It works most of the time, but at some point a downloaded file will not be flushed to disk completely when you try to read the file.

Reading only the partially written file from disk will, of course, result in corrupted file contents. I was able to avoid this behavior by explicitly closing the downloaded file before reading it again. But this was not very intuitive, so please consider closing the file within the download method.

tmp_file = Tempfile.new(file_name, tmp_dir)
src = File.join(dir, file_name)
          
download! src, tmp_file, log_percent: -1

# it's important to close the tmp file, otherwise data might not be written 
# to disk already, when we try to read the file again
tmp_file.close 

begin
   File.read tmp_file
ensure
   tmp_file.close
   tmp_file.unlink
end

@mattbrictson
Copy link
Member

mattbrictson commented Jul 23, 2023

@roooneey thanks for the report!

I agree, this behavior is a bit unintuitive. You would think that download! with a string argument would work similar to Ruby's File.write, which opens a file, writes to it, and closes it.

However in this case SSHKit is just delegating to download! from the net-scp gem, so we inherit its file handling behavior.

I hesitate to a make change here, because it would introduce an inconsistency between SSHKit's download! and the net-scp method that it aliases.

Maybe you could report this issue upstream, to net-scp and see if they would be willing to fix the behavior there?

https://github.com/net-ssh/net-scp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants