Opened 3 years ago

Closed 3 years ago

#5088 regression closed fixed (fixed)

cfreactor cannot be imported

Reported by: exarkun Owned by: exarkun
Priority: highest Milestone: Twisted-11.1
Component: core Keywords:
Cc: Branch: branches/cfreactor-import-5088
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

>>> from twisted.internet import cfreactor
/System/Library/Frameworks/Python.framework/Versions/2.6/Extras/lib/python/zope/__init__.py:1: UserWarning: Module twisted was already imported from twisted/__init__.pyc, but /System/Library/Frameworks/Python.framework/Versions/2.6/Extras/lib/python is being added to sys.path
  __import__('pkg_resources').declare_namespace(__name__)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/exarkun/Projects/Twisted/trunk/twisted/internet/cfreactor.py", line 24, in <module>
    from twisted.internet.selectreactor import _NO_FILENO, _NO_FILEDESC
ImportError: cannot import name _NO_FILENO

Change History (12)

comment:1 Changed 3 years ago by glyph

Looks like #4539 is the culprit. Why don't we have a failing buildbot?

comment:2 follow-up: Changed 3 years ago by exarkun

Because ImportError gets turned into a skip automatically for the reactor builder tests.

comment:3 in reply to: ↑ 2 Changed 3 years ago by glyph

I guess I meant it as a rhetorical question.

Put more directly: one aspect of this fix should be a platform-specific "this should always run" thing which tells the OS X slaves to puke if they can't import this reactor for any reason.

comment:4 Changed 3 years ago by glyph

  • Priority changed from normal to highest

Regression, so: highest priority. I would just revert the other change but it looks like an important bugfix for its own right.

comment:5 Changed 3 years ago by glyph

  • Milestone set to Twisted-11.1

And it needs to be completed before 11.1.

comment:6 Changed 3 years ago by glyph

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

I guess I'll take it over.

comment:7 Changed 3 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/cfreactor-import-5088

(In [31936]) Branching to 'cfreactor-import-5088'

comment:8 Changed 3 years ago by itamar

Anything else that needs to be done before this can go into review, other than adding a newsfile? This is only remaining blocker for 11.1.

comment:9 Changed 3 years ago by exarkun

  • Keywords review added
  • Owner glyph deleted
  • Status changed from assigned to new

comment:10 Changed 3 years ago by glyph

  • Keywords review removed
  • Owner set to exarkun

Thanks for turning the crank. Land it.

comment:11 Changed 3 years ago by glyph

I filed #5216 to try to improve the reactor builder tests so we won't see something like this again.

comment:12 Changed 3 years ago by exarkun

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

(In [32411]) Merge cfreactor-import-5088

Author: exarkun
Reviewer: glyph
Fixes: #5088

Fix a bad import in cfreactor which caused it to be unusable. Also fix the
call-fileno-after-removal behavior which #4539 fixed for the other reactors.

Note: See TracTickets for help on using tickets.