Opened 9 years ago

Closed 9 years ago

#1440 enhancement closed fixed (fixed)

Remove old cred

Reported by: exarkun Owned by:
Priority: highest Milestone:
Component: core Keywords:
Cc: exarkun, spiv Branch:
Author: Launchpad Bug:

Description


Change History (10)

comment:1 Changed 9 years ago by exarkun

  • Status changed from new to assigned

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 9 years ago by exarkun

  • Keywords review added
  • Owner exarkun deleted
  • Priority changed from low to highest
  • Status changed from assigned to new

Ready for review in remove-old-cred-1440

comment:3 Changed 9 years ago by spiv

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

comment:4 Changed 9 years ago by spiv

  • Keywords review removed
  • Owner set to exarkun

In twisted/test/test_pb.py:

-    def testImmediateClose(self):
-        cc = protocol.ClientCreator(reactor, protocol.Protocol)
-        d = cc.connectTCP("127.0.0.1", 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 9 years ago by exarkun

  • Keywords review added
  • Owner exarkun 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, authorizer.py and identity.py, 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/error.py

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 9 years ago by spiv

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

comment:7 Changed 9 years ago by spiv

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

You broke the docs build: http://twistedmatrix.com/buildbot/full-2.3/builds/1591/step-process-docs/0

Otherwise, this looks fine to me.

comment:8 Changed 9 years ago by spiv

  • Cc spiv added

comment:9 Changed 9 years ago by exarkun

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

(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 4 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.