Code review comment for lp:~gz/bzr/test_ssh_client_medium_eintr__read_bytes

Martin [gz] wrote:
[...]
> I urge you to read the mailing list thread if you've not already, it included
> a complete list of which interfaces are affected and how in the first post.
> The only tricky bit is indeed SmartSSHClientMedium but the best fix is already
> upstream.

I have read it, but will re-read. It's been a long, slightly convoluted
discussion!

> > I also changed the test to check for the existence of the EINTR symbol rather
> > than just checking the platform name. So now test should run on POSIX-like
> > platforms that don't say os.name == 'posix', yet might be vulnerable to this
> > bug.
>
> That is incorrect. EINTR *exists* on other platforms but this PC loser-ing
> issue is specific to unix.

But isn't it valid to test for correct behaviour in the face of EINTR on all
platforms that have EINTR?

[...]
> > I'm not totally satisfied with the shape of the new test, but I think it's
> > getting closer, and it does clearly pinpoint the bug. Thanks for your
> > persistence on this issue!
>
> Your changes keeps around an interface I'm trying to junk and is a little
> harder to reason about, but losing the sleeps is good. Please CC me on the
> review when you post it, as I've some specific comments.

Which interface is it that you're trying to junk?

I'll definitely CC you on the merge proposal.

> > We may to backport the eventual fix to 2.0, if it isn't too hairy. Thinking
> > of which, is there a bug number for this?
>
> Nope, given that there are about five different bugs with the same underlying
> cause, it seemed easier to take it to the mailing list so all the discussion
> stayed in one place.

I see what you mean. On the other hand, I have to search for and then read
through a multi-message thread with no clear summary. Neither a cluster of
bugs or a mailing list thread is entirely satisfying. Hmm.

Thanks for your comments!