Re: Remove pyinotify autoreloader support

2018-10-08 Thread Tom Forbes
Thanks for the feedback! In the pull request I have re-added support for
pyinotify with tests, it was not as hard to write them as I believed. They
still fail, but I’m working on that!

I’ve found an interesting module confusingly called watchgod. The author
says[1] that with the new os.scandir() method added in python 3.5 a stat
based method can scan a project of 850 files in about 24ms.

While this is watching a single directory tree and not the entirety of
sys.modules I believe we could get similar speedups by being a bit more
intelligent in our approach. For example we really don’t need to stat every
single django.* module every second. I’m guessing that those change very
rarely (unless you’re hacking on Django!), so could maybe stat them every
2–3 seconds or even not at all. We could reduce the stating time on
site-packages modules as well for similar reasons.

If we do this right we could potentially get the overhead down to an
acceptable level that we don’t necessarily need platform specific watchers.

   1.
   
https://github.com/samuelcolvin/watchgod#why-no-inotify–kqueue–fsevent–winapi-support



On 6 October 2018 at 20:56:17, charettes (charett...@gmail.com) wrote:

While I understand the complexity of 1. I think shipping a version of
Django without
an equivalent inotify replacement such as watchdog could be problematic.

>From my personal experience when using Django's development server in Docker
containers sharing local volumes installing pyinotify resulted a
significant performance
CPU and I/O improvement.

Simon

Le samedi 6 octobre 2018 15:32:33 UTC-4, Tom Forbes a écrit :
>
> What do we think about removing the pyinotify functionality in the
> autoreloader? For context, if pyinotify is installed (Linux only) the
> autoreloader will attempt to use it to detect file changes over the
> standard polling-based approach. It is generally much more efficient but is
> not cross platform, and is not well documented currently IMO.
>
> I’m hacking away at my attempt at refactoring the autoreloader (
> https://github.com/django/django/pull/8819) and I’ve made some good
> progress but I am worried about the lack of tests for pyinotify (there are
> none!). The current code in master works but it’s really hard to refactor
> in any meaningful way without them. I would very much like to explore
> adding support for watchdog (https://pythonhosted.org/watchdog/) instead
> of pyinotify and these changes I’m working on are a means to that end.
>
> Wwatchdog is a library that wraps efficient platform-specific filesystem
> notifications in cross-platform way, and it includes an inotify
> implementation.
>
> So I think there are three ways forward:
>
>1.
>
>Spend time and effort adding special tests for PyInotify (testing this
>stuff is not simple, especially with the current code)
>2.
>
>Remove the functionality with an eye to using something like watchdog
>in the near future
>3.
>
>Never really touch the autoreloader much (it is some of the oldest and
>nastiest code we have).
>
> Does anyone have any strong opinions on this? Does anyone use the
> pyinotify speedup while developing?
>
> Tom
>
>
> --
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/62d4d8da-b5ca-4494-96ef-f58917b44e63%40googlegroups.com

.
For more options, visit https://groups.google.com/d/optout.

-- 
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/CAFNZOJNQvxQM3umqXbfRrRXJLsz6aJoSN1%2BKP7J5nMdeNV4TEg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Requiring sqlparse for sqlite introspection

2018-10-08 Thread Andrew Godwin
Adding sqlparse into the introspection code for SQLite specifically means
it's going to be a runtime dependency for anything to do with migrations.

I'm alright having that be the case, but I do think we should make sure the
tests run with SQLite as otherwise a large section of the most complicated
code in migrations won't be tested properly.

Andrew


On Mon, 8 Oct 2018, 00:59 Ian Foote,  wrote:

> Hi all,
>
> On my pull request (https://github.com/django/django/pull/10406)
> refactoring how Django creates database constraints I introduced a
> dependency on sqlparse in the sqlite introspection code. This allows Django
> to correctly read information about constraints on sqlite, particularly the
> name.
>
> When reviewing (
> https://github.com/django/django/pull/10406#issuecomment-424542217) Tim
> Graham raised the question of whether we should make sqlparse a mandatory
> requirement for the sqlite tests or if we should skip those tests that
> require it. In later discussion (
> https://github.com/django/django/pull/10406#issuecomment-427362983), Tim
> raised the point that the introspection code is used by migrations.
>
> I'm not clear myself what the best route forwards here is, so I'm bringing
> it to Django Developers for discussion (as Tim suggested).
>
> Thanks,
> Ian
>
> --
> 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/CAFv-zfKadOeWit8M6GMmx4H2ChUCU6u%3DscHX8F7oBKJkHRbuVg%40mail.gmail.com
> 
> .
> For more options, visit https://groups.google.com/d/optout.
>

-- 
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/CAFwN1upormR5UjCNMU7xXQiqeLtU%2BP-qBQAdgLx%2Be2Jno%2ByBew%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.