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

[Vincent Ladeuil]

>>>>> "Andrew" == Andrew Bennetts <andrew.bennetts at canonical.com> writes:

    Andrew> 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).

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

That wasn't and still isn't a major concern. Yet,
bzrlib.remote._translate_error isn't the clearer piece of code
I've seen. One drawback, for example, is that such a design make
it impossible for an outsider to find which smart verbs can raise
which exception (or even a single given exception). As long as we
don't need to find these relations too often, there is no need to
change it though.

One big pro is that unknown errors are handled, which may be more
complicated in an alternate design.

    >> 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).

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

Great.

<snip/>

    Andrew> -    def _translate_error(self, err, orig_path=None):
    Andrew> -        remote._translate_error(err, path=orig_path)
    Andrew> +    def _translate_error(self, err, relpath=None):
    Andrew> +        remote._translate_error(err, path=relpath)

I had a wtf here until I looked into
bzrlib.remote._translate_error :-)

So 'relpath' is the local idiom for transport, that's fine.
 
  Vincent