Opened 4 years ago

Closed 4 years ago

#5847 task closed fixed (fixed)

Remove twisted/python/_epoll.c

Reported by: Vladimir Perić Owned by: therve
Priority: normal Milestone:
Component: core Keywords:
Cc: Thijs Triemstra Branch: branches/remove-epoll-5847-2
branch-diff, diff-cov, branch-cov, buildbot
Author: therve, vperic

Description

Our own epoll wrapper is not built on Python2.6+. As older Python versions are no longer supported, we can remove the module and instead use the implementation in the official library.

Attched patch removes mentions of it in twisted/topfiles/setup.py and a related private helper from twisted.python.dist. The _epoll module itself (t.p._epoll.{c,pyx}) and it's tests (t.test.test_epoll) have also been removed. Finally, t.internet.epollreactor has been changed to directly refer to the select module (rather than calling it _epoll) and a conditional import was removed.

Attachments (1)

remove-epoll.patch (156.4 KB) - added by Vladimir Perić 4 years ago.

Download all attachments as: .zip

Change History (12)

Changed 4 years ago by Vladimir Perić

Attachment: remove-epoll.patch added

comment:1 Changed 4 years ago by Itamar Turner-Trauring

Keywords: review removed
Owner: set to Vladimir Perić

Looks good; merge with a news file (a .misc is fine, since this change is not user visible in any way), and as usual only if buildbot passes on all platforms.

comment:2 Changed 4 years ago by Vladimir Perić

Author: vperic
Branch: branches/remove-_epoll-5847

(In [35187]) Create branch remove-_epoll-5847

comment:3 Changed 4 years ago by Vladimir Perić

comment:4 Changed 4 years ago by Itamar Turner-Trauring

There's test failure on FreeBSD and some of the other non-Linux platforms.

comment:5 Changed 4 years ago by Thijs Triemstra

Cc: Thijs Triemstra added

comment:6 Changed 4 years ago by therve

Owner: changed from Vladimir Perić to therve

comment:7 Changed 4 years ago by therve

Author: vperictherve, vperic
Branch: branches/remove-_epoll-5847branches/remove-epoll-5847-2

(In [37427]) Branching to 'remove-epoll-5847-2'

comment:8 Changed 4 years ago by therve

Keywords: review added
Owner: therve deleted

comment:9 Changed 4 years ago by Jean-Paul Calderone

Owner: set to Jean-Paul Calderone
Status: newassigned

comment:10 Changed 4 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: changed from Jean-Paul Calderone to therve
Status: assignednew

Thanks. I'm slightly concerned that the stdlib-based epoll implementation is broken under load (<http://twistedmatrix.com/pipermail/twisted-python/2013-March/026622.html>) but I guess it's easy to revert this change later if it turns out we want to restore our version of the bindings for some reason.

Apart from that, the builds look good. The news file is missing, though. Please fix that and merge.

comment:11 Changed 4 years ago by therve

Resolution: fixed
Status: newclosed

(In [37439]) Merge remove-epoll-5847-2

Authors: vperic, therve Reviewers: exarkun, itamar Fixes: #5847

Remove custom epoll bindings, the epoll reactor using stdlib version since python 2.6.

Note: See TracTickets for help on using tickets.