Opened 2 years ago

Last modified 6 months 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: jeladi@… Branch: branches/parser-to-plugin-5720
(diff, github, buildbot, log)
Author: glyph Launchpad Bug:

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 7 months ago.
Differences for migration of TCP4, SSL, and UNIX endpoints string parsers to plugins
5720.feature (157 bytes) - added by BurboBaggins 7 months ago.
Addition to topfiles directory
5720-2.patch (40.0 KB) - added by BurboBaggins 6 months ago.
Updated patch

Download all attachments as: .zip

Change History (11)

comment:1 Changed 2 years ago by ashfall

  • Owner set to ashfall

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

Changed 7 months ago by BurboBaggins

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

comment:2 Changed 7 months ago by BurboBaggins

  • Cc jeladi@… added
  • Keywords review added

comment:3 Changed 7 months ago by ashfall

  • Owner ashfall deleted

Changed 7 months ago by BurboBaggins

Addition to topfiles directory

comment:4 Changed 7 months 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 7 months 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 7 months 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 6 months 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 6 months ago by BurboBaggins

Updated patch

comment:8 Changed 6 months 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.