Deprecate pickling support (__getstate__ and __setstate__) in twisted.internet._sslverfiy

[itamar]

Pickleability is no longer a goal for Twisted. As such, the various __getstate__ and __setstate__ methods of twisted.internet._sslverify classes should be deprecated, so we can remove the extra code involved.

[itamar]

See twisted/python/reflect.py and twisted/test/test_reflect.py for an example of deprecation and testing of deprecation.

[canibanoglu]

Deprecation of getstate and setstate along with new test which check for deprecation

[tom.prince]

The patch you attached is reversed, and appears to be against the most recent release, rather than trunk. If you are planning on contributing to twisted, you should be working in a source checkout (see [TwistedDevelopment]).

[tom.prince]

The attached patch appears to not apply to trunk.

1. The deprecations want to use twisted.python.deprecate.deprecated, rather than calling warnings.warn directly. The message should probably say something about pickling not being supported anymore.
2. Running

   python -Wall bin/trial twisted.test.test_sslverify
   

   gives me a bunch of deprecation warnings. They should be suppressed.

[canibanoglu]

Thanks a lot for the input.

First of all, the release and trunk version mixup was bordering on idiocy, sorry about that.

1. I have tried using twisted.python.deprecate.deprecated but it doesn't pass the tests because it doesn't work the same way for class methods and module-level functions. I found a ticket which is now marked as fixed but deprecated still acts in a peculiar way. More specifically this is what I get

{{{twisted.trial.unittest.FailTest: not equal: a = 'twisted.internet._sslverify.OpenSSLCertificateOptions.getstate was deprecated in Twisted 13.1.0' b = 'twisted.internet._sslverify.getstate was deprecated in Twisted 13.1.0'}}}

2. Thank you very much for pointing that out, I will definitely take care of that.

[canibanoglu]

New patch on trunk which deprecates getstate and setstate in twisted.internet._sslverify and adds a 6166.removal in twisted/topfiles

[canibanoglu]

Here's my second go at it.

- I have suppressed deprecation warnings when running the tests with

python -Wall bin/trial twisted.test.test_sslverify

- Still can't get @deprecated to work correctly with class methods. This patch still uses warnings.warn for deprecation warnings. Added "Pickling is no longer supported." to the deprecation warning message.

- Added 6166.removal topfile.

I have a question though. While working on this I have found that @deprecated decorator is not functioning as expected when decorating class methods and the full test run hangs on OS X Mountain Lion (10.8). There are tickets for both of these and they both appear as fixed. Should I create another ticket?

Thanks a lot for the assistance.

[exarkun]

New patch on trunk which ...

Can you re-attach the patch, this time without a space in the filename? trac cannot serve attachments with spaces in them.

[canibanoglu]

There's an underscore between 6166 and 2 :) Should I rename them differently from now on?

[exarkun]

Sorry. I guess there's also a problem with filenames with "#" in them.

[canibanoglu]

Removed # from the filename

[canibanoglu]

Uploaded the same patch file without the "#" character in the filename.

[tom.prince]

1. There are heplers to suppres warnings. See twisted.trial.util.suppress and the .suppress attribute of TestCase and/or test_* methods.
2. Hmm. Looking at the r24931 diff for #3254, deprecated works with methods, but it needs to called as

    Class.method = deprecated(version)(Class.method)
   

   after the class definition. You should file a ticket to support the obvious thing.
   • (Making this work is probably tricky, and would involve making deprecate return a descriptor)
3. Is there any reason you created your own helper, rather than using callDeprecated?
4. The newsfile could be improved by listing all the things that are getting deprecated, rather than saying each individual thing is deprecated, but might be better to just say that pickling the two relevant classes is now deprecated.
5. Even if not using @deprecated, you should still use the same format for the docstring.

[canibanoglu]

Tests now use callDeprecated to test deprecations, use .suppress attribute to suppress deprecation warning messages