Code review comment for lp:~spiv/bzr/lockcontention-bugs

Revision history for this message
Andrew Bennetts (spiv) wrote :

This patch does a couple of things:

 - fixes the generic server-side error translation function so that it can
   serialise real LockContention errors, which now tend to have their 'lock'
   attribute be a LockDir rather than a str.
 - simplifies the Branch.lock_write HPSS request implementation so that it uses
   that generic translation rather than its own, less informative one.
 - improves the client-side translation so that it will optionally use the lock
   repr returned over the wire if one is given. (Existing clients will
   unconditionally say "(remote lock)" even if the server gives more info.)
 - changes LockDir's repr and error message text to use
   transport.external_url(), where possible — especially over HPSS this is much
   more likely to be useful. It will still use .base if external_url raises
   InProcessTransport, as some url, even a memory:/// one, is probably more
   helpful than none at all.

Hmm, perhaps I should extend test_errors to explicitly construct LockContention
with both a LockDir (as happens in process) and with a str (as happens when
constructing one from an HPSS error response)...

The change to use external_url *might* help fix the Launchpad-bazaar bug where
ephemeral and internal lp-1234:/// URLs are shown to users of the codehosting
service, rather than something useful.

-Andrew.

« Back to merge proposal