[MERGE][#299254] Fix RemoteTransport's translation of errors involving paths; it wasn't passing orig_path to _translate_error.

[Andrew Bennetts]

Vincent Ladeuil wrote:
> Vincent Ladeuil has voted approve.
> Status is now: Approved
> Comment:
> I'm not a big fan of error translation factorization because it makes it  
> hard to refine diagnosis under some circumstances. But since this patch  
> doesn't make that part worse, I think it should go in. And in that  
> particular case, having a single point to translate the errors may be  
> more appropriate than usual (since a serialization is involved anyway).

There's nothing stopping an individual callsite from handling the error
translation manually if it has special needs.  So far that hasn't been
necessary.  And we definitely want to re-use the code that does the
deserialisation, repeating that logic is needlessly error-prone.

> I also find a bit strange (not to say it violates some abstraction) that  
> 'orig_path' seems to pop out of nowhere but at least the comment  
> explains that.
>
> In a followup patch, you may want to review the other calls to  
> _translate_error to make them more consistent (_readv for example  
> doesn't pass relpath as a parameter and the parameter name itself is a  
> bit unclear since path and orig_path are used in different places).

I've now fixed the other calls in bzrlib/transport/remote.py.

John Arbash Meinel wrote:
[...]
> Just as a follow up, why call it "orig_path" rather than "relpath" like
> all of the transport functions call it.

No good reason, it was just the name that occurred to me.  I've changed it to
relpath.

Seeing as Vincent's vote was approve and John also thought this was fine,
I've sent my fix to PQM.  For the curious, here's an incremental diff of the
changes since the initial patch:

=== modified file 'bzrlib/transport/remote.py'
--- bzrlib/transport/remote.py	2008-11-20 06:17:07 +0000
+++ bzrlib/transport/remote.py	2008-11-21 00:23:54 +0000
@@ -174,7 +174,7 @@
         except errors.ErrorFromSmartServer, err:
             # The first argument, if present, is always a path.
             if args:
-                context = {'orig_path': args[0]}
+                context = {'relpath': args[0]}
             else:
                 context = {}
             self._translate_error(err, **context)
@@ -184,7 +184,12 @@
         try:
             return self._client.call_with_body_bytes(method, args, body)
         except errors.ErrorFromSmartServer, err:
-            self._translate_error(err)
+            # The first argument, if present, is always a path.
+            if args:
+                context = {'relpath': args[0]}
+            else:
+                context = {}
+            self._translate_error(err, **context)
 
     def has(self, relpath):
         """Indicate whether a remote file of the given name exists or not.
@@ -352,7 +357,7 @@
                     [(c.start, c.length) for c in cur_request])
                 resp, response_handler = result
             except errors.ErrorFromSmartServer, err:
-                self._translate_error(err)
+                self._translate_error(err, relpath)
 
             if resp[0] != 'readv':
                 # This should raise an exception
@@ -416,8 +421,8 @@
         if resp[0] != 'ok':
             raise errors.UnexpectedSmartServerResponse(resp)
         
-    def _translate_error(self, err, orig_path=None):
-        remote._translate_error(err, path=orig_path)
+    def _translate_error(self, err, relpath=None):
+        remote._translate_error(err, path=relpath)
 
     def disconnect(self):
         self.get_smart_medium().disconnect()

-Andrew.