Opened 7 years ago

Closed 5 years ago

#4963 task closed fixed (fixed)

Deprecate twisted.mail.protocols.SSLContextFactory

Reported by: Jean-Paul Calderone Owned by: Jean-Paul Calderone
Priority: normal Milestone:
Component: mail Keywords: easy
Cc: moijes12 Branch: branches/deprecate-redundant-ssl-context-4963
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

SSLContextFactory provides no particular value over something like twisted.internet.ssl.DefaultOpenSSLContextFactory (in fact, less, since it allows SSLv2). We should change our internal usage of the former (probably to the latter) and deprecate the former.

Attachments (3)

4963.patch (2.3 KB) - added by moijes12 5 years ago.
doubt.patch (3.0 KB) - added by moijes12 5 years ago.
4963.2.patch (6.2 KB) - added by moijes12 5 years ago.
The tests for pop3sBackwardCompatibilty now use DefaultOpenSSLContextFactory. For this, a copy of server.pem has been placed in twisted/mail/test. All other changes that were supplied in the first patch are in here too. So, now, SSLContextFactory has been deprecated and its usage has been replaced with DefaultOpenSSLContextFactory.

Download all attachments as: .zip

Change History (14)

Changed 5 years ago by moijes12

Attachment: 4963.patch added

comment:1 Changed 5 years ago by moijes12

Cc: moijes12 added
Keywords: review added

comment:2 Changed 5 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to moijes12

Thanks for working on this, moijes12. It looks like your patch does a good job addressing the second half of this ticket, but I don't think it covers the first half.

Twisted itself should never trigger its own deprecation warnings. Therefore, when deprecating a Twisted API, we also need to remove all usages of that API within Twisted itself. (You may have noticed that we don't always do a good job of this, by the number of warnings emitted when running the test suite. This is an area where we need to improve.)

Apart from that:

  1. make sure you wrap lines at 80 columns or before
  2. name tests like test_foo instead of testFoo
  3. Don't start test method docstrings with "Test ..."

Thanks again!

Changed 5 years ago by moijes12

Attachment: doubt.patch added

comment:3 Changed 5 years ago by moijes12

Keywords: review added
Owner: moijes12 deleted

comment:4 Changed 5 years ago by moijes12

Keywords: review removed

Changed 5 years ago by moijes12

Attachment: 4963.2.patch added

The tests for pop3sBackwardCompatibilty now use DefaultOpenSSLContextFactory. For this, a copy of server.pem has been placed in twisted/mail/test. All other changes that were supplied in the first patch are in here too. So, now, SSLContextFactory has been deprecated and its usage has been replaced with DefaultOpenSSLContextFactory.

comment:5 Changed 5 years ago by moijes12

Keywords: review added

comment:6 Changed 5 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/deprecate-redundant-ssl-context-4963

(In [34763]) Branching to 'deprecate-redundant-ssl-context-4963'

comment:7 Changed 5 years ago by Jean-Paul Calderone

(In [34765]) Just use the server.pem from the source tree, no need to copy it to a temporary file first.

refs #4963

comment:8 Changed 5 years ago by Jean-Paul Calderone

(In [34766]) - Rewrite the test method docstring in the conventional style

  • Remove the call of SSLContextFactory, attribute access is enough
  • wrap the long warning assertion line to < 80 columns

refs #4963

comment:9 Changed 5 years ago by Jean-Paul Calderone

(In [34767]) - Remove trailing whitespace

  • Wrap long deprecation line to < 80 columns

refs #4963

comment:10 Changed 5 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to Jean-Paul Calderone

Thanks! I made a few small changes to the patch, mostly whitespace. Otherwise it looks good, so I'll apply it. Thanks again!

comment:11 Changed 5 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [34769]) Merge deprecate-redundant-ssl-context-4963

Author: moijes12 Reviewer: exarkun Fixes: #4963

Deprecate twisted.mail.protocols.SSLContextFactory.

Note: See TracTickets for help on using tickets.