Deprecate __getstate__ and __setstate__ implementations across the board

[teratorn]

It would seem there is no extant reason for keeping these methods around, except for backwards-compatibility with poorly-written, legacy Twisted-using code.

TAP (Twisted Application PICKLE) support, now gone, was what was depending on these methods, AFAIK.

There are 77 occurences in 32 files:

application/internet.py
application/service.py
cred/checkers.py
enterprise/adbapi.py
internet/protocol.py
internet/ssl.py
internet/_sslverify.py
mail/relaymanager.py
mail/smtp.py
manhole/service.py
manhole/telnet.py
names/authority.py
names/cache.py
names/client.py
news/database.py
persisted/aot.py
persisted/styles.py
python/failure.py
python/filepath.py
python/logfile.py
python/threadpool.py
runner/procmon.py
runner/test/test_procmon.py
spread/jelly.py
spread/publish.py
test/test_jelly.py
test/test_persisted.py
test/test_sslverify.py
web/distrib.py
web/server.py
words/im/basesupport.py
words/protocols/irc.py

What are the action items for this ticket?

• Add news file deprecating these implementations.
• Emit a deprecation warning on the use of these methods
• Come back in a few releases and remove the implementations (the interval is defined by CompatibilityPolicy)

Anything else?

[tom.prince]

buildbot currently uses pickles (although we would like to move away from them). So this change will affect us. We don't use TAP though, just styles.Versioned and (indirectly) styles.Ephemeral. I think that those, since they are directly related to pickling (and are useless otherwise) should be handled separately.

(As a note, even if we get rid of pickles entirely, we will still need to support them (and hence styles.Versioned) for a long time, to support migrating data.

[exarkun]

__getstate__ and __setstate__ are also used by twisted.spread.jelly. Any types that can be sent over a PB connection might be taking advantage of these hooks (including, possibly, types defined in Twisted). Removing support for pickle should not reduce the functionality of twisted.spread.jelly.

(Also, adjusting ticket description to clarify some points.)

[canibanoglu]

I have tried my best to deprecate getstate and setstate in twisted.internet._sslverify, so I know there's a ticket specific to that module. Should I open new tickets for each of the modules listed above or should I just provide a patch for the whole thing?

[exarkun]

I think it's a little bit too much to do at once. By the time you get done with all the methods and writing tests for them all, it's going to be fairly tedious patch to review.

One ticket for core and one ticket for all the subprojects together might make sense. That roughly splits the task in half. Smaller chunks of work is also totally fine, if you'd prefer.

I suggest explicitly addressing the twisted.spread.jelly compatibility question before doing much work here, though.

[canibanoglu]

I have asked this on IRC and couldn't receive an answer so I think it'll be nice to have the question here as well.

I have taken a look at twisted.spread.jelly and its usage of __getstate__ and __setstate___ methods but currently all of these have fallbacks to the ___dict___ attribute of any given class. Granted, some of these methods in their classes return other dictionaries compared to their __dict__ attributes but in the end of they day apart from objects which have no __dict__ attribute, twisted.spread.jelly would work as expected.

If I'm missing something crucial, I would really appreciate any pointers.

[exarkun]

Using __getstate__ and __setstate__ to customize the behavior of Jelly is part of the public, stable, compatibility-guaranteed API of twisted.spread. Code outside of Twisted may be taking advantage of this feature - code we can't see or change. This means that Jelly needs to continue to offer this feature at least until it has gone through the deprecation procedure.

Whether there are any further concerns with any of the specific __getstate__ or __setstate__ implementations can only be answered after looking at each one and thinking about what consequences removing it will have. This is a good reason to do the work in small chunks instead of trying to tackle the entire Twisted codebase in one pass.