Replace uses of twisted.python.reflect.allYourBase with inspect.getmro

[exarkun]

These functions are not exactly the same, but the switch should be pretty easy in most cases. getmro is several times faster than allYourBase because it knows about __mro__. It also takes care of duplication in a diamond inheritance hierarchy, which at least one caller of allYourBase doesn't realize it cares about (Failure doesn't break because of the duplication, but it ends up doing extra work).

Maybe we can eventually deprecate allYourBase and accumulateBases, too.

[ironfroggy]

Patch to remove calls to allYourBase

[ironfroggy]

The only place the attached patch does not fix is in twisted.persisted, which #3843 calls to deprecate. That is also the only usage of the extra parameter to allYourBase(). Is it required for this ticket to fix the code in a module that is being deprecated?

[exarkun]

Thanks ironfroggy! Leaving the deprecated code alone is fine. It's good enough that we'll eventually delete it. :)

(not a review, just answering that question)

[lvh]

(In [31015]) Branching to 'allyourbase-4928'

[lvh]

Patch looks good, waiting for  http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/allyourbase-4928 to not blow up and then I'll merge it. Works on my box at least, and patch looks pretty benign.

Thanks for your contribution!

[lvh]

(In [31020]) allYourBase replaced with inspect.getmro

Author: ironfroggy Reviewer: lvh Fixes: #4928

[lvh]

(In [31023]) Reverting 31020, apparently *does* fail on some buildbots.

Reopens: #4928

[lvh]

[ERROR]
Traceback (most recent call last):
  File "/var/lib/buildbot/bot-bigdogcloud2/lucid64-python2.6-select/Twisted/twisted/trial/test/test_reporter.py", line 1236, in test_subunitWithoutAddExpectedFailureInstalled
    self.removeMethod(reporter.TestProtocolClient, 'addExpectedFailure')
  File "/var/lib/buildbot/bot-bigdogcloud2/lucid64-python2.6-select/Twisted/twisted/trial/test/test_reporter.py", line 1222, in removeMethod
    for base in [klass] + getmro(klass):
exceptions.TypeError: can only concatenate list (not "tuple") to list

So getmro returns a tuple, and you can't concat that to a list.

My suggestion:

for base in [klass] + list(getmro(klass)):

Or possibly:

bases = [klass]
bases.extend(getmro(klass))
for base in bases:

[lvh]

Okay so scratch that, aumary told me that you should just use getmro(klass) since it already includes the class itself.

[lvh]

There is of course the caveat that maybe there exists code that appends that output to a list (in fact that seems plausible) so perhaps it should be list(getmro(x))

[lvh]

We figured out the intermittent failure: tests were being skipped due to dependence on Subunit which was not installed on most slaves and my own machine which is why I was convinced it wasn't broken. Fix is still identical as mentioned in last comment

[lvh]

#4946 is now blocking on this so it'd be cool if someone could fix it, I have some reviews to do right now so I can't do it for the moment.

[binjured]

Done. (see attached)

[binjured]

Proper use of getmro()

[lvh]

 http://buildbot.twistedmatrix.com/builders/lucid64-py2.6-select/builds/667

Yaay buildbot agrees it is no longer broken. Thanks binjured!

[lvh]

(In [31049]) twisted.python.reflect.allYourBase replaced by inspect.getmro

Author: ironfroggy, binjured Reviewer: lvh Fixes: #4928

[amaury]

One more nit: in failure.py, self.parents now contains the initial class *twice*. For example:

>>> Failure(Exception()).parents
['exceptions.Exception', 'exceptions.BaseException', '__builtin__.object', 'exceptions.Exception']

It's not very important (parents is only used in a statement if err in self.parents:), but it's not optimal.

[lvh]

Good call. Made #4957.