On Mon, May 13, 2019 at 4:31 AM Tobias Kunze <r...@rixx.de> wrote:
> Hi Andrew (and everybody following the discussion, of course),
>
> First off, thank you for your work here. DEP9 is an excellent technical
> document, and it was as easy and pleasant to read as a document of this
> scope and depth can be.
>
> Especially the Motivation section was very insightful – it might be
> worth moving it up a bit, as I found myself dropping about a third of my
> notes and questions from the first half of the DEP after reaching the
> motivation section.
> Also, discussing the Why first and the How later is a bit better
> argumentatively – if I just spent half an hour reading about the How,
> I'll start to see the Why as a given, so it's harder to reason about
> alternatives upon reaching the motivation section. Maybe that's just me,
> though.
>
The ordering that's there is defined by the DEP template, alas - though
maybe that's just because of historical reasons. I agree it might be better
to swap them.
>
> The Sequencing section is equally helpful to get a feeling for the
> implementation work. It might be worth including a note on additional
> future DEPs there, as those are only mentioned in the high level summary
> (and in the templating section, I think).
>
I've added another paragraph in there to flesh it out a bit.
>
> I have a couple of questions and comments – none of which are meant to
> criticise the direction of the DEP or arguments. It's just niggles,
> really. I also left some comments on the PR that concern only some
> phrasing.
>
> The DEP doesn't really include a discussion of potential downsides. Even
> if that's not in scope of a DEP, I'd like to ask this here:
>
> - Is there a potential negative impact for Django users who just
> continue to use Django? The only mention of this says "Running code in
> threads is likely not going to increase performance - the overhead
> added will probably decrease it very slightly in the case where you're
> just running normal, linear code". Do you have any details on that? I
> would have expected some (even just a sentence or two) discussion of
> potential downsides in the Rationale section next to the alternatives.
> - Is there a potential negative impact on Django if work on this
> proposal takes a long time/is abandoned?
>
These are both good questions - I have added in explainers about them to
the "Motivation" section. (In short: Yes, but we'll keep it to a small
impact; No, as long as we're good about code committing and being
sustainable).
>
>
> Less general remarks:
>
> > Every single feature that is converted to be async internally will also
> present
> > a synchronous interface that is backwards-compatible with the API as it
> stands
> > today (in 2.2), at least for the normal deprecation period.
>
> As it stands, this will sound a lot like "we'll deprecate lots of
> synchronous interfaces soon" to people who are afraid of exactly that.
> It's probably worth to clarify this here, or to choose different
> phrasing, unless that's what you're planning to do.
>
Good point, I've made this a lot clearer.
>
> > This general overview works on nearly all features on Django that need
> to be
> > async, with the exceptions mostly being places where the Python language
> itself
> > does not provide async equivalents to features we already use.
>
> Can you give examples for this? They don't need to be in the draft, I
> think, I'd just like to understand this part better.
>
The classic one is attribute access - Django uses "
model_instance.related_field.name" to do work in the background. Attribute
access can't be async, though, so we can no longer call the database.
It also exists for length - doing "len(queryset)" does not pass down an
async context, so you can't do blocking work down there in a subthread. The
pattern continues for most things that Python makes objects supply
__special__ methods for; unless serious work is done to Python as an async
language, these will likely stay the same indefinitely.
>
> > Asynchronous views will continue to be wrapped in an ``atomic()`` block
> by
> > default - while this reduces immediate performance gains, as it will
> lock all
> > ORM queries to a single subthread (see "The ORM" below), it is what our
> users
> > will expect and much safer. If they want to run ORM queries
> concurrently, they
> > will have to explicitly opt out of having the transaction around the
> view using
> > the existing ``non_atomic_requests`` mechanism, though we will need to
> improve
> > the documentation around it.
>
> By default, Django's views are not wrapped in ``atomic()`` blocks. This
> is only the case if ``ATOMIC_REQUESTS`` is ``True``, which it isn't by
> default. Not sure if an off-by-default feature is worth an entire
> paragraph here, but in any case, please make it clear that not every
> async view will be wrapped in an ``atomic()`` block (unless I'm mistaken
> and they will be?).
>
I have updated this. I have kept the paragraph about it because I think
it's an important illustrative point about the whole problem.
>
> > In some ways, this
> > will end up looking more like Django 1.0 era middleware again from an
> internal
> > perspective.
>
> It might be worth to make it clear that the middleware interface doesn't
> change on the user-facing side, though.
>
Done!
>
> > Whenever a
> > ``new_connections()`` block is entered, the transactions do not persist
> inside,
> > but transactions can be made inside the ``new_connections()`` block and
> run
> > against those connections.
>
> I think this was the most complicated sentence in the entire document.
> It took me several tries to parse it in a way that could be correct.
> Could you try to clarify? I think the missing reference for the "the
> transactions" and "those connections" probably led to my confusion.
>
I reworked this quite a bit to be a lot clearer about transactions and a
point about isolation levels that I think is important to bring up.
>
> > This means that, at some point, the ``valid`` methods and ``save``, at
> > minimum, need to be able to be called in an async fashion.
>
> The ``valid`` methods? Did you mean the ``clean`` (and ``clean_*``)
> methods, or am I missing something?
>
I did mean that. It has been a bit too long since I wrote form code!
>
> > While Python is not a perfect asynchronous language, and there are some
> flaws
> > in the core design of ``asyncio``,
>
> This leads me to a question I haven't really seen discussed so far: How
> stable is the asyncio API by now? In the 3.x releases so far, asyncio
> API has shifted quite a bit. I looked through the 3.8 release notes, but
> didn't see any major changes (except maybe task names). This is mostly
> relevant to figure out how much work we'll have with the support of
> future Python version, and/or keeping backwards compatibility.
>
It is mostly-stable, and everyone I talk to (myself included) is slightly
unsatisfied with it but generally thinks it does the job, which makes me
believe it's probably an alright API.
It's certainly more of a moving target than Python itself is, though. If
Django adopts async in a big way and becomes a large user of asyncio, we
may have a knockback effect on its development, for better or worse.
>
> You're mentioning `asgiref` a lot – do you expect it to become a
> dependency of Django? Do you expect any other new dependencies to be
> introduced?
>
I do expect it to, and in fact the ASGI patch already does this, and causes
`async_timeout` to be required too (as asgiref needs this). I expect no
other dependencies. I consider `asgiref` acceptable as it is a Django
project, though I have considered just copying over ("vendoring") the code
we need from async_timeout to remove that extra dependency.
>
> Thank you again for your work (and making it through this “light reading”)
>
>
And thank you for your feedback and your GIF :)
Andrew
--
You received this message because you are subscribed to the Google Groups
"Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/CAFwN1upxvhSjxRMbCvWFmqA0DDjojGVyiCO235PmnagL%2BZ%2BEoA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
