Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecated twisted.internet.defer.TimeoutError in favor of twisted.internet.error.TimeoutError #261

Closed
wants to merge 13 commits into from
Closed
24 changes: 15 additions & 9 deletions twisted/internet/defer.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
from functools import wraps

# Twisted imports
from twisted.internet import error
from twisted.python.compat import cmp, comparable
from twisted.python import lockfile, failure
from twisted.logger import Logger
from twisted.python.deprecate import warnAboutFunction, deprecated
from twisted.python.deprecate import (warnAboutFunction, deprecated,
deprecatedModuleAttribute)
from twisted.python.versions import Version

log = Logger()
Expand All @@ -45,11 +47,14 @@ class CancelledError(Exception):
"""


class TimeoutError(Exception):
class TimeoutError(error.TimeoutError):
"""
This exception is deprecated. It is used only by the deprecated
L{Deferred.setTimeout} method.
This exception is deprecated.
"""
deprecatedModuleAttribute(
Version("Twisted", 16, 3, 0),
"Use twisted.internet.error.TimeoutError instead",
"twisted.internet.defer", "TimeoutError")



Expand Down Expand Up @@ -160,8 +165,9 @@ def maybeDeferred(f, *args, **kw):



@deprecated(Version('Twisted', 16, 3, 0))
def timeout(deferred):
deferred.errback(failure.Failure(TimeoutError("Callback timed out")))
deferred.errback(failure.Failure(error.TimeoutError("Callback timed out")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another missing coverage which will block the merge

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method was added in e0b4d6a

setTimeout() was deleted in in https://github.com/twisted/commit/0d2674cb7567e33ae8bdfca67ab9e4847643696e

at that time timeout() was not deleted, but it should have been.
I think that the best way forward is to deprecate this method as well and write a deprecation test for it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. Let's deprecate it and the deprecation test should handle the coverage:)

It can be deprecated in this branch as it is somehow related to defer.TimeoutError




Expand Down Expand Up @@ -1594,7 +1600,8 @@ def deferUntilLocked(self, timeout=None):
lock has not been acquired.

@return: a L{Deferred} which will callback when the lock is acquired, or
errback with a L{TimeoutError} after timing out or an
errback with a
L{twisted.internet.error.TimeoutError} after timing out or an
L{AlreadyTryingToLockError} if the L{deferUntilLocked} has already
been called and not successfully locked the file.
"""
Expand Down Expand Up @@ -1634,10 +1641,9 @@ def _tryLock():
d.callback(None)
else:
if timeout is not None and self._timeoutCall is None:
reason = failure.Failure(TimeoutError(
reason = failure.Failure(error.TimeoutError(
"Timed out acquiring lock: %s after %fs" % (
self.name,
timeout)))
self.name, timeout)))
self._timeoutCall = self._scheduler.callLater(
timeout, _cancelLock, reason)

Expand Down
2 changes: 1 addition & 1 deletion twisted/internet/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def getHostByName(name, timeout = (1, 3, 11, 45)):
preference to A records. If there are multiple records of
the type to be returned, one will be selected at random.

@raise twisted.internet.defer.TimeoutError: Raised
@raise twisted.internet.error.TimeoutError: Raised
(asynchronously) if the name cannot be resolved within the
specified timeout period.
"""
Expand Down
4 changes: 2 additions & 2 deletions twisted/names/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ def queryUDP(self, queries, timeout = None):
When the last timeout expires, the query is considered failed.

@rtype: C{Deferred}
@raise C{twisted.internet.defer.TimeoutError}: When the query times
@raise L{twisted.internet.error.TimeoutError}: When the query times
out.
"""
if timeout is None:
Expand Down Expand Up @@ -317,7 +317,7 @@ def _reissue(self, reason, addressesLeft, addressesUsed, query, timeout):
# protocol we're giving up on it and return a terminal timeout failure
# to our caller.
if not timeout:
return failure.Failure(defer.TimeoutError(query))
return failure.Failure(error.TimeoutError(query))

# Get an address to try. Take it out of the list of addresses
# to try and put it ino the list of already tried addresses.
Expand Down
2 changes: 1 addition & 1 deletion twisted/names/error.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from __future__ import division, absolute_import

