Port this script to python 3

[bitwave]

On January 1st 2020 ends the python2 support. It would be cool, if the script is usable with python3.

[hartwork]

Just had a look if it's working with Python 3 already. At least some fixing seems needed:

# python3 ./servefile -l .
Serving "." at port 8080.

Some addresses this file will be available at:
Traceback (most recent call last):
  File "./servefile", line 1253, in <module>
    main()
  File "./servefile", line 1245, in main
    server.serve()
  File "./servefile", line 987, in serve
    if not ips or len(ips) == 0 or ips[0] == '':
TypeError: object of type 'filter' has no len()

[sebageek]

@hartwork which version did you test? The current master should be fully python3 compatible and also includes some tests for that.

The last release on the other hand only works with python2. I originally planned to also fix the way I use setup.py for the next release, but maybe I'll postpone this for the release after. There is still one PR I need to process for a release (on another git instance) but afterwards there's nothing in the way of a new release. I'll try to do a release in the next days. Thanks for getting my attention onto this again.

Also, if you encounter any bugs when running the current master with python3 please report them to me. :)

[hartwork]

@hartwork which version did you test?

I tested latest Git master. Like this:

# cd "$(mktemp -d)"

# git clone --depth 1 https://github.com/sebageek/servefile.git
Cloning into 'servefile'...
remote: Enumerating objects: 11, done.
remote: Counting objects: 100% (11/11), done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 11 (delta 0), reused 4 (delta 0), pack-reused 0
Unpacking objects: 100% (11/11), done.

# cd servefile/

# python3 ./servefile -l .
Serving "." at port 8080.

Some addresses this file will be available at:
Traceback (most recent call last):
  File "./servefile", line 1253, in <module>
    main()
  File "./servefile", line 1245, in main
    server.serve()
  File "./servefile", line 987, in serve
    if not ips or len(ips) == 0 or ips[0] == '':
TypeError: object of type 'filter' has no len()

# git rev-parse HEAD
e5f9b39025ea757af5eb59f40278de8a4ef696ec

[hartwork]

PS: This is the changes suggested by 2to3-3.5 servefile:

--- servefile   (original)
+++ servefile   (refactored)
@@ -5,7 +5,7 @@
 # Written by Sebastian Lohff ([email protected])
 # http://seba-geek.de/stuff/servefile/
 
-from __future__ import print_function
+
 
 __version__ = '0.4.4'
 
@@ -25,9 +25,9 @@
 
 # fix imports for python2/python3
 try:
-    import BaseHTTPServer
-    import SocketServer
-    from urllib import quote, unquote
+    import http.server
+    import socketserver
+    from urllib.parse import quote, unquote
 except ImportError:
     # both have different names in python3
     import http.server as BaseHTTPServer
@@ -47,7 +47,7 @@
        now = datetime.datetime.fromtimestamp(time.mktime(time.gmtime()))
        return now.strftime("%a, %d %b %Y %H:%M:%S GMT")
 
-class FileBaseHandler(BaseHTTPServer.BaseHTTPRequestHandler):
+class FileBaseHandler(http.server.BaseHTTPRequestHandler):
        fileName = None
        blockSize = 1024 * 1024
        server_version = "servefile/" + __version__
@@ -559,7 +559,7 @@
                return path
 
 
-class FilePutter(BaseHTTPServer.BaseHTTPRequestHandler):
+class FilePutter(http.server.BaseHTTPRequestHandler):
        """ Simple HTTP Server which allows uploading to a specified directory
        either via multipart/form-data or POST/PUT requests containing the file.
        """
@@ -712,9 +712,9 @@
                        return extraDestFileName
                # never reached
 
-class ThreadedHTTPServer(SocketServer.ThreadingMixIn, BaseHTTPServer.HTTPServer):
+class ThreadedHTTPServer(socketserver.ThreadingMixIn, http.server.HTTPServer):
        def handle_error(self, request, client_address):
