[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Louie Lu]

Changes by Louie Lu :


--
pull_requests: +1598

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Changes by Dominic Mayers :


Removed file: 
http://bugs.python.org/file46775/Issue29947_for_discussion_03.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

An improved version of the patch, I hope. I will remove the old patch, because 
it's really does not help to see the old versions.

--
Added file: http://bugs.python.org/file46781/Issue29947_for_discussion_04.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Changes by Dominic Mayers :


Added file: http://bugs.python.org/file46775/Issue29947_for_discussion_03.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Changes by Dominic Mayers :


Removed file: 
http://bugs.python.org/file46774/Issue29947_for_discussion_02.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Changes by Dominic Mayers :


Removed file: http://bugs.python.org/file46770/Issue29947_for_discussion.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

I simplified the patch. Using a class as a factory function is the simplest 
case, so I took advantage of this. I also give one way to pass extra parameters 
to the handler in the factory function, because it's the main reason why we 
cannot always use a class.

--
Added file: http://bugs.python.org/file46774/Issue29947_for_discussion_02.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

The key point, IMHO, is that the BaseRequestHandler class is just provided as 
an option and its API (setup, handle and finish) is ignored by the code that we 
support. 

Some applications may have used the API, but these are details in applications. 
Simply, we keep BaseRequestHandler as it is so that we do not break these 
applications. 

So, yes, we should keep supporting the API, but we do not expect this API on 
our side. The latter is the key point.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

I started to look at the documentation to see what would need to be changed, 
assuming that we agree for a change in the API. Just for the purpose of this 
discussion, I created a patch that only change the comments in socketserver.py.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Changes by Dominic Mayers :


--
keywords: +patch
Added file: http://bugs.python.org/file46770/Issue29947_for_discussion.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Changes by Dominic Mayers :


Removed file: http://bugs.python.org/file46768/factorymixinclass

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[R. David Murray]

R. David Murray added the comment:

Yes, the difficulty in renaming the parameter was why I suggested a doc change 
only.  I'm not sure it it is worth it to go through a deprecation cycle for 
socketserver to change the name, though it certainly would be nice.  Martin and 
I could make that decision, but it would be better to get input from other 
devs.  And, if we make this the documentation issue, we should open a separate 
issue for the parameter rename.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

I did not think very far when said that renaming the parameter could not 
possibly break the code ! Oh well ! But, renaming the parameter was not 
important in itself.  It was to make the situation clearer and easier for those 
who write the documentation. Martin mentioned that it is a big change for the 
API. This is what I had in mind. And yes, I should have used 'factory 
function', not 'factory instance'. Oh well ! I am glad things are working and 
that we will move ahead to resolve this issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Martin Panter]

Martin Panter added the comment:

By “factory instance”, I presume you just mean a function (or class or method) 
that returns an appropriate object when called. (I think these are normally 
called “factory functions”. Instances are objects, the things that class 
constructors and factories return.)

What is being proposed? The best idea that I understand is hinted in 
. That is, to document and allow 
any function (not just a class or factory) to be passed as (or instead of) the 
RequestHandlerClass parameter.

This is a significant change to the documentation and API, but I would support 
it, and I think it should already be supported by the implementation.

The question of renaming the RequestHandlerClass parameter is a harder decision 
IMO. I think it would be easier to make the documentation clear that it is a 
misnomer and does not have to be a class. But in the long term, renaming the 
parameter seems nicer.

If it was “renamed”, we would first have to add a competing parameter, keep the 
old parameter around, consider adding deprecation warnings in 2020 when Python 
2 is dead, document both parameters, decide if anything should happen when both 
parameters are passed, etc. Much trickier, but possible. This renaming process 
is similar to “base64.encodestring” vs “encodebytes”, but more involved. I 
can’t think of an example where a function or constructor parameter was renamed 
like this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

Oops, I did not realize that David was one of the developers. Well, may be this 
needs the attention of more than one developer.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

The reason why I feel we should not make it immediately a documentation issue 
is that I don't know how a person working on documentation could proceed ahead 
without a clear go ahead signal from developers. In that sense,  having a 
documentation that says that the variable name does not reflect its intent 
seems tricky. It's very easy to change a variable name. It cannot possibly 
break the code, except for a collision, but they are easy to avoid.  If we 
don't have the interest of developers, not even for a simple renaming, I don't 
see a go ahead signal in this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[R. David Murray]

R. David Murray added the comment:

Given how old socket server is, and that it doesn't actually require that API, 
we should probably just reword the documentation so that it is clear that 
RequestHandlerClass is the default Handler implementation (and that you can 
work with setup/handle/finish if you want to subclass it), and document what 
the *actual* requirements on the Handler are.  

We are unlikely to change socketserver's actual API at this point.

I don't think there's any need for a new issue, let's just make this a 
documentation issue.  The original title still even applies :)  We will need 
signoff from someone with more direct interest in the socketserver module than 
me, but given that the "future" for the stuff socketserver handles is asyncio, 
I suspect the only objection we might get is "it's not worth it".  But if 
someone wants to do the work, *I* don't see a reason to reject it.

--
assignee:  -> docs@python
components: +Documentation
nosy: +docs@python
stage: resolved -> 
type: enhancement -> behavior
versions: +Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[R. David Murray]

R. David Murray added the comment:

