[merge] MVC progress bars, and indication of network traffic

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> This reworks the progress bar / ui infrastructure a bit so that we get
> an indication of network activity.
> 
> Previously, the progress bars shown by bzr rely on the library code that
> is for instance copying revisions updating a progress bar object.  We
> have a concept of stacking where one operation like fetching revisions
> can be composed of some lower-level tasks.  The good thing about this is
> that it gives us a chance to give an overall progress indication or
> prediction.  The drawbacks are that if the code in one place doesn't
> update the progress bar often enough, it just stays stuck - there is a
> case at the moment where it just shows '0/0' for an extended period.
> 
> This patch allows for transports to pass information back to the ui
> object to report on traffic coming in and out.
> 
> At the moment only the sftp transport does this, and only for a couple
> of read operations.  You can see it working with e.g. './bzr branch
> sftp://bazaar.launchpad.net/~bzr/bzr/trunk'.  I expect we'd only do this
> on network transports, including hpss.
> 
> I think the result is pretty good, it really gives a sense that
> something is happening and may also point out cases where we're either
> reading too much data or not filling the pipe.
> 
> In adding this I realized it wouldn't fit well with the current
> progressbar code, which has different classes for different types of
> display.  It seems to me what we want is really a model/view
> distinction, where most of the code updates a general model of a task
> happening, and then that's drawn appropriately for the UI.
> 
> There's a similar issue that I think we want to just choose the right UI
> object for the situation we're in, and then that should determine how
> the progress indicator is drawn.  So the new make_ui_for_terminal
> function does that.  I think this will help with GUIs too.  I have not
> checked this against bzr-gtk; I probably should.
> 
> This removes MissingProgressBarFinish which I think adds little compared
> to just an AssertionError.

I believe one thing it adds is that we don't get a full traceback, since
MissingProgressBarFinish is not an actual "failure".

If anything, we should turn that code into a warning rather than raising
an exception.

At a minimum:
@@ -147,7 +206,7 @@
     def return_pb(self, bar):
         """Return bar after its been used."""
         if bar is not self._stack[-1]:
- -            raise errors.MissingProgressBarFinish()
+            raise AssertionError()
         self._stack.pop()

^- You are raising a plain Assertion which doesn't give *any*
information about what is being asserted. So you could change this to:
raise AssertionError('last progress bar on stack was not properly finished')


> 
> There's more we could do here but this is getting long, so I'll ask for
> review now.  One of the main things to follow on would be to deprecate
> or just remove the older ProgressBar classes, and also look at
> simplifying the code which currently passes pb objects around.
> 
> 

At one part of the code you set "direction" but then you get rid of that
when you "show_transport_activity". Wouldn't it be nice to know that we
are downloading a lot during a small push? I suppose it doesn't help
users, but it may help developers.

The new layering seems fine, though I wonder about CLIUIFactory versus
TextUIFactory.

If only because the scripts and manual work I've done basically involved
manually setting ui.ui_factory = TextUIFactory(), and I was wondering
about preserving that.

Is it that CLIUI gets a progress bar with '\r', while Text just gets a
Dots progress bar? (Maybe I have that backwards)

Also, no tests for 'make_ui_for_terminal' ?

Anyway, I'd like to see this land early in a release cycle so we can get
a feel for it in bzr.dev before we make a release. If you feel it is
early enough in this release, go for it.

BB:tweak

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAklIaYAACgkQJdeBCYSNAANZ1wCfVdre2zFZzwuVPXYxIYAtxqei
qswAn1FWA39MpIuKREF1RvSEgqI2ZAme
=kgJb
-----END PGP SIGNATURE-----