Hi,

Thanks to everybody participating in the PEP 567 discussion!  I want
to summarize a few topics to make sure that we are all on the same
page (and maybe provoke more discussion).


1. Proposal: ContextVar has default set to None.

>From the typing point of view that would mean that if a context
variable is declared without an explicit default, its type would be
Optional.  E.g. say we have a hypothetical web framework that allows
to access the current request object through a context variable:

  request_var: ContextVar[Optional[Request]] = \
      ContextVar('current_request')

When we need to get the current request object, we would write:

  request: Optional[Request] = request_var.get()

And we'd also need to explicitly handle when 'request' is set to None.
Of course we could create request_var with its default set to some
"InvalidRequest" object, but that would complicate things.  It would
be easier to just state that the framework always sets the current
request and it's a bug if it's not set.

Therefore, in my opinion, it's better to keep the current behaviour:
if a context variable was created without a default value,
ContextVar.get() can raise a LookupError.


2. Context.__contains__, Context.__getitem__ and ContexVar.default

So if we keep the current PEP 567 behaviour w.r.t. defaults,
ContextVar.get() might return a different value from Context.get():

    v = ContextVar('v', default=42)
    ctx = contextvars.copy_context()

    ctx.get(v)   # returns None
    v.get()   # returns 42
    v in ctx  # returns False

I think this discrepancy is OK.  Context is a mapping-like object and
it reflects the contents of the underlying _ContextData mapping
object.

ContextVar.default is meant to be used only by ContextVar.get().
Context objects should not use it.

Maybe we can rename ContextVar.get() to ContextVar.lookup()?  This
would help to avoid potential confusion between Context.get() and
ContextVar.get().


3. Proposal: Context.get() and __getitem__() should always return up
to date values.

The issue with the current PEP 567 design is that PyThreadState points
to a _ContextData object, and not to the current Context.  The
following code illustrates how this manifests in Python code:

    v = ContextVar('v')

    def foo():
        v.set(42)
        print(v.get(), ctx.get(v, 'missing'))

    ctx = Context()
    ctx.run(foo)

The above code will print "42 missing", because 'ctx' points to an
outdated _ContextData.

This is easily fixable if we make PyThreadState to point to the
current Context object (instead of it pointing to a _ContextData).
This change will also make "contextvars.copy_context()" easier to
understand--it will actually return a copy of the current context that
the thread state points to.

Adding a private Context._in_use attribute would allow us to make sure
that Context.run() cannot be simultaneously called in two OS threads.
As Nathaniel points out, this will also simplify cache implementation
in ContextVar.get().  So let's do this.


4. Add Context.copy().

I was actually going to suggest this addition myself.  With the
current PEP 567 design, Context.copy() can be implemented with
"ctx.run(contextvars.copy_context)", but this is very cumbersome.

An example of when a copy() method could be useful is capturing the
current context and executing a few functions with it using
ThreadPoolExecutor.map().  Copying the Context object will ensure that
every mapped function executes in its own context copy (i.e.
isolated).  So I'm +1 for this one.


5. PEP language.

I agree that PEP is vague about some details and is incorrect in some
places (like calling Context objects immutable, which is not really
true, because .run() can modify them). I'll fix the language in v3
once I'm back home.


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

Reply via email to