On Jul 2, 2014, at 9:26 AM, [email protected] wrote:
> Hello all,
>
> twisted.web.http.Request.getClient has a terrible implementation. It does
> blocking network I/O (DNS). Fortunately it is only used in one place in
> Twisted - the CGI implementation. Unfortunately this makes the CGI
> implementation somewhat unsuited for real-world use.
>
> `Request.getClient` has always been allowed to return `None` under certain
> circumstances. I propose making it always return `None` and deprecating it.
>
> This is implemented in the branch linked to <https://tm.tl/2252>.
>
> Chris Armstrong suggested that this change might not be strictly keeping with
> our backwards compatibility policy.
I agree that this is a troubling area but in general I tend to believe that
changes like this are in keeping with our compatibility policy. Changing the
signature or the allowed return type of a value ("type" speaking in terms of
public features of its interface) should not be allowed. If we already
returned None sometimes, then a correct program would already have to deal with
None sometimes, so making this change wouldn't break it, per se.
Bringing up such a change on the list is always a good policy, since it gives
people a chance to audit their code and look for places where they might have
been depending too intimately on accidental features, so by no means take my
belief that this is in-policy to mean that we shouldn't broadly discuss changes
like this in the future :-). Real, actual, broken code is what the policy
strives to prevent, so real world code that broke should usually take
precedence.
> I suggest that either it is - because `None` was always a possible return
> value - or that removing the possibility of blocking I/O from applications
> that are mistakenly using this API makes it worth the not-
> strictly-compatible change.
>
> A minor adjustment might be to make it always return the IP address instead,
> as this was another behavior it previously had.
I think that this adjustment is the best option. IP addresses are mostly
interchangeable with hostnames, so during the transition period while it's
being deprecated, even an application relying on this API heavily would at
least have an opportunity to keep functionality equivalent during an upgrade.
Making it always return None means that a correct application (one which dealt
with the None return value), while not becoming crash-with-an-exception buggy,
might lose functionality (logging a source IP of "None" all the time, for
example, and losing track of an audit log of who is making what changes).
Unbidden, I have some ideas about how we might preserve even _more_ of the
functionality involving DNS lookups, but more effort than just giving back the
IP is probably wasted, so I won't mention them. Let's deprecate the API and
move on.
Thanks for bringing this up,
-glyph
_______________________________________________
Twisted-Python mailing list
[email protected]
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
