Opened 4 years ago

Last modified 3 years ago

#5720 enhancement new

Convert parsers for UNIX, SSL, and TCP (IPv4) into plugins

Reported by: ashfall Owned by: BurboBaggins
Priority: normal Milestone:
Component: core Keywords:
Cc: BurboBaggins Branch: branches/parser-to-plugin-5720
branch-diff, diff-cov, branch-cov, buildbot
Author: glyph

Description

There are a few parsers in twisted/internet/endpoints.py that aren't plugins, and were written before the plugin system was set. These should be converted into plugins.

Attachments (3)

5720-1.patch (33.5 KB) - added by BurboBaggins 3 years ago.
Differences for migration of TCP4, SSL, and UNIX endpoints string parsers to plugins
5720.feature (157 bytes) - added by BurboBaggins 3 years ago.
Addition to topfiles directory
5720-2.patch (40.0 KB) - added by BurboBaggins 3 years ago.
Updated patch

Download all attachments as: .zip

Change History (11)

comment:1 Changed 4 years ago by ashfall

  • Owner set to ashfall

Assigning to self for the moment, feel free to take this up if you want.

Changed 3 years ago by BurboBaggins

Differences for migration of TCP4, SSL, and UNIX endpoints string parsers to plugins

comment:2 Changed 3 years ago by BurboBaggins

  • Cc BurboBaggins added
  • Keywords review added

comment:3 Changed 3 years ago by ashfall

  • Owner ashfall deleted

Changed 3 years ago by BurboBaggins

Addition to topfiles directory

comment:4 Changed 3 years ago by glyph

  • Author set to glyph
  • Branch set to branches/parser-to-plugin-5720

(In [41726]) Branching to parser-to-plugin-5720.

comment:5 follow-up: Changed 3 years ago by glyph

  • Keywords review removed
  • Owner set to BurboBaggins

Thanks very much for the patch BurboBaggins!

Looks like buildbot has spotted some problems, unfortunately:

  1. The dependency on OpenSSL is supposed to be soft, not hard; some of the buildbots are failing because you've added a test that isn't skipped if it's not installed.
  2. twistedchecker has many complaints about epytext markup.
  3. pyflakes found a redefinition which means a test will be skipped.
  4. This broke Py3 support pretty badly.
  5. This should probably block until the ticket fixing the broken signature of `parseStreamClient` is landed. It's already approved with some minor feedback, so if habnabit doesn't land it soon I'll probably do it myself.

Otherwise this looks good though, nice to get this out of the way, I look forward to re-reviewing soon.

comment:6 in reply to: ↑ 5 Changed 3 years ago by BurboBaggins

Will try to knock this out while I am at PyTennessee this weekend. Thanks for your comments.

Thanks very much for the patch BurboBaggins!

Looks like buildbot has spotted some problems, unfortunately:

  1. The dependency on OpenSSL is supposed to be soft, not hard; some of the buildbots are failing because you've added a test that isn't skipped if it's not installed.
  2. twistedchecker has many complaints about epytext markup.
  3. pyflakes found a redefinition which means a test will be skipped.
  4. This broke Py3 support pretty badly.
  5. This should probably block until the ticket fixing the broken signature of `parseStreamClient` is landed. It's already approved with some minor feedback, so if habnabit doesn't land it soon I'll probably do it myself.

Otherwise this looks good though, nice to get this out of the way, I look forward to re-reviewing soon.

comment:7 Changed 3 years ago by habnabit

(In [41738]) Merge parseStreamClient-reactor-5069-4: Add IStreamClientEndpointStringParserWithReactor, deprecating IStreamClientEndpointStringParser.

Author: habnabit Reviewers: exarkun, ashfall Fixes: #5069 Refs: #5720

The new interface IStreamClientEndpointStringParserWithReactor will supply the reactor to its parseStreamClient method, passed along from twisted.internet.endpoints.clientFromString. The old interface, IStreamClientEndpointStringParser, which did not supply the reactor, is now deprecated.

Changed 3 years ago by BurboBaggins

Updated patch

comment:8 Changed 3 years ago by BurboBaggins

I think the latest attachment should take care of it. I do have a question for you regarding this error:

twisted/internet/test/test_endpoints.py:2491: redefinition of unused 'test_sslDHparameters' from line 2470

I could not find that function in my copy of test_endpoints.py. Is this error a product of a merge with another version of the file?

Note: See TracTickets for help on using tickets.