Repository navigation
SimpleXMLRPCServer.SimpleXMLRPCServer.register_function as decorator #52017
Description
| BPO | 7769 |
|---|---|
| Nosy | @loewis, @rhettinger, @PCManticore, @berkerpeksag, @serhiy-storchaka, @zhangyangyu |
| PRs | Misc/NEWS so that it is managed by towncrier #552 |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/zhangyangyu'
closed_at = <Date 2017-02-28.09:15:31.500>
created_at = <Date 2010-01-24.12:48:44.749>
labels = ['3.7', 'type-feature', 'library']
title = 'SimpleXMLRPCServer.SimpleXMLRPCServer.register_function as decorator'
updated_at = <Date 2017-03-31.16:36:30.456>
user = 'https://bugs.python.org/ahojnnes'bugs.python.org fields:
activity = <Date 2017-03-31.16:36:30.456>
actor = 'dstufft'
assignee = 'xiang.zhang'
closed = True
closed_date = <Date 2017-02-28.09:15:31.500>
closer = 'xiang.zhang'
components = ['Library (Lib)']
creation = <Date 2010-01-24.12:48:44.749>
creator = 'ahojnnes'
dependencies = []
files = ['15986', '21057', '21058', '43401', '43655']
hgrepos = []
issue_num = 7769
keywords = ['patch']
message_count = 21.0
messages = ['98219', '98230', '98233', '98236', '98238', '98239', '98241', '130403', '130406', '130411', '130433', '266798', '268620', '269875', '269941', '269952', '274777', '288692', '288696', '288701', '290359']
nosy_count = 8.0
nosy_names = ['loewis', 'rhettinger', 'ahojnnes', 'santoso.wijaya', 'Claudiu.Popa', 'berker.peksag', 'serhiy.storchaka', 'xiang.zhang']
pr_nums = ['231', '703', '552']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue7769'
versions = ['Python 3.7']
Activity
ahojnnes commentedon Jan 24, 2010
I would suggest to make SimpleXMLRPCServer.SimpleXMLRPCServer.register_function a decorator function.
See the attached file for the solution I wrote (l.209-240), which also works with the current syntax:
@server.register_function
@server.register_function('name')
@server.register_function(name='name')
or:
server.register_function(func)
server.register_function(func, name='name')
server.register_function(function=func, name='name')
So as far as I've tested it (py2.6), it is fully backwards compatible and supports decorators in addition.
briancurtin commentedon Jan 24, 2010
Can you submit your patch as a unified diff -- it's not easy to tell where you made changes to that file.
Can you also include tests showing how this is used, especially using register_function in both the old way and the new way you propose? Lib/test/test_xmlrpc.py is a good place to put your tests.
ahojnnes commentedon Jan 24, 2010
I'm not very used to working with bug/issue trackers, is there any tutorial here, where this is explained?
I did the stuff you asked me to do:
diff: http://paste.pocoo.org/compare/169357/169359/
test: http://paste.pocoo.org/show/169360/
briancurtin commentedon Jan 24, 2010
http://www.python.org/dev/workflow/ and http://python.org/dev/faq/ should help you get started. The workflow doc will let you know what the process is around here. The FAQ will tell you how to setup Subversion, how to make a diff, etc.
With that said, I think your change could be useful. Being that it's a feature request, it will go into 2.7 if accepted.
ahojnnes commentedon Jan 24, 2010
OK, thank you for the links!
Do you still want me to do anything (like test cases etc.)?
briancurtin commentedon Jan 24, 2010
Once you get a Subversion checkout of the source, have a look in Lib/test/test_xmlrpc.py for examples of how things are currently tested (using unittest). Once you get a feel for it, add new tests for what your changes do. The file you paste'd looks good as an example, you just have to make actual tests out of it now.
ahojnnes commentedon Jan 24, 2010
OK, will work on it and reply as soon as I have results!
santosowijaya commentedon Mar 9, 2011
I'm attaching a patch against 2.7 tip for an initial implementation of this decorator feature as well as sample usage in unittest, to get the ball rolling.
The modified function should work as a decorator while preserving backward compatibility to be used in a traditional method call.
briancurtin commentedon Mar 9, 2011
Santoso - since this is a feature request it would need to be retargeted to 3.3
santosowijaya commentedon Mar 9, 2011
I see. Attaching a patch against 3.3 tip, then.
ahojnnes commentedon Mar 9, 2011
sorry, I totally forgot about this...
berkerpeksag commentedon Jun 1, 2016
Here's some quick review comments:
xmlrpc_register_decorator_py33.patch doesn't apply cleanly anymore.
+ serv.register_function(_my_function, name='my_function')
We should keep
serv.register_function(my_function)as is and add a separate line that uses the new 'name' parameter.We can now change set([...]) to {...] in test_introspection1
+ myfunc_result, myfunc2_result = multicall()
No need to use the \ character.
The docstring of Lib/xmlrpc/server.py is already too long. It's better not to update it.
Please use the existing code style (name = None -> name=None)
We can make register_function act both as a decorator and as a decorator factory without changing its signature.
We need to add a test to cover TypeError case.
Documentation is missing. See Doc/library/xmlrpc.server.rst.
Please add a note to Doc/whatsnew/3.6.rst.
zhangyangyu commentedon Jun 15, 2016
Hi, I've written a patch to accomplish this in Py3.6.
This patch is much cleaner but has one drawback, when used as decorator factory, you have to specify name as a keyword argument. But considering the codes that have to been imported to check arguments, I think it's not bad. (I don't want to check
functionis a string or not.)Also one side effect that I can not eliminate is when it is used as a normal function, the function instead of None is returned. I see the former patches get this problem too.
Hope to get feedback.
zhangyangyu commentedon Jul 6, 2016
ping
serhiy-storchaka commentedon Jul 7, 2016
I don't have an opinion about needing this feature. But for accepting it needs more documenting. The signature should be updated, the new feature should be documented in the main text, not only in the versionchanged note. Needed examples of using egister_function as decorator.
zhangyangyu commentedon Jul 7, 2016
From the old comments from Brian it seems he's willing to accept such a feature. But unfortunately Brian seems not interested in this now. IMHO, this is good feature especially to people familiar with Bottle and Flask. Writing in a decorator way seems more elegant to me. Nevertheless, add more doc.
zhangyangyu commentedon Sep 7, 2016
I think this does not matter after seeing functools.singledispatch. The versionadded tags should be changed to versionchanged in my last patch.
rhettinger commentedon Feb 28, 2017
+1 for the decorator idea. It feels very natural.
A little off topic, I do not like the with-statement example that we currently have in the docs. I think it is bad design to put some much code inside with-block. In the micro-webframeworks, we register functions separately from launching the server. The same practice should apply here as well.
zhangyangyu commentedon Feb 28, 2017
Thanks Raymond. I would like to keep the example style another issue, not this one. :-) And I hope someone is willing to review the PR.
zhangyangyu commentedon Feb 28, 2017
Thanks everyone involved!
zhangyangyu commentedon Mar 24, 2017
New changeset 267b9d2 by Xiang Zhang in branch 'master':
bpo-7769: enable xmlrpc.server.SimpleXMLRPCDispatcher.register_function used as decorator (GH-231)
267b9d2