Add a string endpoint description plugin for twisted.internet.endpoints.TCP6ServerEndpoint

[ashfall]

A string endpoint description plugin for this TCP server endpoint that supports IPv6 addresses (added in #5694) is needed so that the endpoint is accessible from the command-line.

[ashfall]

(In [34609]) Branching to 'tcp6-server-string-description-plugin-5699'

[ashfall]

 Build Results

[exarkun]

Thanks! This looks pretty good. A couple things are missing though, I think:

1. There aren't any tests that actually exercise the new parser. See, eg, _parseStreamServerTest (combined with, eg, test_parseStreamServerINET) in SystemdEndpointPluginTests. In other words, a test should call parseStreamServer with a "tcp6:" endpoint string description.
2. Hard-coding the backlog to 50 doesn't seem necessary. I don't think the backlog argument is actually very important (almost no one ever wants to specify a value for it), but I wonder why you didn't allow it to be supplied in the string. "tcp:1234:backlog=50" works for TCP4 servers, so it's surprising for it to not work for TCP6 servers, I think. On the other hand, if we want to make an explicit, intentional decision to not support it in TCP6, and drop support for it in TCP4 eventually, that may be fine too. So, mostly I wonder why it's hard-coded instead of parameterized. (Also, it is documented as a parameter in _parseServer.)
3. The new class is private, and generally we don't need to bother to advertise private classes in news fragments. More importantly is the end-user facing feature, which is that IPv6 TCP servers can now be created using the endpoint string description feature. That's what the news fragment should talk about (perhaps mentioning twisted.internet.endpoints.serverFromString).

Thanks again.

[ashfall]

Replying to exarkun:

1. There aren't any tests that actually exercise the new parser. See, eg, _parseStreamServerTest (combined with, eg, test_parseStreamServerINET) in SystemdEndpointPluginTests. In other words, a test should call parseStreamServer with a "tcp6:" endpoint string description.

Added a test that uses _parseStreamServerTest in TCP6ServerEndpointPluginTests, I think that should suffice.

Fixed the other issues as well.

[exarkun]

Thanks. There's still no good test coverage for the actual format for this endpoint plugin parser. "8080" is not a TCP6 server endpoint string! It's a TCP4 server endpoint string. test_parseStreamServerPort cheats by instantiating endpoints._TCP6ServerParser instead of calling serverFromString.

[ashfall]

Added the said test.

[exarkun]

Thanks! Looks great now. Please merge. :)

[ashfall]

(In [34680]) Merge tcp6-server-string-description-plugin-5699: A string description plugin for the TCP6ServerEndpoint.

Author: ashfall Reviewer: exarkun Fixes: #5699

Added a string endpoint description plugin for the TCP6ServerEndpoint so that it can be accessed from serverFromString.