Opened 4 years ago

Closed 4 years 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
branch-diff, diff-cov, branch-cov, buildbot
Author: therve, vperic


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/ 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 4 years ago.

Download all attachments as: .zip

Change History (12)

Changed 4 years ago by vperic

comment:1 Changed 4 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 4 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 4 years ago by itamar

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

comment:5 Changed 4 years ago by thijs

  • Cc thijs added

comment:6 Changed 4 years ago by therve

  • Owner changed from vperic to therve

comment:7 Changed 4 years 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 4 years ago by therve

  • Keywords review added
  • Owner therve deleted

comment:9 Changed 4 years ago by exarkun

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

comment:10 Changed 4 years 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 (<>) 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 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.