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 #1

Merged
merged 1 commit into from Mar 16, 2020
Merged

Add support for Tornado 6 #1

merged 1 commit into from Mar 16, 2020

Conversation

owais
Copy link

@owais owais commented Mar 12, 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.

tests/helpers.py Outdated Show resolved Hide resolved
def wrapper(self, *args, **kwargs):
tracing = self.settings.get('opentracing_tracing')
attrs = self.settings.get('opentracing_traced_attributes', [])
tracing._apply_tracing(self, attrs)

Choose a reason for hiding this comment

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

tracing may be None here

setup.py Outdated Show resolved Hide resolved
@owais owais force-pushed the tornado6 branch 2 times, most recently from 629cc7d to 4724975 Compare March 16, 2020 00:23
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.
Copy link

@rmfitzpatrick rmfitzpatrick left a comment

Choose a reason for hiding this comment

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

Very nice!

@owais owais merged commit cdb8162 into signalfx:master Mar 16, 2020
@owais owais deleted the tornado6 branch March 16, 2020 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants