Hi Yury,
I like the overall idea and I prefer this PEP over PEP 550 since it's
shorter and easier to read :-)
Question: Is there an API to list all context variables?
Would it be possible to have a very summary of the changes in the PEP?
I propose:
"""
* Added contextvars module with ContextVar, Context and Token classes,
and a get_context() function
* asyncio: Added keyword-only context parameter to call_at(),
call_later(), call_soon() methods of event loops and
Future.add_done_callback(); Task are modified internally to maintain
their own isolated context.
"""
Each get_context() call returns a new Context object. It may be worth
to mention it. I understand why, but it's surprising that "assert
get_context() is not get_context()" fails. Maybe it's a naming issue?
Maybe rename it to contextvars.context()?
> Abstract: ... This concept is similar to thread-local variables but, unlike
> TLS, ...
nitpick: please write "Thread Local Storage (TLS)". When I read TLS, I
understand HTTPS (Transport Layer Security) :-)
Your PEP seems to be written for asyncio. Maybe it would help to
understand it to make it more explicit in the abstract... even if I
understand perfectly that it's not strictly specific to asyncio ;-)
> # Declare a context variable 'var' with the default value 42.
> var = ContextVar('var', default=42)
nitpick: I suggest to use 'name' rather than 'var', to make it obvious
that the first parameter is the variable name.
> ``contextvars.Token`` is an opaque object that should be used to
> restore the ``ContextVar`` to its previous value, or remove it from
> the context if it was not set before. The ``ContextVar.reset(Token)``
> is used for that::
>
> old = var.set(1)
> try:
> ...
> finally:
> var.reset(old)
I don't see where is the token in this example. Does set() return a
token object? Yes according to ContextVar pseudo-code below.
When I read "old", I understand that set() returns the old value, not
an opaque token. Maybe rename "old" to "token"?
> The ``Token`` API exists to make the current proposal forward
> compatible with :pep:`550`, in case there is demand to support
> context variables in generators and asynchronous generators in the
> future.
Cool. I like the idea of starting with something simple in Python 3.7.
Then extend it in Python 3.8 or later (support generators), if it
becomes popular, once the first simple (but "incomplete", without
generators) implementation is battle-tested.
> Any changes to any context variables that ``function`` causes, will
> be contained in the ``ctx`` context::
>
> var = ContextVar('var')
> var.set('spam')
>
> def function():
> assert var.get() == 'spam'
>
> var.set('ham')
> assert var.get() == 'ham'
>
> ctx = get_context()
> ctx.run(function)
>
> assert var.get('spam')
Should I read assert var.get() == 'spam' here?
At the first read, I understood that that ctx.run() creates a new
temporary context which is removed once ctx.run() returns.
Now I understand that context variable values are restored to their
previous values once run() completes. Am I right?
Maybe add a short comment to explain that?
# Call function() in the context ctx
# and then restores context variables of ctx to their previous values
ctx.run(function)
> Backwards Compatibility
> =======================
>
> This proposal preserves 100% backwards compatibility.
Ok.
> Libraries that use ``threading.local()`` to store context-related
> values, currently work correctly only for synchronous code. Switching
> them to use the proposed API will keep their behavior for synchronous
> code unmodified, but will automatically enable support for
> asynchronous code.
I'm confused by this sentence. I suggest to remove it :-)
Converting code to contextvars makes it immediately backward
incompatible, no I'm not sure that it's a good it to suggest it in
this section.
Victor
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
