Raise the minimum required version of zope.interface to 3.6.0

[itamar]

As mentioned in  http://pypi.python.org/pypi/zope.interface/4.0.0, newer versions of zope.interface give deprecation warning for implements(), which is used widely by Twisted. This was done in order to support Python 3; the newer version is a class decorator.

As part of Python 3 support we should probably switch to the new style. It was added in zope.interface 3.6, in 2010, so switching completely doesn't seem like an issue:  http://pypi.python.org/pypi/zope.interface/3.6.0#id5

Using class decorators shouldn't be a problem post-12.1, since we'll have dropped Python 2.5 support by then (#5553).

We should ensure Twisted mentions it requires zope.interface 3.6 in the appropriate places (import time, dependency documentation).

[mithrandi]

This is still relevant for Python 3 support, but note that z.i 4.0.1 removed the DeprecationWarning again:

4.0.1 (2012-05-22)

Dropped explicit DeprecationWarnings for "class advice" APIS (these APIs are still deprecated under Python 2.x, and still raise an exception under Python 3.x, but no longer cause a warning to be emitted under Python 2.x).

[vperic]

There is a package, zope.fixers, which has a 2to3 fixer to change this. It's very useful for the manual copy/pasting work (but obviously, all changes should have tests and this should be checked manually). I have written a short script that will run the fixer on all the files in a directory:

#!/usr/bin/python
import sys

from lib2to3.refactor import RefactoringTool, get_fixers_from_package
fixers = get_fixers_from_package('zope.fixers')

tool = RefactoringTool(fixers)
tool.refactor_dir(sys.argv[1], write=True)

I recommend running it under Python 3 (as it's much faster) and not running it on the whole twisted package at once, as it can be resource intensive.

[vperic]

Update docs

[vperic]

I've attached a patch which notes in INSTALL and setup.py (the only places I found it mentioned) that the required zope.interface is now 3.6.0 or later. The topfile is marked as "feature" per itamar's suggestion on IRC (it's technically a .misc but then it wouldn't be displayed and it's an important change).

I suggest to change the scope of this ticket to cover just this, and then to open separate tickets for individual modules or packages; we could also open a new "meta" ticket for tracking this (basically, I'd like to get this small patch in so I can start fixing this).

[itamar]

You can change the scope, sure, just change the subject of the ticket. Then open a ticket per package, say, for the API conversion.t

[vperic]

Ok, changing the scope of this to just raising the version requirement and submitting for review. Maybe the wording of the topfile can be improved? I'll open the new tickets once this gets in (since they don't make much sense otherwise).

[itamar]

Assuming the buildbots are happy running a branch, please merge.

[vperic]

(In [35037]) Create branch zope-requirement-5683

[vperic]

 Buildbot run.

[vperic]

Ok, this isn't tested (the change in setup.py) so it cannot go in. I'm not sure exactly *how* to test it either, so if anyone has a suggestion...

(As exarkun pointed on chat, the "practical" issue is that some of the buildbots don't have zope.interface >= 3.6.0 and this wasn't caught by the run above)

[vperic]

Phew! I've created tickets for individual modules now (tickets #5831 - #5846) and attached patches to most of them; I'll add the rest once at least this one gets in. I didn't submit them for review yet, since we want this to get merged first. I've manually checked (with coverage) that these are all executed - is this enough to consider them tested? In any case, this is something to be handled in each individual review.

About the issue here, I've found a Launchpad bug for zope.interface ( There is no version attribute anywhere in zope.interface) and I see that exarkun and glyph both participated in it, but no solution was found (and current versions of zope.interface have no method of checking as far as I can see). As such, I'm not sure how this should be tested. I guess there should be at least one buildbot which actually installs Twisted (in a virtualenv I guess -- have we considered using Tox?) and that would help. The other solution is to drop the change from setup.py (leaving the dependency unqualified, like now) and just manually make sure all our buildbots are updated. Personally, I feel this is an inferior solution, because we lose one check somewhere down the line (even if we can't test it), but it does get around the "all code should be covered" requirement.

Whatever happens, our buildbots should be updated - should I open a separate ticket for this, or just mail the appropriate people, or.. ?

[itamar]

You can check version, more or less, by checking for the existence of the decorator attribute ("implementor"?). So personally I think a unit test that checks for that would be fine. JP didn't like that idea for some reason, though.

[vperic]

Right, I think that's a reasonable solution, checking for the exact feature we need. Where should such a test live, I don't think the setup.py is tested anyway? twisted.test.test_setup? Or folded into another test suite?

[exarkun]

Sorry, I'm sure I'm just slow, but I don't see what purpose this unit test is going to serve. It's certainly not going to demonstrate that setup.py is declaring accurate dependencies. If we start making changes to make Twisted depend on zope.interface 3.6, plenty of existing unit tests will fail. So what good is a test that checks for the existence of implementor in zope.interface going to do?

[vperic]

I'm not sure I understand what you mean by It's certainly not going to demonstrate that setup.py is declaring accurate dependencies. setup.py currently doesn't declare a minimum version, adding this is surely an improvement? If you don't consider it an improvement, I can still drop that change (remember, we still depend on zope.interface 3.3.0, it's just not tested anywhere). As for If we start making changes to make Twisted depend on zope.interface 3.6, plenty of existing unit tests will fail., I'm again not sure I follow? Why would updating a dependency make unit tests fail?

I agree with Itamar - we need a specific feature from zope.interface (well, @provides is also used in a few plces but they go together) and a reasonable way of testing we've got the right version is to check if we can get the specific feature we need.

[vperic]

Of course, I would also be very happy if you could suggest an alternative that would be acceptable to you.

[mithrandi]

If functionality in Twisted relies on zope.interface 3.6.0, then the unit tests that cover that functionality will fail when run against a version of zope.interface that is too low. Thus, there's no need for some arbitrary version-detection test, either code works or it doesn't.

However, this branch doesn't introduce any code that relies on functionality in zope.interface 3.6.0, it introduces a change in setup.py to declare a dependency on zope.interface 3.6.0; thus there should be a test that verifies the behaviour of setup.py, which has nothing to do with the behaviour of zope.interface at all.

[vperic]

v2, without the change in setup.py

[vperic]

Ok then, no changes in setup.py, since I couldn't figure out how to test it. The patch now only changes the INSTALL file. Here's a snippet from IRC where I talked about this with exarkun:

[13:16] <vperic> exarkun, idnar: Do you have a suggestion on *how* to test setup.py? If not, I'll just drop the change in setup.py and keep just the change in the docs (this for ticket 5683)
[13:18] <exarkun> vperic: nope, I have no suggestions on that subject.
[13:19] <vperic> exarkun: ok, would you then accept a patch which does not change setup.py?
[13:20] <exarkun> I dunno.  But I would not reject a patch that does not change setup.py just because it does not change setup.py.

Submitting for review again; ah, I forgot to change the topfile: just imagine it says "removal" instead of "feature".

[itamar]

I'm going to accept this as is... and open a separate ticket to add version requirements (ideally unconditionally rather than tied to setuptools) to setup.py. I have separately started looking into upgrading zope.interface on buildslaves so we can actually start using 3.6's functionality; the benefit of merging this now is that it warns users they should start upgrading.

[itamarst]

(In [35161]) Minimum version of zope.interface is now 3.6.

Author: vperic Review: itamar Fixes: #5683 This is just a documentation change, noting to users that they should switch to zope.interface 3.6 or later.

[itamar]

Jean-Paul believes we should revert this, and only require 3.6 in release after next; this is viable only if we do a release in the very near future. Until we figure it out, I suggest holding off on merging any of of the @implementer tickets.

[glyph]

Replying to itamar:

Jean-Paul believes we should revert this, and only require 3.6 in release after next;

Not that I necessarily disagree, but: would Jean-Paul (or you) care to give a specific reason for the revert?

[itamar]

We did not announce in advance that we're dropping support for older versions of zope.interface, so it would be good to have 12.2 to announce the soon-to-be-discontinued-support and the release after that to drop it.

[glyph]

Replying to itamar:

We did not announce in advance that we're dropping support for older versions of zope.interface, so it would be good to have 12.2 to announce the soon-to-be-discontinued-support and the release after that to drop it.

Perhaps we should add this grace period for dependency version compatibility to CompatibilityProcess?

[glyph]

Replying to glyph:

Replying to itamar:

We did not announce in advance that we're dropping support for older versions of zope.interface, so it would be good to have 12.2 to announce the soon-to-be-discontinued-support and the release after that to drop it.

Perhaps we should add this grace period for dependency version compatibility to CompatibilityProcess?

Ahem. CompatibilityPolicy.

[itamarst]

(In [35278]) Revert #5683; we will require zope.interface 3.6 only after 12.2.

Reopens: #5683.

[exarkun]

(In [35405]) Remove the zope.interface version requirement change

refs #5683

[exarkun]

(In [35416]) Merge zope-requirement-5683 Author: vperic Reviewer: itamar Fixes: #5683

Update the install document to indicate Zope Interface 3.6 or newer is now required.