Overall, I like this PEP. It's definitely easier to follow
conceptually than PEP 550. Thanks for taking the time to re-think the
idea. I have a few comments in-line below.
-eric
On Tue, Dec 12, 2017 at 10:33 AM, Yury Selivanov
<yselivanov...@gmail.com> wrote:
> This is a new proposal to implement context storage in Python.
+1
This is something I've had on my back burner for years. Getting this
right is non-trivial, so having a stdlib implementation will help open
up clean solutions in a number of use cases that are currently
addressed in more error-prone ways.
>
> It's a successor of PEP 550 and builds on some of its API ideas and
> datastructures. Contrary to PEP 550 though, this proposal only focuses
> on adding new APIs and implementing support for it in asyncio. There
> are no changes to the interpreter or to the behaviour of generator or
> coroutine objects.
Do you have any plans to revisit extension of the concept to
generators and coroutine objects? I agree they can be addressed
separately, if necessary. TBH, I'd expect this PEP to provide an
approach that allows such applications of the concept to effectively
be implementation details that can be supported later.
> Abstract
> ========
>
> This PEP proposes the new ``contextvars`` module and a set of new
> CPython C APIs to support context variables. This concept is
> similar to thread-local variables but, unlike TLS, it allows
s/it allows/it also allows/
> correctly keeping track of values per asynchronous task, e.g.
> ``asyncio.Task``.
>
> [snip]
>
> Rationale
> =========
>
> Thread-local variables are insufficient for asynchronous tasks which
> execute concurrently in the same OS thread. Any context manager that
> needs to save and restore a context value and uses
> ``threading.local()``, will have its context values bleed to other
> code unexpectedly when used in async/await code.
FWIW, I'd consider the concept to extend to all execution contexts in
the interpreter, of which threads and async/await are the only kinds
we have currently. That said, I don't see us adding any new kinds of
execution context so what you've said is entirely satisfactory. :)
>
> [snip]
>
> Introduction
> ============
>
> [snip]
>
> Specification
> =============
>
> A new standard library module ``contextvars`` is added
Why not add this to contextlib instead of adding a new module? IIRC
this was discussed relative to PEP 550, but I don't remember the
reason. Regardless, it would be worth mentioning somewhere in the
PEP.
> with the
> following APIs:
>
> 1. ``get_context() -> Context`` function is used to get the current
> ``Context`` object for the current OS thread.
>
> 2. ``ContextVar`` class to declare and access context variables.
It may be worth explaining somewhere in the PEP the reason why you've
chosen to add ContextVar instead of adding a new keyword (e.g.
"context", a la global and nonlocal) to do roughly the same thing.
Consider that execution contexts are very much a language-level
concept, a close sibling to scope. Driving that via a keyword would a
reasonable approach, particularly since it introduces less coupling
between a language-level feature and a stdlib module. (Making it a
builtin would sort of help with that too, but a keyword would seem
like a better fit.) A keyword would obviate the need for explicitly
calling .get() and .set().
FWIW, I agree with not adding a new keyword. To me context variables
are a low-level tool for library authors to implement their high-level
APIs. ContextVar, with its explicit .get() and .set() methods is a
good fit for that and better communicates the conceptual intent of the
feature. However, it would still be worth explicitly mentioning the
alternate keyword-based approach in the PEP.
>
> 3. ``Context`` class encapsulates context state. Every OS thread
> stores a reference to its current ``Context`` instance.
> It is not possible to control that reference manually.
> Instead, the ``Context.run(callable, *args)`` method is used to run
> Python code in another context.
I'd call that "Context.call()" since its for callables. Did you have
a specific reason for calling it "run" instead?
>
>
FWIW, I think there are some helpers you could add that library
authors would appreciate. However, they aren't critical so I'll hold
off and maybe post about them later. :)
> contextvars.ContextVar
> ----------------------
>
> The ``ContextVar`` class has the following constructor signature:
> ``ContextVar(name, *, default=no_default)``. The ``name`` parameter
> is used only for introspection and debug purposes.
It doesn't need to be required then, right?
> [snip]
>
> ``ContextVar.set(value) -> Token`` is used to set a new value for
> the context variable in the current ``Context``::
>
> # Set the variable 'var' to 1 in the current context.
> var.set(1)
>
> ``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)
>
> 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.
The "restoring values" focus is valuable on its own, It emphasizes a
specific usage pattern to users (though a context manager would
achieve the same). The token + reset() approach means that users
don't need to think about "not set" when restoring values. That said,
is there otherwise any value to the "not set" concept? If so,
"is_set()" (not strictly necessary) and "unset()" methods may be
warranted.
Also, there's a strong context manager vibe here. Some sort of
context manager support would be nice. However, with the token coming
out of .set() and with no alternative (e.g. "get_token()"), I'm not
sure what an intuitive CM interface would be here.
> [snip]
>
> contextvars.Context
> -------------------
>
> [snip]
>
> 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')
Shouldn't this be "assert var.get() == 'spam'"?
>
> Any changes to the context will be contained and persisted in the
> ``Context`` object on which ``run()`` is called on.
For me this would be more clear if it could be spelled like this:
with ctx:
function()
Also, let's say I want to run a function under a custom context,
whether a fresh one or an adaptation of an existing one. How can I
compose such a Context? AFAICS, the only way to modify a context is
by using ContextVar.set() (and reset()), which modifies the current
context. It might be useful if there were a more direct way, like a
"Context.add(*var) -> Context" and "Context.remove(*var) -> Context"
and maybe even a "Context.set(var, value) -> Context" and
"Context.unset(var) -> Context".
_______________________________________________
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