-               print("%s ABORTED transmission (Reason: %s)" % (client_address[0], sys.exc_value))
+               print("%s ABORTED transmission (Reason: %s)" % (client_address[0], sys.exc_info()[1]))
 
 
 def catchSSLErrors(BaseSSLClass):
@@ -794,7 +794,7 @@
        """ Main class to manage everything. """
 
        _NUM_MODES = 4
-       (MODE_SINGLE, MODE_SINGLETAR, MODE_UPLOAD, MODE_LISTDIR) = range(_NUM_MODES)
+       (MODE_SINGLE, MODE_SINGLETAR, MODE_UPLOAD, MODE_LISTDIR) = list(range(_NUM_MODES))
 
        def __init__(self, target, port=8080, serveMode=0, useSSL=False):
                self.target = target
@@ -808,7 +808,7 @@
                self.listenIPv4 = True
                self.listenIPv6 = True
 
-               if self.serveMode not in range(self._NUM_MODES):
+               if self.serveMode not in list(range(self._NUM_MODES)):
                        self.serveMode = None
                        raise ValueError("Unknown serve mode, needs to be MODE_SINGLE, MODE_SINGLETAR, MODE_UPLOAD or MODE_DIRLIST.")
 
@@ -849,9 +849,9 @@
 
                        # filter out ips we are not listening on
                        if not self.listenIPv6:
-                               ips = filter(lambda ip: ":" not in ip, ips)
+                               ips = [ip for ip in ips if ":" not in ip]
                        if not self.listenIPv4:
-                               ips = filter(lambda ip: "." not in ip, ips)
+                               ips = [ip for ip in ips if "." not in ip]
 
                        return ips
                return None

[hartwork]

I think that last hunk above might be the fix needed here because filter changed semantics from Python 2 to 3:

# python2 -c 'print(filter(str, [1, 2, 3]))'
[1, 2, 3]

# python3.5 -c 'print(filter(str, [1, 2, 3]))'
<filter object at 0x7fd207a585c0>

I ran into this very issue with map elsewhere. It was… not very pleasant 😄

[sebageek]

The imports from your 2to3 output should be fine. The try block has the python2 imports, the except block uses the python3 imports if the python2 imports fail.

It is indeed the case that the problem is that filter returns an iterator, which has no length. A list() or the [] expression will fix that, as you stated. Still, the interesting part is why it happens. Does your system by chance have no IPv6 support? On my system this error only happens when I use -4 or -6. I'll write a test for this and commit the fix in the next days.

[hartwork]

Does your system by chance have no IPv6 support?

Good guess: Indeed this system has IPv6 support disabled, globally.

[sebageek]

Something like f7f24a4 should do it, but will not do a commit to master in my current tired state ;)

[hartwork]

I gave the patch a try: The fix works but the test succeeds even without the fix so it does not seem to test the right thing, yet:

# cd "$(mktemp -d)"
# git clone --depth 2 --branch fix-listen-filter https://github.com/sebageek/servefile.git
# cd servefile
# git checkout HEAD^ -- servefile
# git --no-pager diff --cached
diff --git a/servefile b/servefile
index 1b0553a..b594cf3 100755
--- a/servefile
+++ b/servefile
@@ -849,9 +849,9 @@ class ServeFile():
 
                        # filter out ips we are not listening on
                        if not self.listenIPv6:
-                               ips = [ip for ip in ips if '.' in ip]
+                               ips = filter(lambda ip: ":" not in ip, ips)
                        if not self.listenIPv4:
-                               ips = [ip for ip in ips if ':' in ip]
+                               ips = filter(lambda ip: "." not in ip, ips)
 
                        return ips
                return None

# python3 -m pytest -v tests/test_servefile.py::test_ipv4_only
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.6.9, pytest-4.6.7, py-1.8.0, pluggy-0.13.1 -- /usr/lib/python-exec/python3.6/python3
[..]
tests/test_servefile.py::test_ipv4_only PASSED                                                                                                                                                              [100%]

============================================================================================ 1 passed in 0.64 seconds =============================================================================================

[sebageek]

For my tests I use tox and there the test fails with the filter() function and passes with the [] expression.