from twisted.internet.defer import TimeoutError
from twisted.internet.error import TimeoutError


class DomainError(ValueError):
Expand Down
2 changes: 1 addition & 1 deletion twisted/names/resolve.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def __init__(self, resolver, query, timeout):

def __call__(self, failure):
# AuthoritativeDomainErrors should halt resolution attempts
failure.trap(dns.DomainError, defer.TimeoutError, NotImplementedError)
failure.trap(dns.DomainError, error.TimeoutError, NotImplementedError)
return self.resolver(self.query, self.timeout)


Expand Down
5 changes: 3 additions & 2 deletions twisted/names/test/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
from twisted.python.runtime import platform

from twisted.internet import defer
from twisted.internet.error import CannotListenError, ConnectionRefusedError
from twisted.internet.error import (CannotListenError, ConnectionRefusedError,
TimeoutError)
from twisted.internet.interfaces import IResolver
from twisted.internet.test.modulehelpers import AlternateReactor
from twisted.internet.task import Clock
Expand Down Expand Up @@ -1173,7 +1174,7 @@ def _cbRoundRobinBackoff(self, result):


def _ebRoundRobinBackoff(self, failure, fakeProto):
failure.trap(defer.TimeoutError)
failure.trap(TimeoutError)

# Assert that each server is tried with a particular timeout
# before the timeout is increased and the attempts are repeated.
Expand Down
3 changes: 2 additions & 1 deletion twisted/names/test/test_rootresolve.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
from twisted.python.log import msg
from twisted.trial import util
from twisted.trial.unittest import SynchronousTestCase, TestCase
from twisted.internet.defer import Deferred, succeed, gatherResults, TimeoutError
from twisted.internet.defer import Deferred, succeed, gatherResults
from twisted.internet.error import TimeoutError
from twisted.internet.interfaces import IResolverSimple
from twisted.names import client, root
from twisted.names.root import Resolver
Expand Down
2 changes: 1 addition & 1 deletion twisted/protocols/ftp.py
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ def timeoutFactory(self):
d = self.deferred
self.deferred = None
d.errback(
PortConnectionError(defer.TimeoutError("DTPFactory timeout")))
PortConnectionError(error.TimeoutError("DTPFactory timeout")))


def cancelTimeout(self):
Expand Down
3 changes: 2 additions & 1 deletion twisted/protocols/memcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ def doSomething(proto):

from twisted.protocols.basic import LineReceiver
from twisted.protocols.policies import TimeoutMixin
from twisted.internet.defer import Deferred, fail, TimeoutError
from twisted.internet.defer import Deferred, fail
from twisted.internet.error import TimeoutError
from twisted.python import log


Expand Down
39 changes: 36 additions & 3 deletions twisted/test/test_defer.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

from twisted.python import failure, log
from twisted.python.compat import _PY3
from twisted.internet import defer, reactor
from twisted.internet import defer, reactor, error
from twisted.internet.task import Clock
from twisted.trial import unittest

Expand Down Expand Up @@ -2344,7 +2344,7 @@ def test_waitUntilLockedWithTimeoutLocked(self):
self.assertTrue(self.lock.lock())

d = self.lock.deferUntilLocked(timeout=5.5)
self.assertFailure(d, defer.TimeoutError)
self.assertFailure(d, error.TimeoutError)

self.clock.pump([1] * 10)

Expand All @@ -2357,7 +2357,7 @@ def test_waitUntilLockedWithTimeoutUnlocked(self):
but the lock is unlocked before our timeout.
"""
def onTimeout(f):
f.trap(defer.TimeoutError)
f.trap(error.TimeoutError)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is a missing coverage which will block the merge :(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I test that? That code is inside a test that is supposed to be testing a condition that is never supposed to happen.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question :)

I would say that we should have a # pragma: no-cover comment to let coverage know that this line should be ignored

... but first, let's talk about it over the mailing list. I will start the conversation :)

Thanks!

self.fail("Should not have timed out")

self.assertTrue(self.lock.lock())
Expand Down Expand Up @@ -2446,3 +2446,36 @@ def test_cancelDeferUntilLockedWithTimeout(self):
self.assertFalse(timeoutCall.active())
self.assertIsNone(self.lock._timeoutCall)
self.failureResultOf(deferred, defer.CancelledError)



class TimeoutErrorTests(unittest.TestCase, ImmediateFailureMixin):
def test_deprecatedTimeout(self):
"""
L{twisted.internet.defer.timeout} is deprecated.
"""
deferred = defer.Deferred()
defer.timeout(deferred)
self.assertFailure(deferred, error.TimeoutError)
warningsShown = self.flushWarnings([self.test_deprecatedTimeout])
self.assertEqual(len(warningsShown), 1)
self.assertIdentical(warningsShown[0]['category'], DeprecationWarning)
self.assertEqual(
warningsShown[0]['message'],
'twisted.internet.defer.timeout was deprecated in '
'Twisted 16.3.0')


def test_deprecatedTimeoutError(self):
"""
L{twisted.internet.defer.TimeoutError} is deprecated.
"""
defer.TimeoutError
warningsShown = self.flushWarnings([self.test_deprecatedTimeoutError])
self.assertEqual(len(warningsShown), 1)
self.assertIdentical(warningsShown[0]['category'], DeprecationWarning)
self.assertEqual(
warningsShown[0]['message'],
'twisted.internet.defer.TimeoutError was deprecated in '
'Twisted 16.3.0: Use twisted.internet.error.'
'TimeoutError instead')
2 changes: 1 addition & 1 deletion twisted/test/test_ftp.py
Original file line number Diff line number Diff line change
Expand Up @@ -1226,7 +1226,7 @@ def test_connectionFailedAfterTimeout(self):

# Now fail the connection attempt. This should do nothing. In
# particular, it should not raise an exception.
self.factory.clientConnectionFailed(None, defer.TimeoutError("foo"))
self.factory.clientConnectionFailed(None, error.TimeoutError("foo"))

# Give the Deferred to trial so it can make sure it did what we
# expected.
Expand Down
5 changes: 3 additions & 2 deletions twisted/test/test_memcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@
from twisted.trial.unittest import TestCase
from twisted.test.proto_helpers import StringTransportWithDisconnection
from twisted.internet.task import Clock
from twisted.internet.defer import Deferred, gatherResults, TimeoutError
from twisted.internet.defer import Deferred, gatherResults
from twisted.internet.defer import DeferredList
from twisted.internet.error import TimeoutError



Expand Down Expand Up @@ -314,7 +315,7 @@ def test_timeOut(self):
self.assertFailure(d2, TimeoutError)

def checkMessage(error):
self.assertEqual(str(error), "Connection timeout")
self.assertRegex(str(error), "Connection timeout")

d1.addCallback(checkMessage)
self.assertFailure(d3, ConnectionDone)
Expand Down
1 change: 1 addition & 0 deletions twisted/topfiles/8533.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
twisted.internet.defer.TimeoutError is deprecated in favor of twisted.internet.error.TimeoutError
3 changes: 2 additions & 1 deletion twisted/trial/_asynctest.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
# installs a user-specified reactor, installing the default reactor and
# breaking reactor installation. See also #6047.
from twisted.internet import defer, utils
from twisted.internet.error import TimeoutError
from twisted.python import failure

from twisted.trial import itrial, util
Expand Down Expand Up @@ -81,7 +82,7 @@ def _run(self, methodName, result):
from twisted.internet import reactor
timeout = self.getTimeout()
def onTimeout(d):
e = defer.TimeoutError("%r (%s) still running at %s secs"
e = TimeoutError("%r (%s) still running at %s secs"
% (self, methodName, timeout))
f = failure.Failure(e)
# try to errback the deferred that the test returns (for no gorram
Expand Down
8 changes: 4 additions & 4 deletions twisted/trial/test/test_deferred.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import unittest as pyunit

from twisted.internet import defer
from twisted.internet.error import TimeoutError
from twisted.trial import unittest, reporter
from twisted.trial import util
from twisted.trial.test import detests
Expand Down Expand Up @@ -93,7 +93,7 @@ def test_setUp(self):
self.assertEqual(len(result.failures), 0)
self.assertEqual(len(result.errors), 1)
self.assertFalse(detests.DeferredSetUpNeverFire.testCalled)
self.assertTrue(result.errors[0][1].check(defer.TimeoutError))
self.assertTrue(result.errors[0][1].check(TimeoutError))


class TestTester(unittest.TestCase):
Expand Down Expand Up @@ -178,8 +178,8 @@ def getTest(self, name):
return detests.TimeoutTests(name)

def _wasTimeout(self, error):
self.assertEqual(error.check(defer.TimeoutError),
defer.TimeoutError)
self.assertEqual(error.check(TimeoutError),
TimeoutError)

def test_pass(self):
result = self.runTest('test_pass')
Expand Down
5 changes: 4 additions & 1 deletion twisted/web/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def urlunparse(parts):
from twisted.internet.abstract import isIPv6Address
from twisted.internet.interfaces import IProtocol
from twisted.internet.endpoints import TCP4ClientEndpoint, SSL4ClientEndpoint
from twisted.internet.error import TimeoutError
from twisted.python.util import InsensitiveDict
from twisted.python.components import proxyForInterface
from twisted.web import error
Expand Down Expand Up @@ -248,7 +249,9 @@ def handleResponse(self, response):
def timeout(self):
self.quietLoss = True
self.transport.abortConnection()
self.factory.noPage(defer.TimeoutError("Getting %s took longer than %s seconds." % (self.factory.url, self.factory.timeout)))
self.factory.noPage(
TimeoutError("Getting %s took longer than %s seconds." %
(self.factory.url, self.factory.timeout)))


class HTTPPageDownloader(HTTPPageGetter):
Expand Down
17 changes: 9 additions & 8 deletions twisted/web/test/test_webclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from twisted.web.static import Data
from twisted.web.util import Redirect
from twisted.internet import reactor, defer, interfaces
from twisted.internet.error import TimeoutError
from twisted.python.filepath import FilePath
from twisted.python.log import msg
from twisted.protocols.policies import WrappingFactory
Expand Down Expand Up @@ -478,13 +479,13 @@ def test_timeoutTriggering(self):
"""
When a non-zero timeout is passed to L{getPage} and that many
seconds elapse before the server responds to the request. the
L{Deferred} is errbacked with a L{error.TimeoutError}.
L{Deferred} is errbacked with a L{twisted.internet.error.TimeoutError}.
"""
# This will probably leave some connections around.
self.cleanupServerConnections = 1
return self.assertFailure(
client.getPage(self.getURL("wait"), timeout=0.000001),
defer.TimeoutError)
TimeoutError)


def testDownloadPage(self):
Expand Down Expand Up @@ -694,7 +695,7 @@ def test_downloadTimeout(self):
L{client.HTTPDownloader.__init__} elapses without the complete response
being received, the L{defer.Deferred} returned by
L{client.downloadPage} fires with a L{Failure} wrapping a
L{defer.TimeoutError}.
L{twisted.internet.error.TimeoutError}.
"""
self.cleanupServerConnections = 2
# Verify the behavior if no bytes are ever written.
Expand All @@ -709,8 +710,8 @@ def test_downloadTimeout(self):
self.mktemp(), timeout=0.01)

return defer.gatherResults([
self.assertFailure(first, defer.TimeoutError),
self.assertFailure(second, defer.TimeoutError)])
self.assertFailure(first, TimeoutError),
self.assertFailure(second, TimeoutError)])


def test_downloadTimeoutsWorkWithoutReading(self):
Expand All @@ -719,8 +720,8 @@ def test_downloadTimeoutsWorkWithoutReading(self):
L{client.HTTPDownloader.__init__} elapses without the complete response
being received, the L{defer.Deferred} returned by
L{client.downloadPage} fires with a L{Failure} wrapping a
L{defer.TimeoutError}, even if the remote peer isn't reading data from
the socket.
L{twisted.internet.error.TimeoutError}, even if the remote peer isn't
reading data from the socket.
"""
self.cleanupServerConnections = 1

Expand All @@ -729,7 +730,7 @@ def test_downloadTimeoutsWorkWithoutReading(self):
d = client.downloadPage(
self.getURL("never-read"),
self.mktemp(), timeout=0.05)
return self.assertFailure(d, defer.TimeoutError)
return self.assertFailure(d, TimeoutError)


def test_downloadHeaders(self):
Expand Down