Support Python 3.x #59

yurishkuro · 2017-07-09T21:49:26Z

MrSaints · 2017-07-10T09:45:44Z

Remaining steps (in order of severity):

Upgrade Thrift for Python 3 support (I believe this is not going to be a backwards compatible change)
Fix make test errors [0]
opentracing_instrumentation is not Python 3 compatible, and it is a tests requirement (ModuleNotFoundError: No module named 'urlparse')
Add python 3 to travis

[0] Some test errors / failures that needs resolving:

opentracing.propagation.InvalidCarrierException: carrier not a collection
TypeError: '<=' not supported between instances of 'MagicMock' and 'MagicMock'
TypeError: '>' not supported between instances of 'NoneType' and 'int'
Several others ...

yurishkuro · 2017-07-10T17:59:21Z

As far as Thrift upgrade, we currently require unbound thrift. If possible, I would like to keep it this way, and install different versions in the Travis matrix build. If we need to regenerate the files under jaeger_client/thrift_gen, then we may need to create subdirs for different Thrift versions.

The main reason for this is that many services at Uber pin Thrift to older versions, and pip-compile won't be able to resolve it if we require thrift>=0.10

Re opentracing_instrumentation - it would be good to make it Py3 compatible as well. I don't think it uses Jaeger directly, it should only use basic-tracer which should be Py3 compatible.

dsludwig · 2017-07-25T23:57:28Z

I've got a collection of patches that I used to make jaeger-client 3.4.0 and opentracing_instrumentation 2.2.0 run on Python 3, attached here. A number of these were covered by #57, but there are a few that are not. I was only trying to support Python 3 rather than both, so they're not applicable as-is, but hopefully this helps!

Edit by @yurishkuro - I moved the diffs to https://pastebin.com/zuLge71t

yurishkuro · 2017-07-26T00:04:14Z

@dsludwig interesting - so you did not have to upgrade thrift-generated classes, just fix them up a bit? Or did you still have to use higher version of thrift?

dsludwig · 2017-07-26T00:39:47Z

I still needed a higher version of thrift at runtime, but I did not end up regenerating the thrift classes.

yurishkuro · 2017-07-27T03:43:11Z

I still needed a higher version of thrift at runtime, but I did not end up regenerating the thrift classes.

That's good to know. I started applying your patch to opentracing_instrumentation but got too busy with other things. Want to get that out of the way first.

yurishkuro · 2017-07-31T02:54:41Z

@dsludwig would you be able to test this? uber-common/opentracing-python-instrumentation#28

I didn't port all of your changes, e.g. this one

         @property
         def host_port(self):
-            host_string = self.request.get_host()
+            host_string = self.request.host
             return split_host_and_port(host_string=host_string,
-                                       scheme=self.request.get_type())
+                                       scheme=self.request.type)

olevold · 2017-10-27T13:54:30Z

Does anyone know if there will be a new release in the near future?

yurishkuro · 2017-10-27T14:42:01Z

@olevold 3.6.1 was released on Sep 26, there have been no changes since then.

nehaljwani · 2017-11-13T20:12:49Z

The get_host() and get_type() change was made to opentracing-python-instrumentation in uber-common/opentracing-python-instrumentation@5ade4a5#diff-ef3e0ebde91ef405f72ce139bc8cd5ef

dudymas · 2017-11-21T19:38:52Z

I made #99 to hopefully move this along

kbroughton · 2018-01-08T16:43:15Z

What help is wanted for this issue?

My environment is
GCE kubernetes engine or docker-compose
pip install django_opentracing jaeger-client
jaeger.version=Python-3.7.1
django_opentracing/_version.py
{
"full-revisionid": "21b48cd0998ffdc3f1284341beb2a9606fd8f30e",
"version": "0.1.18"
}

I was just able to get my django python3 app running with the latest pip installs after fixing one iteritems error:
opentracing-contrib/python-django#16

yurishkuro · 2018-01-08T16:56:47Z

@kbroughton there have been a few PRs merged that fix various incompatibilities with py3, but it's not finished. Some PRs are not merged because they fail the tests (e.g. #101).

The preference is to make a set of smaller PRs that fix specific syntax issues one at a time.

yurishkuro · 2018-01-14T19:28:41Z

@kbroughton I am surprised #113 addresses this issue. I just tried enabling 3.6 in #114, and it fails with a bunch of errors, like not recognizing xrange keyword, etc. All these changes have been attempted in different other PRs, but not signed off / completed. Ideally we just need to do a series of small PRs that address one feature at a time, like xrange->range.

ror6ax · 2018-02-14T09:45:44Z

Same question, what's left to be done here? Which help is needed?

yurishkuro · 2018-02-14T16:42:42Z

There are still some tests failing, most of them around unicode handling. We had an outage internally caused by unicode in python, it's currently being investigated, I am hoping it will finally reveal what was causing httplib to blow up, then we can have a proper fix and it should be easy to port to py3.

sc68cal · 2018-03-06T19:50:59Z

I also see that we have some logic that skips installing tchannel when running in a py3 environment, yet the tests still are run.

______________________________ ERROR collecting tests/test_crossdock.py _______________________________
tests/test_crossdock.py:22: in <module>
    from crossdock.server import server
crossdock/server/server.py:33: in <module>
    from tchannel import TChannel, thrift
E   ModuleNotFoundError: No module named 'tchannel'

It looks like tchannel is not ready for py3, since if you install it, you end up with another error.

______________________________ ERROR collecting tests/test_crossdock.py _______________________________
lib/python3.6/site-packages/_pytest/python.py:611: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
lib/python3.6/site-packages/py/_path/local.py:668: in pyimport
    __import__(modname)
<frozen importlib._bootstrap>:971: in _find_and_load
    ???
<frozen importlib._bootstrap>:955: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:656: in _load_unlocked
    ???
<frozen importlib._bootstrap>:626: in _load_backward_compatible
    ???
lib/python3.6/site-packages/_pytest/assertion/rewrite.py:171: in load_module
    py.builtin.exec_(co, mod.__dict__)
tests/test_crossdock.py:22: in <module>
    from crossdock.server import server
crossdock/server/server.py:33: in <module>
    from tchannel import TChannel, thrift
lib/python3.6/site-packages/tchannel/__init__.py:30: in <module>
    from .response import Response  # noqa
lib/python3.6/site-packages/tchannel/response.py:25: in <module>
    from . import schemes
lib/python3.6/site-packages/tchannel/schemes/__init__.py:37: in <module>
    from .json import JsonArgScheme  # noqa
lib/python3.6/site-packages/tchannel/schemes/json.py:32: in <module>
    from ..tracing import ClientTracer, TChannelOpenTracingClientInterceptor
E     File "/Users/scollins/src/jaeger-client-python/lib/python3.6/site-packages/tchannel/tracing.py", line 280
E       parent_id=carrier['parent_id'] or 0L,
E                                          ^
E   SyntaxError: invalid syntax

Even if you fix that one, it gets harder from there.

During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/Users/scollins/src/tchannel-python/.tox/python3/lib/python3.6/site-packages/_pytest/config.py", line 320, in _importconftest
    mod = conftestpath.pyimport()
  File "/Users/scollins/src/tchannel-python/.tox/python3/lib/python3.6/site-packages/py/_path/local.py", line 668, in pyimport
    __import__(modname)
  File "/Users/scollins/src/tchannel-python/tests/conftest.py", line 26, in <module>
    from .mock_server import MockServer
  File "/Users/scollins/src/tchannel-python/tests/mock_server.py", line 28, in <module>
    from tchannel import TChannel
  File "/Users/scollins/src/tchannel-python/tchannel/__init__.py", line 30, in <module>
    from .response import Response  # noqa
  File "/Users/scollins/src/tchannel-python/tchannel/response.py", line 25, in <module>
    from . import schemes
  File "/Users/scollins/src/tchannel-python/tchannel/schemes/__init__.py", line 37, in <module>
    from .json import JsonArgScheme  # noqa
  File "/Users/scollins/src/tchannel-python/tchannel/schemes/json.py", line 32, in <module>
    from ..tracing import ClientTracer, TChannelOpenTracingClientInterceptor
  File "/Users/scollins/src/tchannel-python/tchannel/tracing.py", line 32, in <module>
    from tchannel.messages import common, Tracing
  File "/Users/scollins/src/tchannel-python/tchannel/messages/__init__.py", line 23, in <module>
    from .call_request import CallRequestMessage, call_req_rw
  File "/Users/scollins/src/tchannel-python/tchannel/messages/call_request.py", line 23, in <module>
    from . import common
  File "/Users/scollins/src/tchannel-python/tchannel/messages/common.py", line 29, in <module>
    from .. import rw
  File "/Users/scollins/src/tchannel-python/tchannel/rw.py", line 25, in <module>
    from .errors import ReadError
  File "/Users/scollins/src/tchannel-python/tchannel/errors.py", line 51, in <module>
    class TChannelError(Exception):
ValueError: 'code' in __slots__ conflicts with class variable

Traceback (most recent call last):
  File "/Users/scollins/src/tchannel-python/.tox/python3/lib/python3.6/site-packages/_pytest/config.py", line 283, in _getconftestmodules
    return self._path2confmods[path]
KeyError: local('/Users/scollins/src/tchannel-python/tests')

During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/Users/scollins/src/tchannel-python/.tox/python3/lib/python3.6/site-packages/_pytest/config.py", line 314, in _importconftest
    return self._conftestpath2mod[conftestpath]
KeyError: local('/Users/scollins/src/tchannel-python/tests/conftest.py')

During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/Users/scollins/src/tchannel-python/.tox/python3/lib/python3.6/site-packages/_pytest/config.py", line 320, in _importconftest
    mod = conftestpath.pyimport()
  File "/Users/scollins/src/tchannel-python/.tox/python3/lib/python3.6/site-packages/py/_path/local.py", line 668, in pyimport
    __import__(modname)
  File "/Users/scollins/src/tchannel-python/tests/conftest.py", line 26, in <module>
    from .mock_server import MockServer
  File "/Users/scollins/src/tchannel-python/tests/mock_server.py", line 28, in <module>
    from tchannel import TChannel
  File "/Users/scollins/src/tchannel-python/tchannel/__init__.py", line 32, in <module>
    from .tchannel import TChannel  # noqa
  File "/Users/scollins/src/tchannel-python/tchannel/tchannel.py", line 38, in <module>
    from .health import health
  File "/Users/scollins/src/tchannel-python/tchannel/health/__init__.py", line 23, in <module>
    from .health import health   # noqa
  File "/Users/scollins/src/tchannel-python/tchannel/health/health.py", line 26, in <module>
    from .. import thrift
  File "/Users/scollins/src/tchannel-python/tchannel/thrift/__init__.py", line 25, in <module>
    from .server import register  # noqa
  File "/Users/scollins/src/tchannel-python/tchannel/thrift/server.py", line 248
    raise exc_info[0], exc_info[1], exc_info[2]

yurishkuro · 2018-03-06T20:41:44Z

@sc68cal yeah. I don't think there's much chance tchannel would be ever support py3, it's in maintenance mode. We have two options for Py3:

skip running the crossdock tests (unit and Docker-based), or
implement some sort of wrapper for tchannel imports so that the code still compiles, and change unit + Docker tests to skip the tchannel-related tests, but keep http-related ones (so that they still run on py2)

yurishkuro · 2018-04-16T04:50:16Z

Should be fixed by #154

nlamirault · 2018-04-17T13:11:11Z

Congrats ! @yurishkuro do you know when a new release with Python 3 support will be done ?

yurishkuro · 2018-04-18T01:52:45Z

Planning to release it sometime this week.

ror6ax · 2018-05-07T17:08:09Z

