[issue9244] multiprocessing.pool: Worker crashes if result can't be encoded
report at bugs.python.org
Wed Jul 14 14:34:05 CEST 2010
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
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>
More information about the Python-bugs-list