excessive warning for files of size 0

[retroj]

transfer_summarizer produces a warning for files of size zero:

Error calculating percentage 0/0, is foo empty?

My feeling is that this warning is excessive as it implies that a file of size 0 is some kind of problem. Sshkit's output would be a bit less noisy if it just special-cased the percentage for files of size zero as 100%.

[mattbrictson]

Thanks for the suggestion!

I'm curious, what is your use-case for uploading a zero-byte file? I think using touch would be preferable in this case, but I'd like to understand why you are running into this.

[retroj]

There is a convention with git of placing a zero byte file in a directory in order to ensure that the directory exists when there is no other version controlled content in it. It is just these harmless files (usually .gitkeep) that are occasionally here and there in my project that end up causing warnings during upload with sshkit.

[mattbrictson]

I see. I hesitate fixing this "bug" because an argument could be made that uploading an empty file is a mistake that deserves a warning.

It sounds like you are iterating through files in your project and uploading them using upload!. Could you work around the warning by testing each file with File.empty? before uploading it?

[retroj]

Reasonable, but it leads me to consider another case:

I'm uploading a directory of css files to my website that were generated with Sass. Since this directory contains only generated files, that is why I had an empty .gitkeep file in it. However, generated css files could sometimes be empty themselves, and in such cases, I would still want to upload those files - they are empty, but they are valid css files, and I would not want requests for them to 404.

Also unless there is something really weird going on, the only circumstance that can trigger this warning is a division by zero. The warning is basically reporting a divide-by-zero error, but the division operation isn't the important concern of transfer_summarizer - the transfer status is. The division operation gets us the transfer status, but must be interpreted in the case of a divisor of zero. In that case, the transfer status must be 100%.

Well I do love to go down little philosophical rabbit holes like this, and hope I haven't been a bother. I'll go along with whatever you decide. Cheers :-)

[mattbrictson]

@leehambley any objections to removing the upload warning for empty files? It looks like you were last to touch this code in cbc9064

[leehambley]

Uploading empty files sounds like a bad idea, and I don't buy the .gitignore reasoning, Ruby's dir goobbing should be used to select correct files to sync imho. Disabling a useful warning that would normally indicate an error condition (badly parsed tempates, etc) in order to get SCM artifacts onto the server strikes me as a step backwards, or at least sideways into a less safe tool. Am 22.02.2017 20:12 schrieb "Matt Brictson" <[email protected]>:

…

@leehambley <https://github.com/leehambley> any objections to removing the upload warning for empty files? It looks like you were last to touch this code in cbc9064 <cbc9064> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#391 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABKCOF9HnuF-rLrDQ_pwyusnpWHdCl-ks5rfIi2gaJpZM4MHvYO> .

[retroj]

Hello Lee,

Do you have thoughts on the use case of empty css files? Thank you,

[robd]

@retroj For the css case specifically, it feels like making a user download an empty css file (and possibly block the rendering of the web page until it finishes) is something we should try to avoid. Is dealing with empty SASS generated css files a common problem?

[retroj]

@robd I think that the typical case for an empty css file would be while developing a site, one might put files in place before writing their content. I don't think that is a problem or not-a-problem, but the question is, what is the right thing to do for an empty file when there is one, be it css or something else? Serve the empty file or 404? Those two responses have two different meanings, and could also trigger different web server behaviors in some circumstances. I lean toward treating empty files as ordinary, not warranting of any special callout.

At the risk of being long-winded, if I could resummarize: sshkit does currently upload empty files when told to - that is the correct, expected behavior. The questions are, 1) should capistrano users avoid uploading empty files to their websites, and 2) should sshkit's progress indicator report a division-by-zero warning when they do? I would answer, users should upload whatever they want, and there is no special reason for the progress indicator to report an internal detail like the division by zero that occurred when calculating the progress of a size 0 file because the progress of such a file is known to be 100%.

[robd]

@retroj Ah OK I see - I hadn't thought of that case. For me it still feels desirable to warn for the empty file since this would only be a temporary situation while the site is developed. If it turns out that I put some css in the file, then the warning goes away, if I don't use it then it's a reminder to delete it again, which personally I would find helpful.

In the more general case I tend to agree with Lee that an empty file is normally some kind of problem and probably not what the user wanted to upload. But I would be interested in understanding if there are people who are seeing this noise a lot - I can see that it's annoying to have the logs polluted. Are there any other cases apart from .gitkeep and placeholder css files which you are seeing?

One improvement would be to change the implementation to check explicitly for the empty case and warn with a clearer error in the empty file case File is empty rather than lumping it in with the normal nan error, and then we could remove the is foo empty? in the error case.

[retroj]

@robd I like your idea. The current warning is oddly ambiguous, and to my thinking, outside of the concern of a progress reporter.

[leehambley]

Alright, so to summarize, if we all agree?

• Progress reporter should not complain about division by zero...
• ... because we should raise an error when uploading empty files?

Was that the consensus?

[mattbrictson]

Sounds like consensus! Would anyone like to contribute a PR to implement the agreed behavior?