classification
Title: SimpleXMLRPCServer.SimpleXMLRPCServer.register_function as decorator
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: xiang.zhang Nosy List: Claudiu.Popa, ahojnnes, berker.peksag, loewis, rhettinger, santoso.wijaya, serhiy.storchaka, xiang.zhang
Priority: normal Keywords: patch

Created on 2010-01-24 12:48 by ahojnnes, last changed 2017-03-31 16:36 by dstufft. This issue is now closed.

Files
File name Uploaded Description Edit
SimpleXMLRPCServer.py ahojnnes, 2010-01-24 12:48 SimpleXMLRPCServer.py
xmlrpc_register_decorator_py27.patch santoso.wijaya, 2011-03-09 02:53 register_function as a decorator and unittest usage review
xmlrpc_register_decorator_py33.patch santoso.wijaya, 2011-03-09 03:29 decorator implementation and unittest usage review
issue7769.patch xiang.zhang, 2016-06-15 15:22 review
issue7769_with_more_doc.patch xiang.zhang, 2016-07-07 17:24 review
Pull Requests
URL Status Linked Edit
PR 231 merged xiang.zhang, 2017-02-22 05:35
PR 703 larry, 2017-03-17 21:00
PR 552 closed dstufft, 2017-03-31 16:36
Messages (21)
msg98219 - (view) Author: Johannes Schönberger (ahojnnes) Date: 2010-01-24 12:48
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.
msg98230 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-01-24 16:23
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.
msg98233 - (view) Author: Johannes Schönberger (ahojnnes) Date: 2010-01-24 16:47
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/
msg98236 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-01-24 17:00
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.
msg98238 - (view) Author: Johannes Schönberger (ahojnnes) Date: 2010-01-24 17:09
OK, thank you for the links!

Do you still want me to do anything (like test cases etc.)?
msg98239 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-01-24 17:19
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.
msg98241 - (view) Author: Johannes Schönberger (ahojnnes) Date: 2010-01-24 17:21
OK, will work on it and reply as soon as I have results!
msg130403 - (view) Author: Santoso Wijaya (santoso.wijaya) * Date: 2011-03-09 02:53
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.
msg130406 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2011-03-09 03:04
Santoso - since this is a feature request it would need to be retargeted to 3.3
msg130411 - (view) Author: Santoso Wijaya (santoso.wijaya) * Date: 2011-03-09 03:29
I see. Attaching a patch against 3.3 tip, then.
msg130433 - (view) Author: Johannes Schönberger (ahojnnes) Date: 2011-03-09 09:09
sorry, I totally forgot about this...
msg266798 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-06-01 04:00
Here's some quick review comments:

* xmlrpc_register_decorator_py33.patch doesn't apply cleanly anymore.

* -        serv.register_function(my_function)
  +        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

* +            add_result, pow_result, div_result, \
  +                    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.
msg268620 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-06-15 15:22
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 `function` is 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.
msg269875 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-07-06 10:11
ping
msg269941 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-07-07 15:51
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.
msg269952 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-07-07 17:24
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.
msg274777 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-09-07 08:11
> 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.

I think this does not matter after seeing functools.singledispatch. The versionadded tags should be changed to versionchanged in my last patch.
msg288692 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-02-28 04:46
+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.
msg288696 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-28 06:46
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.
msg288701 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-28 09:15
Thanks everyone involved!
msg290359 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-03-24 23:34
New changeset 267b9d2fa8efce7c5bc34ce50048ebca8fddf04f by Xiang Zhang in branch 'master':
bpo-7769: enable xmlrpc.server.SimpleXMLRPCDispatcher.register_function used as decorator (GH-231)
https://github.com/python/cpython/commit/267b9d2fa8efce7c5bc34ce50048ebca8fddf04f
History
Date User Action Args
2017-03-31 16:36:30dstufftsetpull_requests: + pull_request1032
2017-03-24 23:34:46xiang.zhangsetmessages: + msg290359
2017-03-17 21:00:36larrysetpull_requests: + pull_request625
2017-02-28 09:15:31xiang.zhangsetstatus: open -> closed
resolution: fixed
messages: + msg288701

stage: patch review -> resolved
2017-02-28 06:46:01xiang.zhangsetassignee: xiang.zhang
stage: needs patch -> patch review
messages: + msg288696
versions: + Python 3.7, - Python 3.6
2017-02-28 04:46:46rhettingersetnosy: + rhettinger
messages: + msg288692
2017-02-22 05:35:26xiang.zhangsetpull_requests: + pull_request194
2016-09-07 08:11:52xiang.zhangsetmessages: + msg274777
2016-07-07 17:24:45xiang.zhangsetfiles: + issue7769_with_more_doc.patch

messages: + msg269952
2016-07-07 15:51:19serhiy.storchakasetmessages: + msg269941
2016-07-06 13:04:56brian.curtinsetnosy: - brian.curtin
2016-07-06 10:11:39xiang.zhangsetmessages: + msg269875
2016-06-18 06:07:58xiang.zhangsetnosy: + loewis, serhiy.storchaka
2016-06-15 15:22:12xiang.zhangsetfiles: + issue7769.patch

messages: + msg268620
2016-06-15 07:44:47xiang.zhangsetnosy: + xiang.zhang
2016-06-01 04:00:49berker.peksagsetversions: + Python 3.6, - Python 3.5
nosy: + berker.peksag

messages: + msg266798

stage: patch review -> needs patch
2014-06-17 17:03:28Claudiu.Popasetnosy: + Claudiu.Popa
2014-06-17 16:52:42Claudiu.Popasetstage: test needed -> patch review
versions: + Python 3.5, - Python 3.3
2011-03-09 09:09:32ahojnnessetnosy: brian.curtin, ahojnnes, santoso.wijaya
messages: + msg130433
2011-03-09 03:29:19santoso.wijayasetfiles: + xmlrpc_register_decorator_py33.patch
nosy: brian.curtin, ahojnnes, santoso.wijaya
messages: + msg130411
2011-03-09 03:04:19brian.curtinsetnosy: brian.curtin, ahojnnes, santoso.wijaya
messages: + msg130406
versions: + Python 3.3, - Python 2.7
2011-03-09 02:53:54santoso.wijayasetfiles: + xmlrpc_register_decorator_py27.patch
nosy: brian.curtin, ahojnnes, santoso.wijaya
messages: + msg130403

components: + Library (Lib), - Extension Modules
keywords: + patch
2011-03-08 06:07:40santoso.wijayasetnosy: + santoso.wijaya
2010-01-24 17:21:01ahojnnessetmessages: + msg98241
2010-01-24 17:19:15brian.curtinsetmessages: + msg98239
2010-01-24 17:09:05ahojnnessetmessages: + msg98238
2010-01-24 17:00:50brian.curtinsetmessages: + msg98236
2010-01-24 16:47:04ahojnnessetmessages: + msg98233
2010-01-24 16:23:33brian.curtinsetpriority: normal
versions: + Python 2.7, - Python 2.6
nosy: + brian.curtin

messages: + msg98230

stage: test needed
2010-01-24 12:48:44ahojnnescreate