Opened 11 years ago

Closed 10 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:

Description


Change History (10)

comment:1 Changed 10 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 10 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 10 years ago by spiv

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

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

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

comment:7 Changed 10 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 10 years ago by spiv

  • Cc spiv added

comment:9 Changed 10 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 5 years ago by <automation>

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