[issue9244] multiprocessing.pool: Worker crashes if result can't be encoded

[Ask Solem]

Ask Solem <askh at opera.com> added the comment:

> To be clear, the errback change and the unpickleable result
> change are actually orthogonal, right?

Yes, it could be a separate issue. Jesse, do you think I should I open
up a separate issue for this?

> Why not add an error_callback for map_async as well?

That's a good idea!

> Any reason you chose to use a different internal name
> (errback versus error_callback)? It seems cleaner to me
> to be consistent about the name.

It was actually a mistake. The argument was ``errback`` before, so
it's just a leftover from the previous name.

> In general, I'm wary of nonessential whitespace changes...
> did you mean to include these?

Of course not.

> Using "assertTrue" seems misleading. "assertIsNotNone" is what
> really mean, right?  Although, I believe that's redundant,
> since presumably self.assertIsInstance(None, KeyError) will
> error out anyway (I haven't verified this).

bool(KeyError("foo")) is True and bool(None) is False, so it works either way. It could theoretically result in a false negative if
the exception class tested overrides __nonzero__, but that is unlikely
to happen as the target always returns KeyError anyway (and the test below ensures it) It's just a habit of mine, unless I really want to test for Noneness, I just use assertTrue, but I'm not against changing it to assertIsNotNone either.

> Under what circumstances would these be None?  (Perhaps you
> want wrapped.exc != 'None'?)  The initializer for
> MaybeEncodingError enforces the invariant that exc/value are strings
> right?
It's just to test that these are actually set to something.
Even an empty string passes with assertIsNone instead of assertTrue.
Maybe it's better to test the values set, but I didn't bother.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue9244>
_______________________________________