Patchwork [2,of,2,RESEND] lfs: emit a status message to indicate how many blobs were uploaded

login
register
mail settings
Submitter Matt Harbison
Date Feb. 2, 2018, 1:29 a.m.
Message ID <dac55f22cd775a8d3a21.1517534969@Envy>
Download mbox | patch
Permalink /patch/27211/
State Accepted
Headers show

Comments

Matt Harbison - Feb. 2, 2018, 1:29 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1517281788 18000
#      Mon Jan 29 22:09:48 2018 -0500
# Node ID dac55f22cd775a8d3a213e9b6716f03e49e1e765
# Parent  0088fad7fa84a6ec668682e137273e252bc7441c
lfs: emit a status message to indicate how many blobs were uploaded

Previously, there was a progress bar indicating the byte count, but then it
disappeared once the transfer was done.  Having that value stay on the screen
seems useful.  Downloads are done one at a time, so hold off on that until they
can be coalesced, to avoid a series of lines being printed.  (I don't have any
great ideas on how to do that.  It would be a shame to have to wrap a bunch of
read commands to be able to do this.)

I'm not sure if the 'lfs:' prefix is the right thing to do here.  The others in
the test are verbose/debug messages, so in the normal case, this is the only
line that's prefixed.
Yuya Nishihara - Feb. 2, 2018, 11:07 a.m.
On Thu, 01 Feb 2018 20:29:29 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1517281788 18000
> #      Mon Jan 29 22:09:48 2018 -0500
> # Node ID dac55f22cd775a8d3a213e9b6716f03e49e1e765
> # Parent  0088fad7fa84a6ec668682e137273e252bc7441c
> lfs: emit a status message to indicate how many blobs were uploaded

Queued, thanks.

> Previously, there was a progress bar indicating the byte count, but then it
> disappeared once the transfer was done.  Having that value stay on the screen
> seems useful.  Downloads are done one at a time, so hold off on that until they
> can be coalesced, to avoid a series of lines being printed.  (I don't have any
> great ideas on how to do that.  It would be a shame to have to wrap a bunch of
> read commands to be able to do this.)

No idea other than keeping stats in lfsremoteblobstore and hook a dispatcher
function or repo.close() to report it, which doesn't sound nice.
Matt Harbison - Feb. 3, 2018, 7:19 a.m.
On Fri, 02 Feb 2018 06:07:20 -0500, Yuya Nishihara <yuya@tcha.org> wrote:

> On Thu, 01 Feb 2018 20:29:29 -0500, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1517281788 18000
>> #      Mon Jan 29 22:09:48 2018 -0500
>> # Node ID dac55f22cd775a8d3a213e9b6716f03e49e1e765
>> # Parent  0088fad7fa84a6ec668682e137273e252bc7441c
>> lfs: emit a status message to indicate how many blobs were uploaded
>
> Queued, thanks.
>
>> Previously, there was a progress bar indicating the byte count, but  
>> then it
>> disappeared once the transfer was done.  Having that value stay on the  
>> screen
>> seems useful.  Downloads are done one at a time, so hold off on that  
>> until they
>> can be coalesced, to avoid a series of lines being printed.  (I don't  
>> have any
>> great ideas on how to do that.  It would be a shame to have to wrap a  
>> bunch of
>> read commands to be able to do this.)
>
> No idea other than keeping stats in lfsremoteblobstore and hook a  
> dispatcher
> function or repo.close() to report it, which doesn't sound nice.

The other benefit would be to avoid 2 round trips per LFS file on bulk  
commands like update, merge, archive, bundle(?), export, and (sadly)  
verify.  It would probably be useful to have a resolved matcher for things  
like archive, cat, diff, and revert, which usually deal with a subset of  
the files.  It doesn't sound like either of those ideas would handle these  
cases.

We could probably handle update, clone/share, and merge by wrapping  
mergemod.update().  But I doubt there are other common hook points like  
this.

Patch

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -366,12 +366,23 @@ 
             oids = transfer(sorted(objects, key=lambda o: o.get('oid')))
 
         processed = 0
+        blobs = 0
         for _one, oid in oids:
             processed += sizes[oid]
+            blobs += 1
             self.ui.progress(topic, processed, total=total)
             self.ui.note(_('lfs: processed: %s\n') % oid)
         self.ui.progress(topic, pos=None, total=total)
 
+        if blobs > 0:
+            if action == 'upload':
+                self.ui.status(_('lfs: uploaded %d files (%s)\n')
+                               % (blobs, util.bytecount(processed)))
+            # TODO: coalesce the download requests, and comment this in
+            #elif action == 'download':
+            #    self.ui.status(_('lfs: downloaded %d files (%s)\n')
+            #                   % (blobs, util.bytecount(processed)))
+
     def __del__(self):
         # copied from mercurial/httppeer.py
         urlopener = getattr(self, 'urlopener', None)
diff --git a/tests/test-lfs-test-server.t b/tests/test-lfs-test-server.t
--- a/tests/test-lfs-test-server.t
+++ b/tests/test-lfs-test-server.t
@@ -48,6 +48,7 @@ 
   searching for changes
   lfs: uploading 31cf46fbc4ecd458a0943c5b4881f1f5a6dd36c53d6167d5b69ac45149b38e5b (12 bytes)
   lfs: processed: 31cf46fbc4ecd458a0943c5b4881f1f5a6dd36c53d6167d5b69ac45149b38e5b
+  lfs: uploaded 1 files (12 bytes)
   1 changesets found
   uncompressed size of bundle content:
        * (changelog) (glob)
@@ -86,6 +87,7 @@ 
   lfs: processed: 37a65ab78d5ecda767e8622c248b5dbff1e68b1678ab0e730d5eb8601ec8ad19
   lfs: uploading d11e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998 (19 bytes)
   lfs: processed: d11e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998
+  lfs: uploaded 2 files (39 bytes)
   1 changesets found
   uncompressed size of bundle content:
   adding changesets