$ tox -e py3
GLOB sdist-make: /home/seba/projects/python/servefile/setup.py
py3 recreate: /home/seba/projects/python/servefile/.tox/py3
py3 installdeps: pytest, requests
py3 inst: /home/seba/projects/python/servefile/.tox/dist/servefile-0.4.4.zip
py3 installed: attrs==19.3.0,certifi==2019.11.28,cffi==1.13.2,chardet==3.0.4,cryptography==2.8,idna==2.8,importlib-metadata==1.3.0,more-itertools==8.0.2,packaging==19.2,pkg-resources==0.0.0,pluggy==0.13.1,py==1.8.0,pycparser==2.19,pyOpenSSL==19.1.0,pyparsing==2.4.6,pytest==5.3.2,requests==2.22.0,servefile==0.4.4,six==1.13.0,urllib3==1.25.7,wcwidth==0.1.7,zipp==0.6.0
py3 runtests: PYTHONHASHSEED='492963035'
py3 runtests: commands[0] | pytest --tb=short
========================================================= test session starts ==========================================================
platform linux -- Python 3.7.2, pytest-5.3.2, py-1.8.0, pluggy-0.13.1
rootdir: /home/seba/projects/python/servefile
collected 14 items                                                                                                                     

tests/test_servefile.py ....F.........                                                                                           [100%]

=============================================================== FAILURES ===============================================================
____________________________________________________________ test_ipv4_only ____________________________________________________________
[...]

I haven't tested using pytest directly before, but I could recreate the test not failing in this situation as you describe. With tox I get a fresh virtualenv where servefile is run in and the python used to run servefile is also the one from tox. In your case I presume the tests are run with python3, but servefile itself is run with python2. To prevent this in the future I changed the shebang to #!/usr/bin/env python in 1d8a5bb - now it also works with calling pytest directly . I would recommend using tox anyway, as this is what I use for testing this.

[hartwork]

Interesting!

I think 1d8a5bb helps but only works because on both your and my system /usr/bin/python as Python 3 already — true? Once I switch back my /usr/bin/python to Python 2.7, the test suite calls out to servefile using Python 2 despite 1d8a5bb .

Here's a draft of what I have in mind for a fix:

diff --git a/tests/test_servefile.py b/tests/test_servefile.py
index 5b2c7c7..5feb273 100644
--- a/tests/test_servefile.py
+++ b/tests/test_servefile.py
@@ -3,9 +3,11 @@ import os
 import pytest
 import requests
 import subprocess
+import sys
 import tarfile
 import time
 import urllib3
+from pathlib import Path
 
 # crudly written to learn more about pytest and to have a base for refactoring
 
@@ -18,7 +20,8 @@ def run_servefile():
         if not isinstance(args, list):
             args = [args]
         print("running with args", args)
-        p = subprocess.Popen(['servefile'] + args, **kwargs)
+        servefile = Path(__file__).parent.parent / 'servefile'
+        p = subprocess.Popen([sys.executable, servefile] + args, **kwargs)
         time.sleep(kwargs.get('timeout', 0.3))
         instances.append(p)

With that applied, many tests fail with connection errors — not sure yet why. But running servefile with the same interpreter that called the test suite seems to be the waterproof fix to me, in general. What do you think?

Regarding tox, how about this way forward:

• Making GitHub Actions run tox for every push and pull request
• Adding a minimal README.md documenting that testing with tox is recommended and how to invoke tox the right way as a future servefile contributor
• Creating a setup.py that allows ./setup.py test to run the test suite using py.test with only the current interpreter (because running tox at that place would cause trouble during distro packaging)
• Using long_description_content_type='text/markdown' in setup.py to make it use README.md

What do you think?

[sebageek]

Explicitly calling the interpreter is a nice idea. I'll try to incorporate that the next time I work on this project. And you're also right with the README, even just to describe what this project is actually about.

The setup.py definitely needs some work, but I'm not sure if I'll do all of that before the next release, but I'll look into it. All very good and helpful ideas :)

[sebageek]

Python3 support introduced with v0.5.0