Discussion:
[Twisted-Python] How to handle arguments with undocumented types and argument deprecation
Adi Roiban
2015-10-25 08:34:15 UTC
Permalink
Hi,

This message is based on https://twistedmatrix.com/trac/ticket/5911#comment:14

In twisted/web/http.py we have the addCookie method which has the
'max_age' argument.

In the current code, the max_age is used as just anything which can be
converted into a text.

cookie = cookie +"; Max-Age=%s" % max_age

The latest patch documents max_age as requiring an int and adds tests
for the addCookie method... so to handly Py3 support the code looks
like this:

cookie = cookie + b"; Max-Age=" + intToBytes(max_age)

-----------

Should this code continue to handle str/int/long ?

---------

Is there a way to deprecate a single argument from a method/function?

Since max_age does not comply with the coding convention, can we
deprecated max_age with support for str/int/log and create a new
maxAge argument which only supports bytes.


Thanks!
--
Adi Roiban
Glyph Lefkowitz
2015-10-26 02:30:57 UTC
Permalink
Post by Adi Roiban
Is there a way to deprecate a single argument from a method/function?
Sometimes.

The idea with a deprecation is that you can remove something, emit a warning if it's used, then eventually remove it entirely, and get an error if it's used. The trick with deprecating parameters is what happens when you get to "remove it entirely".

One of the features I'm looking forward to in Py3 is "keyword-only parameters". If we had those then deprecating a parameter (as long as it's not part of a formal interface) would be easier. As it is, there's a problem where you can have a function like this:

def foo(a, broken=None, c=None):
...

If you want to deprecate "broken", you have to ensure that callers who call 'foo' will get a deprecation warning if they pass 'broken' at all, to ensure that callers who do, for example:

foo(a, None, "something valid")

get a deprecation warning during the deprecation period. Further, after the deprecation period, you need to get an error if you "pass broken", which doesn't exist any more. Of course if you've removed it from the function signature, it will give you an error to pass it by keyword, but if you pass it positionally:

foo(a, "something valid for broken")

if you've skipped the deprecation period, the spirit of the policy is that you should get an exception here. Normally this happens as a natural effect of the removal. But in this case, if you've changed the signature of 'foo' to be

def foo(a, c=None):
...

then you have a problem where code that skips the deprecation period might appear to work and break in unpredictable ways later. So when deprecating a parameter you have to be more careful than when deprecating a whole method or class.

Except, oops: addCookie() is part of a formal interface, IRequest. Which means that if external users of Twisted are implementing IRequest, then they may keep accepting / requiring max_age in its old format.

So specifically in the case of max_age, I think we should just retrofit the code to handle str/int/long on py3, to reduce breakage. At some point, we know we need a new, better IRequest, and we should ensure maxAge is better-specified on that interface.

-glyph

Loading...