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

#22042: signal.set_wakeup_fd(fd): set the fd to non-blocking mode

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 2 months ago by vstinner
Modified:
5 years ago
Reviewers:
cf.natali
CC:
AntoinePitrou, haypo, Charles-François Natali, devnull_psf.upfronthosting.co.za
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/test/test_signal.py View 1 2 3 chunks +25 lines, -0 lines 2 comments Download
Modules/signalmodule.c View 1 2 3 chunks +18 lines, -1 line 2 comments Download

Messages

Total messages: 2
Charles-François Natali
http://bugs.python.org/review/22042/diff/12537/Lib/test/test_signal.py File Lib/test/test_signal.py (right): http://bugs.python.org/review/22042/diff/12537/Lib/test/test_signal.py#newcode310 Lib/test/test_signal.py:310: self.assertEqual(str(cm.exception), Would assertRaisesRegexp work here? https://docs.python.org/2/library/unittest.html#unittest.TestCase.assertRaisesRegexp http://bugs.python.org/review/22042/diff/12537/Lib/test/test_signal.py#newcode316 Lib/test/test_signal.py:316: signal.set_wakeup_fd(-1) ...
5 years, 1 month ago #1
victor.stinner_gmail.com
5 years ago #2
Thanks for the review, I commited the modified patch to include the file
descriptor number in the exception message.

http://bugs.python.org/review/22042/diff/12537/Modules/signalmodule.c
File Modules/signalmodule.c (right):

http://bugs.python.org/review/22042/diff/12537/Modules/signalmodule.c#newcode602
Modules/signalmodule.c:602: "the fd must be in non-blocking mode");
On 2014/07/31 20:04:47, Charles-François Natali wrote:
> It's probably a bit overkill, but I like to add information to error messages,
> in this case the FD number. It can make debugging easier.
> You'd need to update the corresponding test.

Done.
Sign in to reply to this message.

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