[MERGE] Fix test suite regression on OSX in TestReadMergeableFromUrl.test_smart_server_connection_reset.
Andrew Bennetts
andrew at canonical.com
Wed Oct 1 08:13:33 BST 2008
Vincent Ladeuil wrote:
[...]
> Andrew> Out of interest, what does the stacktrace for the failing test look
> Andrew> like? I think I can guess, but it'd be good to avoid unnecessary
> Andrew> guesswork :)
>
> Sure:
[...]
Thanks.
[...]
> Andrew> So I think the relevant SmartClientMedium (in this
> Andrew> case SmartTCPClientMedium I guess) should be catching
> Andrew> this error and translating it into something
> Andrew> consistent for the caller, which at the moment would
> Andrew> be to return ''. So that would mean updating
> Andrew> SmartTCPClientMedium._read_bytes. (Or
> Andrew> SmartClientHTTPMediumRequest._read_bytes?)
>
> Done in SmartTCPClientMedium. Which leads me to the forgotten
> question: in what circumstances is the smart involved in reading
> a *bundle* ?
When reading (or attempting to read) a bundle over a transport that
claims to have a smart medium. E.g. doing “bzr merge bzr://...” or “bzr
pull http://...” will attempt to read a bundle as their first network
operation.
> Andrew> I'm starting to think that the right thing to do
> Andrew> would be to make the medium become responsible for
> Andrew> raising ConnectionReset, rather than the caller of
> Andrew> read_bytes. This is probably a deeper change than
> Andrew> necessary for fixing this particular bug, so I
> Andrew> wouldn't demand it happen immediately. But I think
> Andrew> we should be moving in this direction.
>
> And how far should we go there ? 'ConnectionReset', as far as
> hpss is concerned can be viewed as a transient error, should we
> reconnect automatically (even harder and higher in the stack I
> guess) ?
I don't think ConnectionReset should be considered transient. If the
connection has been refused, or interrupted, then usually we ought to
stop and tell the user about it.
> >>>>> "Andrew" == Andrew Bennetts <andrew at canonical.com> writes:
>
> Andrew> Vincent Ladeuil wrote:
> Andrew> [...]
> >> + if len(e.args) and e.args[0] is errno.ECONNRESET:
>
> Andrew> In addition to my other comments, don't use “is” to
> Andrew> compare these integers, just use “==”. It'll
> Andrew> probably work all the time with all current
> Andrew> implementations of CPython, but we shouldn't be
> Andrew> relying on that.
>
> I've been bitten by that in another project (I used 'is' here by
> copy/paste, I should have known better :) and I was wondering:
> using 'is' seems to be an optimized path when both strings are
> interned, but '==' should knows that and use it, so is it really
> worth trying to use 'is' ? And if yes, in what contexts ?
“is” is useful in two cases that I can think of:
* when you really do want to know if one object is the same instance
as another. Use cases are things like implementing object
serialisers like pickle, or maybe caching/memoising mechanisms, or
debug tools.
* when being as fast as possible is important, and you are *sure* that
“is” will give the correct answer in this case.
-Andrew.
More information about the bazaar
mailing list