[MERGE] Fix test suite regression on OSX in TestReadMergeableFromUrl.test_smart_server_connection_reset.
Andrew Bennetts
andrew at canonical.com
Tue Sep 30 09:09:08 BST 2008
Vincent Ladeuil wrote:
> revno 3703.2.1 'Allow ConnectionReset to propagate from
> read_mergeable_from_url' added a test that fails on OSX.
>
> It fails sometimes only as is often the case when sockets are
> involved (python on OSX often produces different execution orders
> when several threads are involved) which made it a bit hard to
> reproduce and may explain why it doesn't seem to occur on linux.
>
> The attached patch fixed the issue but I'd appreciate if Andrew
> can review it since it may not be the right approach (or more
> precisely I suspect there is a better one or more complete one
> but I can't put my finger on it).
bb:resubmit
Out of interest, what does the stacktrace for the failing test look
like? I think I can guess, but it'd be good to avoid unnecessary
guesswork :)
I don't think this is the right fix. This seems to be the wrong layer
to be catching socket.errors. I think we really need to make the
various medium implementations present a consistent interface as far as
possible, or we'll inevitably have to put this sort of band-aid in other
places. It's the medium that should be translating the low-level
details of delivering and receiving bytes into a simple
read_bytes/accept_bytes interface. socket.errors should be an
implementation detail of one particular medium.
So I think the relevant SmartClientMedium (in this case
SmartTCPClientMedium I guess) should be catching this error and
translating it into something consistent for the caller, which at the
moment would be to return ''. So that would mean updating
SmartTCPClientMedium._read_bytes. (Or
SmartClientHTTPMediumRequest._read_bytes?)
I'm starting to think that the right thing to do would be to make the
medium become responsible for raising ConnectionReset, rather than the
caller of read_bytes. This is probably a deeper change than necessary
for fixing this particular bug, so I wouldn't demand it happen
immediately. But I think we should be moving in this direction.
-Andrew.
More information about the bazaar
mailing list