New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Updates for Python 3.x compatibility #59

Merged

joshmarshall merged 6 commits into joshmarshall:master from prs247au:master

Mar 30, 2021

Contributor

prs247au commented May 14, 2017

Updates so jsonrpclib works on both main releases of Python (which I need to use).
tests.py passes (21 of 30, with 9 skipped) for for Python 3.5.2 (windows) and Python 2.7.9 (linux).
dev-requirements.txt updated to more recent package versions.
Hope this is useful.
Paul

prs247au added 3 commits

May 13, 2017 11:02


          Python 3.x compatibility. Tests pass, need to retest on python 2.7.

23f2345


          Try again


          method name now checked against string and unicode for python 2.7

joshmarshall reviewed

View reviewed changes

Owner

joshmarshall left a comment

Thanks so much for all this! I put some comments (and a few questions in here, let me know what you think. Also, it might be good to add this to the .travis.yml file so that we automatically run all the tests for you across versions:

python:
- "2.7"
- "3.3"
- "3.4"
- "3.5"

Thanks again!

jsonrpclib/SimpleJSONRPCServer.py

@@ @@ -1,19 +1,26 @@ @@
               import jsonrpclib
               from jsonrpclib import Fault
               from jsonrpclib.jsonrpc import USE_UNIX_SOCKETS
-              import SimpleXMLRPCServer
-              import SocketServer
+              try:

Owner

joshmarshall May 17, 2017

You may think about reversing these, since Python 3.x will be the default, and these options will be less likely (over time) to be what the system has installed.

Contributor Author

prs247au May 17, 2017

Sounds good. Do you think the names should be "modernized" throughout the code too? ie. SimpleXMLRPCServer becomes xmlrpc.server, SocketServer becomes socketserver? I originally tried to keep changes as minimal as possible
Btw, I'm quite new to github so apologies in advance if I make any process sorts of mistakes.

jsonrpclib/SimpleJSONRPCServer.py Outdated

+              try:
+                  string_types = (str, unicode)
+              except NameError:
+                  string_types = (str, )

Owner

joshmarshall May 17, 2017

Another option my be to simply do:

try:
    basestring
except NameError:
    basestring = str

...and then use isinstance(method, basestring) since basestring is already defined for you in Python 2.7.

Contributor Author

prs247au May 17, 2017

Nice. Much more elegant.

jsonrpclib/SimpleJSONRPCServer.py

@@ @@ -164,12 +170,14 @@ def do_POST(self): @@
                           L = []
                           while size_remaining:
                               chunk_size = min(size_remaining, max_chunk_size)
-                              L.append(self.rfile.read(chunk_size))
+                              chunk = self.rfile.read(chunk_size).decode()

Owner

joshmarshall May 17, 2017

Hm, this is a tricky one -- I need to look at this in the larger context, but what if this is UTF8, etc?

Contributor Author

prs247au May 17, 2017

Right. I'm not sure if there's any catch to using it. These chunks are always bytes from a socket? If they're encoded and decoded with the same encoding then does it all work out ok? I've assumed utf-8 at both ends for that I guess. The effect in python 2.7 seems to be; str was bytes anyway, so encoded to str, but decodes to unicode. Since unicode is also valid in the response there wasn't a problem there. Or am I missing something else/getting confused?

jsonrpclib/SimpleJSONRPCServer.py Outdated

@@ @@ -180,7 +188,7 @@ def do_POST(self): @@
                       self.send_header("Content-type", "application/json-rpc")
                       self.send_header("Content-length", str(len(response)))
                       self.end_headers()
-                      self.wfile.write(response)
+                      self.wfile.write(response.encode())

Owner

joshmarshall May 17, 2017

Same UTF-8 potential question here.

jsonrpclib/SimpleJSONRPCServer.py Outdated

                               size_remaining -= len(L[-1])
                           data = ''.join(L)
                           response = self.server._marshaled_dispatch(data)
                           self.send_response(200)
-                      except Exception:
+                      except Exception as ex:
+                          print('Exception in do_POST(): %s'%ex)

Owner

joshmarshall May 17, 2017

Looks like an extra print statement?

Contributor Author

prs247au May 17, 2017

Yes, removed now.

jsonrpclib/jsonclass.py Outdated

+                  numeric_types = (int, long, float)
+              except NameError:
+                  string_types = (str, )
+                  numeric_types = (int, float)

Owner

joshmarshall May 17, 2017

Same thought re: basestring, etc. here, and there may be something in the six library to help with iterable types.

jsonrpclib/jsonrpc.py

@@ @@ -151,7 +158,7 @@ def feed(self, data): @@
                       self.data.append(data)
                   def close(self):
-                      return ''.join(self.data)
+                      return ''.join([d.decode() for d in self.data])

Owner

joshmarshall May 17, 2017

UTF8 question.

jsonrpclib/jsonrpc.py

@@ @@ -95,7 +101,8 @@ def jdumps(obj, encoding='utf-8'): @@
                   if cjson:
                       return cjson.encode(obj)
                   else:
-                      return json.dumps(obj, encoding=encoding)
+              #        return json.dumps(obj, encoding=encoding)
+                      return json.dumps(obj)

Owner

joshmarshall May 17, 2017

UTF8, etc. question.

jsonrpclib/jsonrpc.py Outdated

    
            @@ -445,7 +458,7 @@ def __init__(self, rpcid=None, version=None):
          
                      self.version = float(version)

                  def request(self, method, params=[]):

                      if type(method) not in types.StringTypes:

                      if not isinstance(method, str):

Owner

joshmarshall May 17, 2017

Assuming you'd need to do the basestring / str check here too.

Contributor Author

prs247au May 17, 2017

Oops, had a feeling I'd missed one.

prs247au added 2 commits

May 20, 2017 10:56


          Put in some changes suggested by Josh

9edb32c


          Include python 3.x versions

20e55bb

picturedots commented Jun 23, 2017

I'd like to use this for a python3 project, so i am looking forward to a release with this pull request.

Contributor Author

prs247au commented Jun 25, 2017

An update on where things are;

I finally got to testing on linux with Python 3.4.2 and tests were failing. When Unix sockets are used the HTTP class is imported from httplib (python 2.x) but is missing from http.client (python 3.x). HTTP is a compatibility class for Python 1.5 and I couldn't really see why it was needed - HTTPConnection seems to do all that (and more) and is in http.client. I've replaced the reference to HTTP with HTTPConnection but still have to retest on linux/python2.7.

After that another problem came up in linux/python 3.4.2 that is actually in the http.client module. There is some encoding that is missing, see Issue 17214, but this is been fixed in python 3.5. I'm not sure where that leaves us with python <3.5?

One more TODO, I need to check if something about to be encoded to bytes for sending is not a bytes object already. That was causing me problems in my use code. There was already an open question from Josh about the decoding received data from bytes to ?. This is particularly relevant if the object was bytes to begin with at the sending end - why would it be decoded to anything else?

I'll commit and push these changes soon and see what Josh thinks?


          Remove HTTP class and use HTTPConnection instead. Fixes bug for Pytho…

ee90663

…n 3.x on linux.

ehaupt commented Nov 3, 2017 •

edited

Loading

It would be great to have this committed. I am currently waiting for this in order to update electrum to 3.0.

Contributor Author

prs247au commented Nov 13, 2017

Not sure where Josh is at with this. If anyone wants to try my fork https://github.com/prs247au/jsonrpclib and let me know if it suits. Thanks.

SomberNight commented Oct 11, 2018

I am currently waiting for this in order to update electrum to 3.0

Electrum depends on a fork of this repo

LouisPi mentioned this pull request

Python 3 ZeroPhone/ZPUI#136

Open

12 tasks

joshmarshall merged commit 9f1cb47 into joshmarshall:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment