THRIFT-4548: python binary accelerated protocol with multiplexing via decoration

[nijm]

THRIFT-4548: multiplexed (decorated) protocols inherit from TProtocolBase
Client: py

TMultiplexedProtocol objects did not inherit from TProtocolBase, so the decorated protocol object passed into the binary accelerated C extension was not really a TProtocolBase object, which caused errors. This implementation decorates the protocol object by dynamically creating a new class that extends both the class of the protocol being decorated and TMultiplexedProtocol.

I tested the changes locally. I had trouble running the full suite of cross tests, but running the py,py3 cross tests worked, although I'm not sure these are testing multiplexed protocols.

[jeking3]

Very nice - I knew something was wrong when I had to disable that many tests as I added multiplexed support to TestClient, so I'm really glad to see you figured it out and removed the test exclusions!

Test failures: Appveyor: Looks like oracle moved a download link and the chocolatey package hasn't been updated yet. Not relevant to this change.

sca build failure:

# Python code style
flake8 --ignore=E501 --exclude=lib/py/build lib/py
lib/py/src/protocol/TProtocolDecorator.py:23:41: E226 missing whitespace around arithmetic operator
lib/py/src/protocol/TProtocolDecorator.py:27:1: W391 blank line at end of file

Recommend you run the flake8 commands in build/docker/scripts/sca.sh after making python code changes to avoid that in the future. We might even want to teach our regular make to do this if it's fast.

ubsan failure is an intermittent lisp compiler issue where it cannot find a temporary file it just wrote - not relevant here.

Cross test failure is real and related to the changes:

*************************** client message ***************************
Tue Apr 17 17:11:46 2018
Executing: /thrift/src/test/py/TestClient.py --verbose --host=localhost --genpydir=gen-py --protocol=multi --transport=framed --port=46135
Directory: /thrift/src/test/py
config:delay: 5
config:timeout: 10
===============================================================================
EEEEEEEEEEEEEEEEEEEEEE
======================================================================
ERROR: testBinary (__main__.MultiplexedBinaryTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/thrift/src/test/py/TestClient.py", line 148, in testBinary
    self.assertEqual(bytearray(self.client.testBinary(bytes(val))), val)
  File "/thrift/src/test/py/gen-py/ThriftTest/ThriftTest.py", line 574, in testBinary
    self.send_testBinary(thing)
  File "/thrift/src/test/py/gen-py/ThriftTest/ThriftTest.py", line 578, in send_testBinary
    self._oprot.writeMessageBegin('testBinary', TMessageType.CALL, self._seqid)
  File "/thrift/src/lib/py/build/lib.linux-x86_64-2.7/thrift/protocol/TMultiplexedProtocol.py", line 33, in writeMessageBegin
    super().writeMessageBegin(
TypeError: super() takes at least 1 argument (0 given)

[jeking3]

super() takes at least 1 argument

[jeking3]

super() takes at least 1 argument

[nijm]

I've cleared most of the cross-test failures. It looks like the remaining failures are SSL-related timeouts (if I've interpreted the logs correctly). I don't think they're related to this change, but please let me know if they are.

[jeking3]

If you could rebase on master and push, it'll kick a new appveyor build for us. When this was submitted, chocolatey was down or something related to jdk8 was unavailable.

[nijm]

That's done. The travis build still has the same SSL timeouts, but the AppVeyor build succeeded (even though it looks like there was a problem with javadoc).

[jeking3]

I think the following failures are related:

cpp-py                  multic            http-ip                  failure(timeout)
cpp-py                  multic            http-ip-ssl              failure(timeout)
cpp-py                  multic-multiac    http-ip                  failure(timeout)
cpp-py                  multic-multiac    http-ip-ssl              failure(timeout)
cpp-py                  multij            http-ip                  failure(timeout)
cpp-py                  multij            http-ip-ssl              failure(timeout)
cpp-py                  multi             http-ip                  failure(timeout)
cpp-py                  multi             http-ip-ssl              failure(timeout)
cpp-py                  multi-multia      http-ip                  failure(timeout)
cpp-py                  multi-multia      http-ip-ssl              failure(timeout)
cpp-py3                 multic            http-ip                  failure(timeout)
cpp-py3                 multic            http-ip-ssl              failure(timeout)
cpp-py3                 multic-multiac    http-ip                  failure(timeout)
cpp-py3                 multic-multiac    http-ip-ssl              failure(timeout)
cpp-py3                 multij            http-ip                  failure(timeout)
cpp-py3                 multij            http-ip-ssl              failure(timeout)
cpp-py3                 multi             http-ip                  failure(timeout)
cpp-py3                 multi             http-ip-ssl              failure(timeout)
cpp-py3                 multi-multia      http-ip                  failure(timeout)
cpp-py3                 multi-multia      http-ip-ssl              failure(timeout)

You can leave the http based cpp server tests disabled. cpp has known issues with http documented in THRIFT-3877 on Jira.

[cosven]

so the decorated protocol object passed into the binary accelerated C extension was not really a TProtocolBase object, which caused errors.

As far as I know, the C extension does not care what protocol type Python passed into. It just uses the BinaryProtocol or CompatProtocol define in bianry/compat.h to do encoding and decoding.

In my opinion, the real problem is that TXxxProtocol()._fast_encode is BuiltInFunction instead of a Method(at the moment). And the TProtocolDecorator._wrap does not handle this case.

[nijm]

I have disabled the http based cpp server tests - the remaining cross language tests are all passing.