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

Support for PEP 567 #420

Closed
Fuyukai opened this issue Jan 24, 2018 · 4 comments · Fixed by #478
Closed

Support for PEP 567 #420

Fuyukai opened this issue Jan 24, 2018 · 4 comments · Fixed by #478

Comments

@Fuyukai
Copy link
Member

Fuyukai commented Jan 24, 2018

PEP 567 got accepted on the 22nd, adding context-local variables to Python 3.7, and it would be super useful to have implemented (especially for something I'm working on).

However, 3.5/3.6 compat is an issue, and if it wasn't for that I would've PR'd in already (since the change is probably super trivial, msg = task.coro.send(next_send) -> context = contextvars.copy_context(); msg = context.run(task.coro.send, next_send) or similar.), and I don't know how that would be solved.

@njsmith
Copy link
Member

njsmith commented Jan 24, 2018

Yeah, ContextVar is super cool!

What I'm thinking we should do is deprecate trio.TaskLocal, and instead have a trio.ContextVar class. On 3.7 this will just be from contextvars import ContextVar, and on earlier versions it'll be our own class that's designed to have the same semantics, but implemented using the same techniques that we currently use for TaskLocal.

Unfortunately, we don't currently have any way to test on a recent 3.7 snapshot (travis-ci/travis-ci#9119), so probably that will need to be resolved first. In particular, we'll need to write tests for our workalike implementation, and I want to run those tests against the real thing too to make sure we don't accidentally mess up the semantics!

(BTW, just in case you haven't seen TaskLocal, you might want to check it out, since it does work now.)

@njsmith
Copy link
Member

njsmith commented Jan 24, 2018

Oh, and we'll want to be calling copy_context from _spawn_impl and attaching the context to the task object, and then doing task.context.run(task.coro.send, next_send).

@njsmith
Copy link
Member

njsmith commented Jan 24, 2018

It looks like Travis's 3.7-dev and nightly Pythons are now updated to have PEP 567 support, so that's no longer a blocker.

It did occur to me that for supporting older Pythons, then instead of having trio.ContextVar, another option would be to have a backport library (pip install pep567 or pip install contextvars or whatever), and then use it in Trio. In some sense this would be equivalent: there'd be a library on PyPI that contains code to emulate the contextvars API. But by splitting that off into a separate library from trio, it would potentially be usable by other projects too.

Personally I'd be happy with a version of this library that was in pure Python and didn't bother monkeypatching asyncio, but I think @1st1 has been thinking about a more ambitious backport project, so CC'ing him to get his thoughts.

@smurfix
Copy link
Contributor

smurfix commented Feb 15, 2018

pypi doesn't have pep567, and contextvars is incomatible with PEP 567.
So the first step would be to implement the "real" contextvars in pure Python.
We don't need to monkeypatch asyncio, since trio-asyncio will supersede the affected functions anyway.

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 a pull request may close this issue.

3 participants