@yurishkuro - there's been a release but it does not say whether this has been merged into it or not. Could you clarify? I'm standing by to release flask tooling with py3 support.

yurishkuro · 2018-05-07T17:13:34Z

I was mentioned in the changelog, I just replicated as Github release https://github.com/jaegertracing/jaeger-client-python/releases/tag/v3.9.0

yurishkuro mentioned this issue Aug 11, 2017

SyntaxError in Python 3 #41

Closed

yurishkuro added the help wanted label Aug 11, 2017

This was referenced Nov 7, 2017

new thrift files #94

Closed

Test py36 #96

Closed

yurishkuro mentioned this issue Nov 22, 2017

fix(python3): add support. skindeep changes for #59 #99

Merged

This was referenced Jan 14, 2018

Fix xrange for Py3 in thrift generated files #115

Merged

Fix dependencies for Py3 compatibility #116

Merged

Fix span and sampler tests to work under Py3 #117

Merged

ror6ax mentioned this issue Feb 19, 2018

opentracing setup version opentracing-contrib/python-flask#9

Closed

worrel mentioned this issue Mar 8, 2018

3.8.0 doesn't report spans in Python 3 #137

Closed

yurishkuro mentioned this issue Apr 16, 2018

Remove support for non-ascii baggage keys; enable testing with Py 3.6 #154

Merged

yurishkuro closed this as completed in #154 Apr 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Python 3.x #59

Support Python 3.x #59

yurishkuro commented Jul 9, 2017

MrSaints commented Jul 10, 2017 •

edited by yurishkuro

Loading

yurishkuro commented Jul 10, 2017

dsludwig commented Jul 25, 2017 •

edited by yurishkuro

Loading

yurishkuro commented Jul 26, 2017

dsludwig commented Jul 26, 2017

yurishkuro commented Jul 27, 2017

yurishkuro commented Jul 31, 2017

olevold commented Oct 27, 2017

yurishkuro commented Oct 27, 2017

nehaljwani commented Nov 13, 2017

dudymas commented Nov 21, 2017

kbroughton commented Jan 8, 2018

yurishkuro commented Jan 8, 2018

yurishkuro commented Jan 14, 2018

ror6ax commented Feb 14, 2018

yurishkuro commented Feb 14, 2018

sc68cal commented Mar 6, 2018 •

edited

Loading

yurishkuro commented Mar 6, 2018

yurishkuro commented Apr 16, 2018

nlamirault commented Apr 17, 2018

yurishkuro commented Apr 18, 2018

ror6ax commented May 7, 2018

yurishkuro commented May 7, 2018

Support Python 3.x #59

Support Python 3.x #59

Comments

yurishkuro commented Jul 9, 2017

MrSaints commented Jul 10, 2017 • edited by yurishkuro Loading

yurishkuro commented Jul 10, 2017

dsludwig commented Jul 25, 2017 • edited by yurishkuro Loading

yurishkuro commented Jul 26, 2017

dsludwig commented Jul 26, 2017

yurishkuro commented Jul 27, 2017

yurishkuro commented Jul 31, 2017

olevold commented Oct 27, 2017

yurishkuro commented Oct 27, 2017

nehaljwani commented Nov 13, 2017

dudymas commented Nov 21, 2017

kbroughton commented Jan 8, 2018

yurishkuro commented Jan 8, 2018

yurishkuro commented Jan 14, 2018

ror6ax commented Feb 14, 2018

yurishkuro commented Feb 14, 2018

sc68cal commented Mar 6, 2018 • edited Loading

yurishkuro commented Mar 6, 2018

yurishkuro commented Apr 16, 2018

nlamirault commented Apr 17, 2018

yurishkuro commented Apr 18, 2018

ror6ax commented May 7, 2018

yurishkuro commented May 7, 2018

MrSaints commented Jul 10, 2017 •

edited by yurishkuro

Loading

dsludwig commented Jul 25, 2017 •

edited by yurishkuro

Loading

sc68cal commented Mar 6, 2018 •

edited

Loading