[Twisted-Python] [Twisted] #7598: Port twisted.spread.pb to Python3

[Wolfgang Rohdewald]

The branch there ports PB to Python 3. There might still be a few coverage
issues, and since I trust codecov, I'm not entiirely sure. But this is
definitely worth at least a first pass review at this point.

Great news!

I should be able to do some testing next week

--
Wolfgang

[Wolfgang Rohdewald]

#7598: Port twisted.spread.pb to Python3

did you test with client on Python2 and server on Python3 and vice versa?

Not a bug report, but anyway ... This was client with Python3,
server with Python2

The same works with my old port to Python3:
git clone https://github.com/wrohdewald/twisted.git
git checkout spread-py3-7598

My version uses helpers in remoteMessageReceived:

kw = nativeStringDict(broker.unserialize(kw))
method = getattr(self, "remote_%s" % nativeString(message), None)


If you need a minimal example and an official bug report -
that would take some more time. Maybe in a week or so.

Peer will receive following PB traceback:
Unhandled Error
Traceback (most recent call last):
File "/home/wr/src/kajongg/src/twisted/spread/banana.py", line 173, in
gotItem
self.callExpressionReceived(item)
File "/home/wr/src/kajongg/src/twisted/spread/banana.py", line 136, in
callExpressionReceived
self.expressionReceived(obj)
File "/home/wr/src/kajongg/src/twisted/spread/pb.py", line 575, in
expressionReceived
method(*sexp[1:])
File "/home/wr/src/kajongg/src/twisted/spread/pb.py", line 896, in
proto_message
self._recvMessage(self.localObjectForID, requestID, objectID, message,
answerRequired, netArgs, netKw)
--- <exception caught here> ---
File "/home/wr/src/kajongg/src/twisted/spread/pb.py", line 913, in
_recvMessage
netResult = object.remoteMessageReceived(self, message, netArgs, netKw)
File "/home/wr/src/kajongg/src/twisted/spread/flavors.py", line 120, in
remoteMessageReceived
state = method(*args, **kw)
builtins.TypeError: remote_move() keywords must be strings


--
Wolfgang

[Glyph Lefkowitz]

Post by Wolfgang Rohdewald

#7598: Port twisted.spread.pb to Python3

did you test with client on Python2 and server on Python3 and vice versa?
Not a bug report, but anyway ... This was client with Python3,
server with Python2

This is a perfectly good bug report :) and it's a case that should definitely be considered. Probably we should up-convert from bytes automatically in the places where we know python will be using the values as identifiers, including callRemote and the keys in a __dict__.

This is unfortunately very tricky to know though, if we support serializing both bytes and strings :-\.

-glyph

[Wolfgang Rohdewald]

Post by Glyph Lefkowitz
Probably we should up-convert from bytes automatically in the places
where we know python will be using the values as identifiers, including
callRemote and the keys in a __dict__.

Attached is some grep for my port showing where I had co convert.

grep -ri -e '\(network\|native\)'

Post by Glyph Lefkowitz
This is unfortunately very tricky to know though, if we support
serializing both bytes and strings :-\.

Why? The wire protocol does not change. PY3 bytes are already
compatible, and PY3 strings are serialized as unicode objects.

--
Wolfgang

[Glyph Lefkowitz]

Post by Wolfgang Rohdewald

Post by Glyph Lefkowitz
Probably we should up-convert from bytes automatically in the places
where we know python will be using the values as identifiers, including
callRemote and the keys in a __dict__.

Attached is some grep for my port showing where I had co convert.
grep -ri -e '\(network\|native\)'

Post by Glyph Lefkowitz
This is unfortunately very tricky to know though, if we support
serializing both bytes and strings :-\.

Why? The wire protocol does not change. PY3 bytes are already
compatible, and PY3 strings are serialized as unicode objects.

The protocol does change though. ['foo'] in jelly means "bytes". ['unicode', 'foo'] means "text". You can fix some problems with this; obviously if you see a ['dictionary', [['unicode', 'foo'], ['unicode', 'bar']]] come in over the wire, and then get sent to setCopyableState, we can translate 'foo' back to a native 'str' in the python 2 implementation. But what to do with 'bar'? It will remain text. If you extend that, and consider that your application code might have a value floating around in a variable which later gets used for a key in **kwargs or something, there's no way we can know as we're deserializing that you're going to do that, so your application is going to need to account for it manually.

Ultimately that's not a bug we can fix, so we won't. But it's still unfortunate.

-glyph

[Wolfgang Rohdewald]

Post by Wolfgang Rohdewald

#7598: Port twisted.spread.pb to Python3

did you test with client on Python2 and server on Python3 and vice versa?
Not a bug report, but anyway ... This was client with Python3,
server with Python2

I have now restricted testing to using the same version of Python
on both sides.

That works nicely with my application.

Hope this is released soon!

--
Wolfgang

[Glyph Lefkowitz]

Post by Wolfgang Rohdewald

Post by Wolfgang Rohdewald

#7598: Port twisted.spread.pb to Python3

did you test with client on Python2 and server on Python3 and vice versa?
Not a bug report, but anyway ... This was client with Python3,
server with Python2

I have now restricted testing to using the same version of Python
on both sides.
That works nicely with my application.

Thank you for testing!

Post by Wolfgang Rohdewald
Hope this is released soon!

It didn't make it for 16.4, but I'd anticipate it will likely land in 16.5. I just need to figure out what's going on with the python 3.3 test failures.

-glyph