Opened 11 years ago

Closed 11 years ago

#1440 enhancement closed fixed (fixed)

Remove old cred

Reported by: Jean-Paul Calderone Owned by:
Priority: highest Milestone:
Component: core Keywords:
Cc: Jean-Paul Calderone, spiv Branch:


Change History (10)

comment:1 Changed 11 years ago by Jean-Paul Calderone

Status: newassigned

This is mostly done. The only thing left to do is re-implement four PB tests which used deprecated APIs but which tested non-deprecated functionality.

comment:2 Changed 11 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted
Priority: lowhighest
Status: assignednew

Ready for review in remove-old-cred-1440

comment:3 Changed 11 years ago by spiv

Mr. Wiki say: source:branches/remove-old-cred-1440 (make a link for the lazy!)

comment:4 Changed 11 years ago by spiv

Keywords: review removed
Owner: set to Jean-Paul Calderone

In twisted/test/

-    def testImmediateClose(self):
-        cc = protocol.ClientCreator(reactor, protocol.Protocol)
-        d = cc.connectTCP("", self.portno)
-        d.addCallback(lambda p: p.transport.loseConnection())
-        d = defer.Deferred()
-        # clunky, but it works
-        reactor.callLater(0.1, d.callback, None)
-        return d

I don't see a replacement for this test. It's obviously a crappy test, but still... it would be nice to have a test that an immediate disconnect doesn't cause an unhandled exception. It can probably be a proper unit test that just instantiates the relevant protocol and calls connectionMade then connectionLost, never touching the reactor, rather than the vile thing it is now.

(I don't see a replacement for many tests in that file, but unlike the others, this one doesn't have any supressions for deprecation warnings)

testClientConnectionLost is horribly complicated for what sounds from the docstring like a fairly simple test, but that seems hard to avoid (and isn't a new problem anyway). Surprisingly though it contains no self.assert/failUnless... calls. It'd be worth putting an self.failUnless(isinstance(anotherRootObj, pb.RemoteReference) in gotAnotherRootObj just for the sake of clarity.

Anyway, I'm very happy with this. It deletes over 3000 lines of cruft! You've just wiped thousands of dollars of value off our sloccount results...

The branch is good enough to merge as-is, but consider my suggestions.

comment:5 Changed 11 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted

I added a test for immediate close that does what you suggested. As I was going to merge this, I realized I had missed some things. Two files in conch, and, as well as a pile of pb perspective documentation. I deleted all of these and removed the references to the documentation from the various index files. I also deleted a couple exception classes from twisted/cred/

I don't _think_ there is any more old cred stuff, but it's hard to tell.

Another pass of review would be appreciated.

comment:6 Changed 11 years ago by spiv

Owner: set to spiv
Status: newassigned

comment:7 Changed 11 years ago by spiv

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

You broke the docs build:

Otherwise, this looks fine to me.

comment:8 Changed 11 years ago by spiv

Cc: spiv added

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

Resolution: fixed
Status: newclosed

(In [17081]) Merge remove-old-cred-1440

Author: exarkun, radix Reviewer: spiv Fixes #1440

This removes the old authentication and authorization APIs from twisted.cred as well as the old Application and Service APIs from twisted.internet. Compatibility support code for these APIs has also been removed, as have their unit tests and documentation which referred to them.

comment:10 Changed 6 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.