Deprecate twisted.python.hashlib

[thijs]

​hashlib was introduced in py2.5 and twisted.python.hashlib was added in 2007 (#2763) to "allow[s] application code to transparently use APIs which existed before C{hashlib} was introduced or to use C{hashlib} if it is available."

Twisted supports Python 2.6 and up and twisted's hashlib can be deprecated, and uses of it replaced, to make it easier to support py3 and discourage users from using this module.

Initially I wanted to file a ticket for porting this module to py3 but why port it when its there for (now) unsupported versions of Python.

[itamar]

You can see an example deprecation in twisted/conch/insults/__init__.py and the applicable tests in twisted/conch/test/test_insults.py.

[tom.prince]

Thanks for your contribution.

1. Don't delete the existing test. Even if people shouldn't be using the code, we don't want the code to break. The tests should only be removed when the code they are testing gets removed.
   • You'll want to make sure that the warning is suppressed in those tests (using the .suppress attribute and twisted.trial.util.suppress).
2. Similarily, the module docstring doesn't want to be completely deleted. On the other hand, it should be changed to include the deprecation information (twisted.python.deprecate._getDeprecationDocstring has the standard format for that). It probably also wants to be changed to indicate that twisted doesn't use it anymore.
3. There are still a couple of uses t.p.hashlib left after applying your patch. - `twisted/conch/client/knownhosts.py'
   • twisted/conch/ssh/keys.py
   • twisted/web/server.py
4. Your change needs a news file.
5. The conditional (try) logic in t.p.hashlib can be removed. On supported versions, the except will never trigger.
6. Refer to 'stdlib' or 'standard library' not 'Python Core'.

[rodney757]

lastest patch

[tom.prince]

Thanks for your contribution.

1. You can probably just do something like

   from hashlib import md5, sha1
   

   The code was structured the way it was before so it only did the import once. (Although, on further consideration, I'm not sure why the code wasn't like that to begin with.
2. 13.0.0 was the most recent release, so 13.1.0 will be the next release. That will be the release where this will be deprecated.
3. Calls to .flushWarnings should include a function to blame. This is to verify that the stacklevel argument is set correctly. It should point to the function that is invoking the deprecated functionality.
4. The spacing between functions and classes doesn't follow the ​coding standard.
   • (minor) Also, there typically isn't a blank line after method docstrings
   • There is a spurious blank line at the begining of hashlib's module docstring.

[thijs]

(In [38141]) Branching to 'deprecate-hashlib-6342'

[thijs]

(In [38145]) correct version number, refs #6342

[thijs]

Thanks for your patch. The version nr was still incorrect so I fixed that and forced a ​build.

Leaving the review keyword so someone else can review (and/or merge) this since I made some minor changes to the patch (inc adding news files).

[rwall]

Reviewing...

[rwall]

Code Review

Thanks for the branch. Here's a quick review.

Notes:

• Branch merges cleanly to trunk
• Tests pass locally on Fedora 18 x86_64

I spotted one minor thing:

1. source:branches/deprecate-hashlib-6342/twisted/python/test/test_hashlib.py
   1. test_deprecation
      1. "Test to" in docstring is unnecessary

Otherwise the branch looks fine and I couldn't find any remaining uses of twisted.python.hashlib. Fix that docstring and then merge.

-RichardW.

[thijs]

Thanks for the review, I'll fix that docstring and merge.

[thijs]

(In [38409]) address review comment, refs #6342

[thijs]

(In [38410]) Merge deprecate-hashlib-6342: Replace usage of twisted.python.hashlib and deprecate it in favor of the hashlib module in the standard library.

Author: rodney757 Reviewer: tom.prince, thijs, rwall Fixes: #6342