Opened 5 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: Jeff White
Priority: normal Milestone:
Component: core Keywords:
Cc: Jeff White 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 Jeff White 4 years ago.
Differences for migration of TCP4, SSL, and UNIX endpoints string parsers to plugins
5720.feature (157 bytes) - added by Jeff White 4 years ago.
Addition to topfiles directory
5720-2.patch (40.0 KB) - added by Jeff White 3 years ago.
Updated patch

Download all attachments as: .zip

Change History (11)

comment:1 Changed 5 years ago by ashfall

Owner: set to ashfall

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

Changed 4 years ago by Jeff White

Attachment: 5720-1.patch added

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

comment:2 Changed 4 years ago by Jeff White

Cc: Jeff White added
Keywords: review added

comment:3 Changed 4 years ago by ashfall

Owner: ashfall deleted

Changed 4 years ago by Jeff White

Attachment: 5720.feature added

Addition to topfiles directory

comment:4 Changed 3 years ago by Glyph

Author: glyph
Branch: branches/parser-to-plugin-5720

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

comment:5 Changed 3 years ago by Glyph

Keywords: review removed
Owner: set to Jeff White

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 Jeff White

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 Jeff White

Attachment: 5720-2.patch added

Updated patch

comment:8 Changed 3 years ago by Jeff White

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.