Support for use_datetime in twisted.web.xmlrpc

[chadmaine]

Python's xmlrpclib.loads will convert xmlrpclib.DateTime objects to datetime.datetime objects when the use_datetime arg is set to True.

I've attached a simple diff that provides this functionality for twisted.web.xmlrpc.

[chadmaine]

Anyone had a chance to look at this patch? We continue to patch Twisted manually to use this feature.

Chad

[rion]

i also want this feature =)

[khorn]

This change seems reasonable. Does anyone else see a reason not to proceed with this? Will it conflict with anything else?

[chadmaine]

Updated against r28820

[exarkun]

Hi Chad, thanks for the update. The patch looks pretty good, but there are a couple changes it'll need before we can include it in trunk:

1. The new useDateTime attribute/parameter of XMLRPC and Proxy needs to be documented. Convention is to document it as an ivar on the class (rather than as a parameter to __init__). This should have been done for allowNone as well, by whoever added that.
2. The two new features (server-side and client-side datetime support) need unit tests. You can find the existing unit tests for this module in twisted/web/test/test_xmlrpc.py.
3. use_datetime was introduced in Python 2.5. Twisted still supports Python 2.4, so we need to do something about that case. Certainly we should not pass it to xmlrpclib if we're running on Python 2.4, as that will raise an exception. Possibly we should also raise an exception in XMLRPC and Proxy if we're running on Python 2.4 and the application passes useDateTime in, to mark this error as early as possible.
4. If you like, it'd also be helpful to include a news fragment for this change: http://twistedmatrix.com/trac/wiki/ReviewProcess#Newsfiles

Thanks again for the patch. To make sure the issue gets attention again quickly when you attach a new patch, please add the "review" keyword to it.

[chadmaine]

Updated diff with exarkun recommended changes, please review

[exarkun]

(In [28854]) Branching to 'xmlrpc-datetime-3219'

[exarkun]

Thanks for the updated patch! Some comments about it:

1. XMLRPCAllowNoneTestCase.setUp should have a docstring.
2. You can use TestCase.addCleanup in XMLRPCAllowNoneTestCase.setUp to get rid of the listening port, instead of using tearDown. It doesn't make much difference in this case, but it does mean you can completely get rid of tearDown (which means no one needs to write a docstring for it :).
3. Test method docstrings shouldn't start with "Test that ...". They can just describe the behavior being tested.
4. The allowNone tests and the useDateTime tests could probably be factored into a mixin pretty easily, reducing the total amount of code needed to test these two features.
5. The new tests need to skip if being run on Python 2.4, since they'll fail with a RuntimeError, but that's expected and correct.
6. There should also be tests for the RuntimeError raised if this code is running on Python 2.4.
7. Since XMLRPC is a classic class, property setters don't work on it. The check needs to be done in __setattr__.
8. There's a mistake in the new docs for useDateTime - they say "<= 2.5" but they mean ">= 2.5".
9. Most of the _QueryFactory docstring changes look good, but a few of the __init__ parameters don't actually end up as instance attributes, so these should stay as docs in __init__ (sorry if I wasn't clear about that).
10. _QueryFactory is private, so it probably doesn't need to be protected against a bad value for useDateTime; the checks in XMLRPC will make sure a bad value isn't passed in.

I'm going to make these changes and re-submit for review.

[exarkun]

Made the above changes, someone else please review.

[chadmaine]

Replying to exarkun:

Made the above changes, someone else please review.

Looking it over, it looks good. Thanks for your help, exarkun. I did notice the docs still have my 'Python <= 2.5' in them in a couple of places.

[itamar]

Looks good, other than same comment as chadmaine (you need to replace Python <= 2.5 with >= 2.5 in a few places.)

[exarkun]

(In [28880]) Merge xmlrpc-datetime-3219

Author: chadmaine, exarkun Reviewer: exarkun, chadmaine, itamar Fixes: #3219

Add support for representing timestamps received over XML-RPC as datetime instances, on versions of Python where xmlrpclib supports that.