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

patch_contextvars() raises Exception on <=py3.6 when contextvars backport is installed #1572

Closed
bloodearnest opened this issue Apr 16, 2020 · 1 comment · Fixed by #1576
Closed
Labels
PyVer: python3 Type: Bug

Comments

@bloodearnest
Copy link

@bloodearnest bloodearnest commented Apr 16, 2020

  • gevent version: 1.5.0
  • Python version: Please be as specific as possible: "cPython 3.6.9 from ubuntu 18.04 archives
  • Operating System: Ubuntu 18.04 amd64

Description:

The new gevent.contextvars supports patching stdlib contextvars module, but explicitly only in py3.7+. The logic is in gevent/contextvars.py:48.

__implements__ = __all__ if PY37 else None

However, the decision in gevent.monkey.patch_contextvars about whether or not to patch is based on whether contextvars can be imported.

In the case of running python <=3.6 and using the backported contextvars package from PyPI, this results in patch_contexvars attempting to patch contextvars, as it can be imported, and raising an exception, because gevent.contextvars.__implements__ is None.

  File "/home/travis/build/canonical-ols/talisker/.tox/py36/lib/python3.6/site-packages/gunicorn/arbiter.py", line 583, in spawn_worker
    worker.init_process()
  File "/home/travis/build/canonical-ols/talisker/.tox/py36/lib/python3.6/site-packages/gunicorn/workers/ggevent.py", line 160, in init_process
    self.patch()
  File "/home/travis/build/canonical-ols/talisker/.tox/py36/lib/python3.6/site-packages/gunicorn/workers/ggevent.py", line 53, in patch
    monkey.patch_all()
  File "/home/travis/build/canonical-ols/talisker/.tox/py36/lib/python3.6/site-packages/gevent/monkey.py", line 1152, in patch_all
    patch_contextvars()
  File "/home/travis/build/canonical-ols/talisker/.tox/py36/lib/python3.6/site-packages/gevent/monkey.py", line 183, in ignores
    return func(*args, **kwargs)
  File "/home/travis/build/canonical-ols/talisker/.tox/py36/lib/python3.6/site-packages/gevent/monkey.py", line 546, in patch_contextvars
    _patch_module('contextvars')
  File "/home/travis/build/canonical-ols/talisker/.tox/py36/lib/python3.6/site-packages/gevent/monkey.py", line 408, in _patch_module
    _call_hooks=_call_hooks)
  File "/home/travis/build/canonical-ols/talisker/.tox/py36/lib/python3.6/site-packages/gevent/monkey.py", line 354, in patch_module
    raise AttributeError('%r does not have __implements__' % source_module)
AttributeError: <module 'gevent.contextvars' from '/home/travis/build/canonical-ols/talisker/.tox/py36/lib/python3.6/site-packages/gevent/contextvars.py'> does not have __implements__

Note: the error message is slightly confusing here, as it does have __implements__, it's just explicitly set to None.

If worked around this in my code by setting gevent.contextvars.__implements__ = gevent.contextvars.__all__ manually iff contextvars backport is installed, and this seems to work, though it's not made it into production yet.

From my POV, patching the backport makes sense, but there may be other considerations, of course. Given that patching is protected via an import check, might it possible to just set __implements__ = __all__? i.e. drop the py37 check?

My particular situation it that my library has a hard dependency on contextvars for <=3.6, and optionally supports gevent.

I'd be happy to work on a PR for this, if can provide some guidance as to what direction you'd like the solution to go in.

Thanks

[Edit: formatting]

@jamadden jamadden added PyVer: python3 Type: Bug labels Apr 17, 2020
jamadden added a commit that referenced this issue Apr 17, 2020
@jamadden
Copy link
Member

@jamadden jamadden commented Apr 17, 2020

Thanks for the report! There are tests that prevent us from setting __implements__ to __all__ without doing something more complicated, but its still easy enough to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PyVer: python3 Type: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants