Hi Victor,
On Tue, Dec 12, 2017 at 6:49 PM, Victor Stinner
<victor.stin...@gmail.com> wrote:
> 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?
Context implements abc.Mapping, so 'get_context().keys()' will give
you a list of all ContextVars in the current context.
>
> 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.
> """
Added.
>
> 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()?
I think the name is fine. While get_context() will return a new instance
every time you call it, those instances will have the same context
variables/values in them, so I don't think it's a problem.
>
>
>> 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) :-)
Fixed.
[..]
>> ``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"?
Fixed.
>
>
>> 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?
Yes, fixed.
>
> 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?
ctx.run(func) runs 'func' in the 'ctx' context. Any changes to
ContextVars that func makes will stay isolated to the 'ctx' context.
>
> Maybe add a short comment to explain that?
Added.
>
> # 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.
If we update decimal to use ContextVars internally, decimal will stay
100% backwards compatible.
Yury
_______________________________________________
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
