new thrift files

[MaxPresman]

Related to (jaegertracing/jaeger-idl#37), this brings the new thrift files for CI runs.

Signed-off-by: Max Presman [email protected]

[coveralls]

Coverage remained the same at 94.252% when pulling 9794f1b on MaxPresman:upgrade_thrift into 995b4b7 on jaegertracing:master.

[coveralls]

Coverage remained the same at 94.252% when pulling d5eb7e2 on MaxPresman:upgrade_thrift into 995b4b7 on jaegertracing:master.

[yurishkuro]

I am going to block this for now, because it's equivalent to a major version bump of Thrift dependency, which I know will break a lot of Uber services. As was mentioned in #59, we may need to find a way to work with both 0.9 and 0.10.

[MaxPresman]

@yurishkuro I understand. I guess the only way forward is It keep two sets of generated files?

The lack of python3 is blocking adoption on using Jaeger : (.

Is python 3 a priority for the project?

[yurishkuro]

Right now we do not restrict thrift to 0.9.3, the dependency is declared simply as thrift.

To confirm, this works fine on master:

pip install thrift==0.10.0
make test

So I think we don't need to regenerate the source files in order to use 0.10.

The reverse doesn't work, i.e. if I generate files with 0.10.0 then tests don't run with thrift 0.9.3 installed (ImportError: cannot import name TFrozenDict). So there might be some performance implication when using 0.10.0 module with 0.9.3-generated files.

If the mix and match doesn't work, then the fallback would be generating two sets of files and switching dynamically (started in #95). But right now it doesn't look like that's necessary.

Is python 3 a priority for the project?

Different orgs (and maintainers) have different priorities. I would like to get the client compatible with py3, but we currently have many higher priority tasks at Uber, that's why the issue #59 is tagged "help wanted". A bunch of changes have already been done towards supporting py3, including fully supporting it in opentracing_instrumentation module that's a dependency for tests.

If you have the cycles to finish what's remaining in #59, it would be https://youtu.be/8r7OR22sE2g.

[MaxPresman]

@yurishkuro thanks for the responses; i'll close for now.