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 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 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? 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. > 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. > 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?). > 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. > 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. > 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? > 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. 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? Thank you again for your work (and making it through this “light reading”) Tobias -- 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/20190512193117.gujh2weuutqpuobk%40cordelia.localdomain. For more options, visit https://groups.google.com/d/optout.
signature.asc
Description: PGP signature
