<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jun 14, 2011, at 2:37 PM, <a href="mailto:exarkun@twistedmatrix.com">exarkun@twistedmatrix.com</a> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>On 07:37 am, <a href="mailto:albert.brandl@weiermayer.com">albert.brandl@weiermayer.com</a> wrote:<br><blockquote type="cite"></blockquote></div></blockquote><br><blockquote type="cite"><div><blockquote type="cite">We've defined a constant GPS_PORT as "SERIAL:/dev/ttyS2:baudrate=4800"<br></blockquote><blockquote type="cite">for the production environment and "tcp:1049" for the test environment<br></blockquote><br>To be clear, though, it sounds like you're defining GPS_PORT in a .py <br>file, and that's more or less what I was speaking against. :) Instead, <br>I would define GPS_PORT as either SerialEndpoint(reactor, "/dev/ttyS2", <br>baudrate=4800) or TCP4ServerEndpoint(reactor, 1049).<br></div></blockquote><div><br></div><div>In a rare show of dissent within the cabal[1], I disagree with Jean-Paul on this.</div><div><br></div><blockquote type="cite"><div>The rest of your code can still be indifferent to which of these is in play (and you can have a service for either of these using StreamServerEndpointService).</div></blockquote><div><br></div><div>This is the most important point to take away, and I agree with it. As much as possible, code should interact with endpoints as parameters. If you need to pass something around, pass the endpoint or the service itself, not the string or the parameters used to construct it.</div><br><blockquote type="cite"><div>If you misspell something or leave off a required argument, though, you're more likely to get an exception that points more directly at the problem.<br></div></blockquote><div><br></div><div>First, if you find bad error reporting while using the string-using APIs, it would be better to file a bug for improved error handling than to switch to hard-coding your endpoint types. There's no particular reason that error-reporting needs to be worse in that case.</div><div><br></div><div>Second, I hate to say so, but the current error-reporting behavior wasn't designed particularly thoughtfully in either system, so in practice, sometimes you'll get better reporting from one and in some you'll get it from the other. For example:</div><div><br></div></div><blockquote class="webkit-indent-blockquote" style="margin: 0 0 0 40px; border: none; padding: 0px;"><div><div><div>>>> from twisted.internet.endpoints import TCP4ServerEndpoint, serverFromString</div><div>>>> from twisted.internet import reactor</div><div><div>>>> serverFromString(reactor, 'bananas')</div><div> ...</div><div>ValueError: Unqualified strport description passed to 'service'. Use qualified endpoint descriptions; for example, 'tcp:bananas'.</div></div><div><div>>>> serverFromString(reactor, 'tcp:bananas')</div><div> ...</div><div>ValueError: invalid literal for int() with base 10: 'bananas'</div></div><div><div>>>> TCP4ServerEndpoint(reactor, 'bananas')</div><div><twisted.internet.endpoints.TCP4ServerEndpoint object at 0x103629190></div></div><div><div>>>> TCP4ServerEndpoint(reactor)</div><div> ...</div><div>TypeError: __init__() takes at least 3 arguments (2 given)</div></div><div><div>>>> serverFromString(reactor, 'tcp:')</div><div>Traceback (most recent call last):</div><div> ...</div><div>ValueError: invalid literal for int() with base 10: ''</div></div><div><div>>>> serverFromString(reactor, "bananas:")</div><div> ...</div><div>ValueError: Unknown endpoint type: 'bananas'</div></div></div></div></blockquote><div><div><br></div><div>Both of these really *should* be saying: "'bananas' is not a valid TCP port number" or "TCP requires a port number". (Python's error reporting could use some improving here too: which __init__, exactly?)</div><div><br></div><div>Personally, I recommend that most developers prefer the string-based APIs, because they allow for more flexibility with less code. (Granted, not a <i>lot</i> less code: just the N imports for each endpoint type you might support vs. the 1 import for serverFromString or strports.service.)</div><div><br></div><div>When you create a TCP4ServerEndpoint, that is an object with a very specific contract. However, an endpoint created with the "tcp:" prefix passed to serverFromString has an opportunity for more dynamic behavior. The former must always bind to TCPv4 sockets. The latter, however, may develop different behavior. For example, IPv6 support "for free" would be possible with this API, because by using the string-based APIs you are explicitly saying "I am passing some input in here, and I don't know what kind of stream-based transport I might get". The compatibility contract with this code is much weaker, because it supports plugins, and <i>already</i> might be giving you lots of different kinds of objects depending on your configuration.</div><div><br></div><div>The place where I would recommend <i>against</i> using the string-based APIs is in systems which require knowledge of their transports. For example, if you want to report about the apparent address of your peer in a protocol message with a specific format, you can't correctly do that with an arbitrary endpoint, because you won't know what datatype to expect from the getPeer() and getHost() methods on your transport. You can't accept any endpoint that serverFromString might give you and then assume it will give you an IPv4Address as a peer.</div><div><br></div><div>So, you should be as flexible as you can, but no more.</div><div><br></div><blockquote type="cite"><div>Put another way, I suggest writing this:<br><br> if debug_mode():<br> gps_port = TCP4ServerEndpoint(reactor, 1049)<br> else:<br> gps_port = SerialEndpoint(reactor, "/dev/ttyS2", baudrate=4800)<br><br> ...<br><br> gps_service = StreamServerEndpointService(gps_port, gps_factory)<br><br>And save the "tcp:1049"-style strings for your .ini files where you <br>cannot write it that way.<br></div></blockquote><div><br></div>This then begs the question: where does "debug_mode()"'s result come from? Presumably someone had to indicate a boolean to this code somehow. It would be simpler for whoever was supplying the value for debug_mode to just supply a value for the endpoint string description instead. And if it's OK to hard-code the debug_mode boolean, why not just hard-code the string? Since gps_factory already needs to accept being connected to different transport types, this is 8 lines of code (including the imports, not shown above) where 3 will do fine.</div><div><br></div><div>Can this code really only run over plain TCP and serial ports? If not, then you might have to add an 'if secure()' to the TCP case. And then you suddenly need a way to get a certificate in there, and all kinds of other information related to TLS. Better to let the existing string-parsing configuration system do that for you.</div><div><br></div><div>-glyph<br><div><br></div>[1]: There is no cabal.<br><br></div></body></html>