Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(130)

#5001: Remove assertion-based checking in multiprocessing

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 11 months ago by jnoller
Modified:
1 month, 2 weeks ago
Reviewers:
pitrou, ezio.melotti, allen.w.smith1
CC:
AntoinePitrou, jnoller_gmail.com, jesstess, sbt, vladris, berkerpeksag, Winterflower, drallensmith
Visibility:
Public.

Patch Set 1 #

Total comments: 28

Patch Set 2 #

Total comments: 11
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/multiprocessing/connection.py View 3 chunks +12 lines, -8 lines 2 comments Download
Lib/multiprocessing/forking.py View 1 3 chunks +8 lines, -3 lines 4 comments Download
Lib/multiprocessing/heap.py View 1 5 chunks +10 lines, -5 lines 1 comment Download
Lib/multiprocessing/managers.py View 1 10 chunks +30 lines, -12 lines 3 comments Download
Lib/multiprocessing/pool.py View 1 10 chunks +32 lines, -13 lines 1 comment Download
Lib/multiprocessing/process.py View 1 8 chunks +20 lines, -13 lines 0 comments Download
Lib/multiprocessing/queues.py View 1 3 chunks +6 lines, -3 lines 0 comments Download
Lib/multiprocessing/sharedctypes.py View 1 1 chunk +2 lines, -1 line 0 comments Download
Lib/multiprocessing/synchronize.py View 1 3 chunks +14 lines, -8 lines 0 comments Download
Lib/multiprocessing/util.py View 1 1 chunk +5 lines, -3 lines 0 comments Download

Messages

Total messages: 3
AntoinePitrou
Thank you for the patch. I've attached a bunch of suggestions. Another issue is that ...
5 years, 11 months ago #1
ezio.melotti
I added a few comments as well. %r doesn't need quotes usually, especially with bytes/str ...
5 years, 11 months ago #2
drallensmith
1 month, 2 weeks ago #3
Given that it's going to be practically identical for '#UNSERIALIZABLE',
something like "elif kind in ('#TRACEBACK', '#UNSERIALIZABLE') seems reasonable,
with a later differentiation into types if the assertion does not get invoked.

https://bugs.python.org/review/5001/diff/3407/Lib/multiprocessing/managers.py
File Lib/multiprocessing/managers.py (right):

https://bugs.python.org/review/5001/diff/3407/Lib/multiprocessing/managers.py...
Lib/multiprocessing/managers.py:114: assert isinstance(result, str)
Might I suggest something like AssertionError("Result {0!r} (kind {1}) type is
{2}, not str".format(result, kind, type(result))? I am seeing this error and it
would be nice if it were more informative...

https://bugs.python.org/review/5001/diff/3407/Lib/multiprocessing/managers.py...
Lib/multiprocessing/managers.py:117: assert isinstance(result, str)
See re line 114.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7