Skip to content
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.

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

Add support for Tornado 6 #10

Closed

Conversation

owais
Copy link

@owais owais commented Mar 18, 2020

This PR adds support for Tornado 6 by conditionally using different
scope manager, context manager and tracing implementation depending
on the version of Tornado and Python being used.

It does not require existing users to change anything other than upgrade
to the latest version of this package.

This package used to use the TornadoScopeManager shipped by
opentracing-python. The scope manager used tornado.stack_context
which was deprecated in Tornado 5 and removed in Tornado 6. Tornado
now recommends using contextvars package introduced in Python3.7.
opentracing-python already provides a ContextVarsScopeManager that
builds on top of the contextvars package. It also implements
AsyncioScopeManager which builds on top of asyncio and falls back
on thread local storage to implement context propagation. We fallback on
this for Python 3.6 and older when using Tornado 6 and newer.

The package also had seen some decay and some tests were not passing.
This PR updates the test suite and unit tests to get them working again.

Changes this PR introduces:

  • Default to ContextVarsScopeManager instead of TornadoScopeManager.
    Fallback on TornadoScopeManager or AsyncioScopeManager based on the
    Tornado and Python version.
  • Added tox support to enable easier testing across Python and Tornado
    versions.
  • Updated travis config to work with tox environments. Now each travis
    build will run tests on every supported python version in parallel. Each
    parallel test will run all tests for all versions of tornado serially.
  • The PR add some code that uses the new async/await syntax. Such code
    is invalid for older versions of python. To make it works for all
    versions, we conditionally import modules depending on the Python
    interpreter version.
  • To preserve backward compatibility and to keep using common code for
    all tornado versions, we've added some noop implementations that are not
    to be used with newer versions of tornado.
  • tornado.gen.coroutine was deprecated in favour of async/await but we
    still support it where we can. There is a bug in Tornado 6 that prevents
    us from support the deprecated feature on Python3.7 with
    ContextVarsScopeManager.
    (Run tornado gen.coroutines in the context of a task. tornadoweb/tornado#2716)
  • Python3.4 also does not pass the tests for tornado.gen.coroutine but
    it is not a regression caused by this PR. Testing on master results in
    the same behavior. For now, I've added skip markers to these tests on
    Python3.4. If needed, we can look into supporting these in future in a
    separate PR.

This PR adds support for Tornado 6 by conditionally using different
scope manager, context manager and tracing implementation depending
on the version of Tornado and Python being used.

It does not require existing users to change anything other than upgrade
to the latest version of this package.

This package used to use the TornadoScopeManager shipped by
opentracing-python. The scope manager used `tornado.stack_context`
which was deprecated in Tornado 5 and removed in Tornado 6. Tornado
now recommends using contextvars package introduced in Python3.7.
opentracing-python already provides a ContextVarsScopeManager that
builds on top of the contextvars package. It also implements
AsyncioScopeManager which builds on top of asyncio and falls back
on thread local storage to implement context propagation. We fallback on
this for Python 3.6 and older when using Tornado 6 and newer.

The package also had seen some decay and some tests were not passing.
This PR updates the test suite and unit tests to get them working again.

Changes this PR introduces:

- Default to ContextVarsScopeManager instead of TornadoScopeManager.
Fallback on TornadoScopeManager or AsyncioScopeManager based on the
Tornado and Python version.
- Added tox support to enable easier testing across Python and Tornado
versions.
- Updated travis config to work with tox environments. Now each travis
build will run tests on every supported python version in parallel. Each
parallel test will run all tests for all versions of tornado serially.
- The PR add some code that uses the new async/await syntax. Such code
is invalid for older versions of python. To make it works for all
versions, we conditionally import modules depending on the Python
interpreter version.
- To preserve backward compatibility and to keep using common code for
all tornado versions, we've added some noop implementations that are not
to be used with newer versions of tornado.
- `tornado.gen.coroutine` was deprecated in favour of async/await but we
still support it where we can. There is a bug in Tornado 6 that prevents
us from support the deprecated feature on Python3.7 with
ContextVarsScopeManager.
(tornadoweb/tornado#2716)
- Python3.4 also does not pass the tests for `tornado.gen.coroutine` but
it is not a regression caused by this PR. Testing on master results in
the same behavior. For now, I've added skip markers to these tests on
Python3.4. If needed, we can look into supporting these in future in a
separate PR.
@owais owais force-pushed the upstream-tornado6-support branch from e3d62ec to 4fa6c10 Compare March 18, 2020 12:49
@owais owais closed this Apr 27, 2020
@owais owais deleted the upstream-tornado6-support branch April 27, 2020 13:18
@alisonatwork
Copy link

Hi @owais, i'm curious why you closed this PR. Is this project not maintained by @carlosalberto any more?

@owais owais restored the upstream-tornado6-support branch May 21, 2020 07:16
@owais owais reopened this May 21, 2020
@owais
Copy link
Author

owais commented May 21, 2020

I don't remember closing this. Probably happened by accident but it does look like this is not being very actively maintained. I guess focus has shifted to OpenTelemetry. This would still be great for OpenTracing users as it won't be feasible for everyone to move to Otel for months still if not years.

@alisonatwork
Copy link

Same thing on our end. We'd love to use OpenTelemetry when it's ready, but we are using OpenTracing in production right now, and getting async/await support and modern Tornado into this project would be great. It would help keep aligned with work like this in main OpenTracing Python lib: opentracing/opentracing-python#118

@alisonatwork
Copy link

@owais I noticed you have some docs for this as well on 8c11210 - could you add that on this PR?

@owais
Copy link
Author

owais commented May 21, 2020

I think I had added those docs for our SignalFx fork of the package. I might not remember correctly but I think those docs didn't apply to this package and would actually go away if this (and a subsequent PR to opentating-python) were merged.

Copy link

@laerte-gaivota laerte-gaivota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need of the PR to mine application.

@owais
Copy link
Author

owais commented Mar 26, 2021

I don't need this anymore so closing. Anyone feel free to adopt the PR or copy/use the code in any way you want.

@owais owais closed this Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants