[Twisted-Python] twisted/runner/inetdtap.py pyflakes warnings cleanup

[Adi Roiban]

Hi,

Right now twisted/runner/inetdtap.py has the following errors reported by
pyflakes:

$ pyflakes twisted/runner/inetdtap.py
twisted/runner/inetdtap.py:58: undefined name 'rpcVersions'
twisted/runner/inetdtap.py:59: undefined name 'name'
twisted/runner/inetdtap.py:62: undefined name 'p'
twisted/runner/inetdtap.py:152: undefined name 'proto'

From what I can see from reading the code, RPCServer.startService is broken.

makeService is also broken for rpc/* services .

There are no tests for all this code.

Link for the code :
https://github.com/twisted/twisted/blob/trunk/twisted/runner/inetdtap.py

----------

I am working on getting the Twisted code clean of pyflakes errors/warnings
so that contributors could run pyflakes check on their own branch, without
relying on buildbot.

I would like to clean the inetdtap.py module of pylakes warning but since
it is broken and has no tests, I have no idea what this code is expected to
do and how end users are expecting to use it.

Does someone volunteer to fix it?

Otherwise, has anyone any objections against deprecating
twisted/runner/inetdtap.py?
Since it is broken, can we just remove it?

Thanks!

[Glyph Lefkowitz]

Post by Adi Roiban
Hi,
$ pyflakes twisted/runner/inetdtap.py
twisted/runner/inetdtap.py:58: undefined name 'rpcVersions'
twisted/runner/inetdtap.py:59: undefined name 'name'
twisted/runner/inetdtap.py:62: undefined name 'p'
twisted/runner/inetdtap.py:152: undefined name 'proto'
From what I can see from reading the code, RPCServer.startService is broken.
makeService is also broken for rpc/* services .
There are no tests for all this code.
https://github.com/twisted/twisted/blob/trunk/twisted/runner/inetdtap.py <https://github.com/twisted/twisted/blob/trunk/twisted/runner/inetdtap.py>
----------
I am working on getting the Twisted code clean of pyflakes errors/warnings so that contributors could run pyflakes check on their own branch, without relying on buildbot.

Thank you. This would be absolutely great.

Post by Adi Roiban
I would like to clean the inetdtap.py module of pylakes warning but since it is broken and has no tests, I have no idea what this code is expected to do and how end users are expecting to use it.

The purpose of inetdtap is to provide an 'inetd'-like server, to allow Twisted to invoke other programs to handle incoming sockets. This is a useful thing as a "networking swiss army knife" tool, like netcat. In fact, it does work, after a fashion; I wrote a file like this:

8123 stream tcp wait glyph /bin/cat -

and then ran this:

put a blank line into 'rpc.conf', and then ran:

twistd -n inetd -f sampleinetd.conf -r rpc.conf

and port 8123 properly became an echo server.

Now, in order to do this, you need a /etc/services file (this is hard-coded) that can be parsed by ServicesConf, which is _extremely_ picky, and probably the one that your OS comes with is still broken anyway. But this code can work if properly configured, and those parts can be fixed.

I haven't run this recently, but partially only because I basically forgot it existed :). Thanks for the reminder.

Post by Adi Roiban
Does someone volunteer to fix it?

I'll gladly fix the parts that implement the possibly-useful functionality, although you don't need to touch those in order to fix the pyflakes warnings :). The parts that are totally broken, RPCServicesConf, RPCServer, and the code that instantiates it, are useful only to NFS server implementors, and don't work at all no matter your configuration. So I would delete the whole implementation just so someone will get a clear notification in case they were importing one of these names but not actually using them, deprecate the module attributes, and remove them in the next available removal cycle. So basically just leave an empty Service subclass there just as a courtesy (since that is slightly more polite, and only a tiny bit harder than just deleting it).

Post by Adi Roiban
Otherwise, has anyone any objections against deprecating twisted/runner/inetdtap.py?
Since it is broken, can we just remove it?

I hope my suggestion makes sense and is useful. However if you'd really like to do the compat-breaking dance I won't object; I seriously doubt anyone is touching the RPC code. I'd prefer you don't delete the whole module though.

-glyph

[Adi Roiban]

On 23 November 2015 at 03:13, Glyph Lefkowitz <***@twistedmatrix.com>
wrote:

[snip]

Post by Glyph Lefkowitz
Thank you. This would be absolutely great.

https://twistedmatrix.com/trac/ticket/8107 is waiting for review... and
should make this happen :)

Post by Glyph Lefkowitz
I would like to clean the inetdtap.py module of pylakes warning but since
it is broken and has no tests, I have no idea what this code is expected to
do and how end users are expecting to use it.
The purpose of inetdtap is to provide an 'inetd'-like server, to allow
Twisted to invoke other programs to handle incoming sockets. This is a
useful thing as a "networking swiss army knife" tool, like netcat. In
8123 stream tcp wait glyph /bin/cat -
twistd -n inetd -f sampleinetd.conf -r rpc.conf
and port 8123 properly became an echo server.
Now, in order to do this, you need a /etc/services file (this is
hard-coded) that can be parsed by ServicesConf, which is _extremely_ picky,
and probably the one that your OS comes with is still broken anyway. But
this code *can* work if properly configured, and those parts can be fixed.
I haven't run this recently, but partially only because I basically forgot
it existed :). Thanks for the reminder.

Is this info already present in the project?

If not I will try to put this info somewhere in the project

[snip]

Post by Glyph Lefkowitz
I hope my suggestion makes sense and is useful. However if you'd really
like to do the compat-breaking dance I won't object; I seriously doubt
anyone is touching the RPC code. I'd prefer you don't delete the whole
module though.

Done https://twistedmatrix.com/trac/ticket/8123

I will go with the deprecation part.

Thanks for the feedback.

[Glyph Lefkowitz]

Post by Adi Roiban
[snip]
Thank you. This would be absolutely great.
https://twistedmatrix.com/trac/ticket/8107 <https://twistedmatrix.com/trac/ticket/8107> is waiting for review... and should make this happen :)

Cool. No promises on the review but I have it in good authority the queue is quite short lately :).

Post by Adi Roiban

Post by Adi Roiban
I would like to clean the inetdtap.py module of pylakes warning but since it is broken and has no tests, I have no idea what this code is expected to do and how end users are expecting to use it.

8123 stream tcp wait glyph /bin/cat -
twistd -n inetd -f sampleinetd.conf -r rpc.conf
and port 8123 properly became an echo server.
Now, in order to do this, you need a /etc/services file (this is hard-coded) that can be parsed by ServicesConf, which is _extremely_ picky, and probably the one that your OS comes with is still broken anyway. But this code can work if properly configured, and those parts can be fixed.
I haven't run this recently, but partially only because I basically forgot it existed :). Thanks for the reminder.
Is this info already present in the project?
If not I will try to put this info somewhere in the project

It's in `twistd --helpÂŽ; I'm not sure where else it belongs.

Post by Adi Roiban
[snip]
I hope my suggestion makes sense and is useful. However if you'd really like to do the compat-breaking dance I won't object; I seriously doubt anyone is touching the RPC code. I'd prefer you don't delete the whole module though.
Done https://twistedmatrix.com/trac/ticket/8123 <https://twistedmatrix.com/trac/ticket/8123>
I will go with the deprecation part.
Thanks for the feedback.
--
Adi Roiban
_______________________________________________
Twisted-Python mailing list
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python