Well, we could document it as a factory argument, and explain that the argument 
name is an historical artifact.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

To sum up, David clarified that we can in fact easily pass an arbitrary factory 
method that creates and starts a request handler, instead of a request handler 
class with setup, handle and finish in its API. This could indeed be a valid 
reason to consider this issue resolved,  but I would like to be sure that it is 
really a normal and supported use of the code. The only place where I can ask 
this is here. If I ask in StackOverflow, etc. I would get all kind of opinions  
and it would not be useful.  To be honest, looking at the parameter name, the 
documentation,  the examples, etc. it does not look at all a normal and 
supported use of the code, but who am I to decide that?  If it turns out to be 
a normal and supported use of the code, then I will create a different issue 
only about the documentation, because it's not clear at all.

--
nosy: +martin.panter

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

Perhaps I should raise a separate issue, but it is related, because the current 
code "requires" that we define an handler class with `setup()`, `handle()` and 
`finish()` in its API. If you look at the actual code, there is no such 
requirement. We only have to pass a factory method that receives three 
arguments, creates a new handler instance and starts it. The handler does not 
have to offer any API - you don't even have to subclass the RequestHandlerBase 
class. I still say that it "requires" it, because it is part of the 
documentation and, if you don't, you could be in trouble in future versions. 
However, this requirement is clearly annoying, because if you have an handler 
for another server, you need to re-factorize the code to provide this API, 
which is not even used. I am sure there is no such requirement, but it's not 
clear at first. 

This is clearly not just a documentation issue. The intent in the code is the 
concern of the designers of the code. David and I assume that the intent is 
that we can provide any arbitrary factory method that receives the three 
arguments, creates and starts the handler, but we can be mistaken. In fact, to 
me this looks as a hack, especially given the parameter name, which says that 
we must pass the handler class. An handler class is indeed a factory method, 
but it is a very specific one, not the one that we want to pass if we must 
include extra parameters.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

I am a bit ashamed that I missed that. Still, the intent in the current code, 
the name of the parameter, the examples, etc. is that we pass the handler 
class. This is more than its __init__ function and less than a generic factory  
method. An important difference, which could become important, is that with a 
factory the handler type could depend on the request.  As pointed out by Eric, 
passing the class was perhaps the intent in the early days. Now, perhaps many 
use it differently and pass a factory method, not a class, but it still appears 
as a hack that does not respect the intent. One could legitimately worry that 
this hack will not be supported in future versions, because it is not 
documented.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[R. David Murray]

R. David Murray added the comment:

I think I'm missing something here.  What prevents one from passing a factory 
function as the RequestHandlerClass argument?  In Python, a class name *is* a 
factory function for class instances.

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

On the other hand, it occurs to me that this seems way more flexible than 
passing the object through the server, because you share the factory with the 
server, not only the object. This means that you could even change the type of 
the handler while the server is running !

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Changes by Dominic Mayers :


--
resolution: wont fix -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

Finally, I looked around and people just use the server to pass any extra 
parameter. I do find it awkward, but it works and it is simple, in fact simpler 
than having to define a factory object. I don't close it, because I will be 
happy to see another opinion. I would use this factory approach for myself, 
because I am just not comfortable to add an attribute to a server that would 
not even look at it, but maybe that it just me.

--
resolution:  -> wont fix
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

Dominic Mayers added the comment:

One way to make the factory optional is to offer a MixIn. I attached a file 
with a FactoryMixIn. It's just that I find it awkward that the proposed 
approach is to pass the extra parameters in a subclassed server. More modern 
approaches should also be offered.

--
resolution: rejected -> 
status: closed -> open
Added file: http://bugs.python.org/file46768/factorymixinclass

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Eric V. Smith]

Eric V. Smith added the comment:

Just a guess, but it's because this code was written at a time when subclassing 
was the preferred way to extend code. Now we know better :), but it's not 
possible to change history, unfortunately. Lots of code depends on the current 
behavior.

If you find a way to make a factory function work, while preserving backward 
comparability, then we can re-open this issue and consider the enhancement 
request then.

--
nosy: +eric.smith
resolution:  -> rejected
stage:  -> resolved
status: open -> closed
versions:  -Python 2.7, Python 3.3, Python 3.4, Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29947] In SocketServer, why not passing a factory instance for the RequestHandlerClass instead of the class itself?

[Dominic Mayers]

New submission from Dominic Mayers:

I am just curious to know if someone considered the idea of passing a factory 
instance that returns RequestHandlerClass instances instead of directly passing 
the class? It may affect existing handlers that read non local variables, but 
there should be a way to make the factory optional. The purpose is only 
aesthetic and a better organization of the code. I find it awkward to have to 
subclass the server every time that we have an handler that needs special 
objects, a database connection, a socket connection to another party, etc. The 
server class should have a single purpose: accept a request and pass it to an 
handler. We should only need to subclass a server when we need to do that in a 
different way : TCP vs UDP, Unix Vs INET, etc. The usage is simpler and more 
natural. Instead of subclassing the server, we create a factory for the handler.

--
components: Library (Lib)
messages: 290840
nosy: dominic108
priority: normal
severity: normal
status: open
title: In SocketServer, why not passing a factory instance for the 
RequestHandlerClass  instead of the class itself?
type: enhancement
versions: Python 2.7, Python 3.3, Python 3.4, Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com