Repository navigation
Send real stats in the progress callback instead of % - #100
Conversation
The format is now:
{event} {index}/{totalFiles} {sentBytes}/{totalBytes} {filename}
this flexibility will let us change formats potentially
|
Okay, this no longer sends the initial log line with 0 bytes. Also, the |
|
👍 |
|
@joshaber: Is that a 👍 that this works for you? |
|
Oh sorry, that was 👍 on the idea. Just tested it out. If I say |
|
@joshaber Why don't you pass a path instead of a file URL? |
|
@jspahrsummers See above:
|
|
If the |
Send real stats in the progress callback instead of %
As described in a previous commit in this PR, several of our Git LFS commands may write progress log messages with a negative value in the denominator of the fraction representing the amount of data downloaded so far for an individual object. Specifically, the "git lfs fetch", "git lfs pull", and "git lfs smudge" commands will write such log messages when a progress log is enabled with the GIT_LFS_PROGRESS environment variable and the response to an HTTP object download request is content-encoded and therefore lacks a Content-Length header. As described in a previous commit in this PR, for historical reasons progress log messages are written either by the anonymous callback function created by the CopyCallbackFile() method of the GitFilter structure in our "lfs" package, or by the logBytes() method of the Meter structure in our "tq" package. However, the format of the messages is identical regardless of which function or method generates them. When the logBytes() method was first introduced in commit 710862b of PR #476, it duplicated the format of the progress log messages generated by the callback function created by the CopyCallbackFile() method. The current progress log message format in that method dates from commit 36fa23d of PR #100, but the original version of the method and its callback function themselves were added in commit fd41906 of PR #97. While we likely should refactor the logBytes() method and the CopyCallbackFile() method's callback function to share a common implementation, that change falls outside of the scope of this PR, so we defer it to a future PR. However, one of the changes we made in a prior commit in this PR now ensures that if an object's total size is initially unknown, when an EOF condition is encountered while reading the object's data, then a final progress log message is guaranteed to be written with the total size of the object represented as equal to the actual number of bytes that were read. Further, this final progress log entry will be output regardless of whether the CopyCallbackFile() method's callback function or the logBytes() method is being used to write messages to the log file. While this change affects the content of the last progress log entry written for a given object, until the EOF condition is reached, intermediate log messages may be generated which still have a negative value in the denominator of the fraction representing the amount of the object's data processed so far. These negative values occur because the length of the data to be read is initially unknown. At present, the only circumstance in which this is possible is when the HTTP response to an object download request lacks a Content-Length header, as will be the case when the response body is gzip-encoded, as we noted in the following comment on PR #6011: #6011 (comment) PR #6011 resolves a bug in our "git lfs filter-progress" and "git lfs clean" commands whereby they can pass an incorrect "totalSize" parameter to the CopyWithCallback() function. The only solution for this problem is for the commands to simply not attempt to determine the total size of an object. As a consequence, we expect that our "git lfs filter-progress" and "git lfs clean" commands will never know in advance the length of the object data they are reading from Git, and it will become a more common occurrence for the CopyCallbackFile() method's callback function and the logBytes() method to receive a "total" parameter with a negative value. We therefore now refine the format of the progress log when the total size of an object is unknown by writing a series of question mark characters ("?") in place of the denominator of the fraction which represents the amount of the object's data processed so far. We adjust the number of these question mark characters to match the length of the numerator, e.g., "123/???" and "12345/?????". To avoid implementing the necessary string formatting logic in both the CopyCallbackFile() method's callback function and the logBytes() method, we introduce a new FormatFraction() function to our "tools" package and then substitute its output in place of the appropriate fraction field in the log messages. We then add a TestFormatFraction() Go test function which verifies that the FormatFraction() function generates the expected output under a range of conditions, including when the number of bytes read so far exceeds the value provided as the total size of an object, and when the total size of an object is unknown and should be represented using various numbers of question mark characters. Finally, we update the comments in several of the tests we added to the "t/t-progress.sh" shell script in a prior commit of this PR, and we revise the "initial total unknown" test case for our TestCopyCallbackFileThrottle() test function. In that test case, we update the expected contents of the progress log because the intermediate entry which precedes the final entry should now have a series of question mark characters rather than a "-1" value in the denominator of the fraction which represents the amount of the object's data processed so far.


The format is now:
One caveat: The initial entry will have 0/0 sent/total bytes. This is because we don't always know the size ahead of time. Should we leave that first message out then?
/cc @joshaber