Opened 2 years ago

Closed 18 months ago

#5847 task closed fixed (fixed)

Remove twisted/python/_epoll.c

Reported by: vperic Owned by: therve
Priority: normal Milestone:
Component: core Keywords:
Cc: thijs Branch: branches/remove-epoll-5847-2
(diff, github, buildbot, log)
Author: therve, vperic Launchpad Bug:

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 vperic 2 years ago.

Download all attachments as: .zip

Change History (12)

Changed 2 years ago by vperic

comment:1 Changed 2 years ago by itamar

  • Keywords review removed
  • Owner set to vperic

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 2 years ago by vperic

  • Author set to vperic
  • Branch set to branches/remove-_epoll-5847

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

comment:4 Changed 2 years ago by itamar

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

comment:5 Changed 20 months ago by thijs

  • Cc thijs added

comment:6 Changed 18 months ago by therve

  • Owner changed from vperic to therve

comment:7 Changed 18 months ago by therve

  • Author changed from vperic to therve, vperic
  • Branch changed from branches/remove-_epoll-5847 to branches/remove-epoll-5847-2

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

comment:8 Changed 18 months ago by therve

  • Keywords review added
  • Owner therve deleted

comment:9 Changed 18 months ago by exarkun

  • Owner set to exarkun
  • Status changed from new to assigned

comment:10 Changed 18 months ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to therve
  • Status changed from assigned to new

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 18 months ago by therve

  • Resolution set to fixed
  • Status changed from new to closed

(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.