Re: Add warning or prohibit module level queries?

[Curtis Maloney]

+1

At the very least we need to provide better feedback or errors to help 
isolate _where_ this is happening.


I've helped quite a number of people on IRC with this problem... 
firstly, people aren't understanding _what_ the problem is, but also 
discerning where it's happening is often quite difficult, even with a 
full traceback.


--
C


On 26/02/16 04:29, Tim Graham wrote:

Simon proposed [0]: "I wonder if we should prevent django.db from
executing queries until django.apps.apps.ready or at least issue a
RuntimeWarning. We would have to go through deprecation but I'm pretty
sure this would uncover a lot of existing application bugs and prevent
future ones. This is related to #25454
 [1] and probably a large
number of closed tickets."


We have this restriction in some places, for example: "Executing
database queries with the ORM at import time in models modules will also
trigger this exception. The ORM cannot function properly until all
models are available."

We also have a warning about using the ORM in AppConfig.ready():
"Although you can access model classes as described above, avoid
interacting with the database in your |ready()| implementation. This
includes model methods that execute queries (|save()|, |delete()|,
manager methods etc.), and also raw SQL queries via
|django.db.connection|. Your |ready()| method will run during startup of
every management command. For example, even though the test database
configuration is separate from the production settings, |manage.py test|
would still execute some queries against your *production* database!"

There's also a warning in the testing docs: "Finding data from your
production database when running tests? If your code attempts to access
the database when its modules are compiled, this will occur /before/ the
test database is set up, with potentially unexpected results. For
example, if you have a database query in module-level code and a real
database exists, production data could pollute your tests. /It is a bad
idea to have such import-time database queries in your code/ anyway -
rewrite your code so that it doesn’t do this. This also applies to
customized implementations of |ready()|."

What do you think? Prohibiting such queries might be too strict at this
point as I guess some users might rely on them. I suppose warnings could
be selectively silenced as/if needed. We could start with a warning and
ask users to let us know if they believe they have a legitimate usage.
If we don't hear anything, we could proceed with the deprecation.

Related tickets:

[0] https://code.djangoproject.com/ticket/26273
[1] https://code.djangoproject.com/ticket/25454

--
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/7f41ba58-09ed-4f78-b5ce-be6d7d5a6fc7%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/56CF8327.6080202%40tinbrain.net.
For more options, visit https://groups.google.com/d/optout.

Re: Add warning or prohibit module level queries?

[Aymeric Augustin]

Hello,

To the best of my knowledge, in Django 1.10, the window for such problematic 
database queries will shrink to the interval between the moment 
`apps.models_ready` becomes `True` and the moment `apps.ready` becomes `True`. 
That’s when AppConfig.ready() methods execute.

In Django 1.9, if a developer references their views with “Django string magic" 
in their URLconf, database queries can also easily happen during the first 
request, when modules containing views get imported. (URLconfs are lazily 
loaded, aren’t they?) This will go away in 1.10, though.

Going back to Django 1.10, I’m wondering if forbidding database queries in 
AppConfig.ready() methods would restrict their usefulness significantly. From 
Django’s perspective, it’s safe to use the ORM at that point (except when 
running tests). It’s pretty clear AppConfig.ready() executes exactly once at 
startup. Developers should expect querysets fetched in AppConfig.ready() not to 
change over the application’s lifetime.

One might consider the ORM not to be safe until `apps.ready` becomes `True` 
e.g. if a developer alters models dynamically in an AppConfig.ready(). Well, in 
that case, it’s up to them to ensure their project remains consistent. There’s 
only so much safety Django can provide in the presence of monkey patching.

I think additional efforts should focus primarily on importing all modules 
during the app loading process, to avoid importing additional modules on the 
first request (the most common case) or on later requests (a possible case, if 
imports are put inside functions instead of at module level.) Currently that’s 
the most likely way import-time database queries and the corresponding bugs can 
still occur in reasonable Django projects.

The problem of running queries before switching to the test database is a real 
one, though, and sways me from -0 to +0 on at least warning and perhaps 
erroring out when queries are run before `apps.ready` becomes `True`. Tim’s 
proposal looks reasonable and consistent with the current documentation.

Best regards,

-- 
Aymeric.

> On 25 févr. 2016, at 18:29, Tim Graham  wrote:
> 
> Simon proposed [0]: "I wonder if we should prevent django.db from executing 
> queries until django.apps.apps.ready or at least issue a RuntimeWarning. We 
> would have to go through deprecation but I'm pretty sure this would uncover a 
> lot of existing application bugs and prevent future ones. This is related to 
> #25454  [1] and probably a large 
> number of closed tickets."
> 
> 
> We have this restriction in some places, for example: "Executing database 
> queries with the ORM at import time in models modules will also trigger this 
> exception. The ORM cannot function properly until all models are available."
> 
> We also have a warning about using the ORM in AppConfig.ready(): "Although 
> you can access model classes as described above, avoid interacting with the 
> database in your ready() implementation. This includes model methods that 
> execute queries (save(), delete(), manager methods etc.), and also raw SQL 
> queries via django.db.connection. Your ready() method will run during startup 
> of every management command. For example, even though the test database 
> configuration is separate from the production settings, manage.py test would 
> still execute some queries against your production database!"
> 
> There's also a warning in the testing docs: "Finding data from your 
> production database when running tests? If your code attempts to access the 
> database when its modules are compiled, this will occur before the test 
> database is set up, with potentially unexpected results. For example, if you 
> have a database query in module-level code and a real database exists, 
> production data could pollute your tests. It is a bad idea to have such 
> import-time database queries in your code anyway - rewrite your code so that 
> it doesn’t do this. This also applies to customized implementations of 
> ready()."
> 
> What do you think? Prohibiting such queries might be too strict at this point 
> as I guess some users might rely on them. I suppose warnings could be 
> selectively silenced as/if needed. We could start with a warning and ask 
> users to let us know if they believe they have a legitimate usage. If we 
> don't hear anything, we could proceed with the deprecation.
> 
> Related tickets:
> 
> [0] https://code.djangoproject.com/ticket/26273 
> 
> [1] https://code.djangoproject.com/ticket/25454 
> 
> 
> -- 
> 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 
> 

Add warning or prohibit module level queries?

[Tim Graham]

Simon proposed [0]: "I wonder if we should prevent django.db from executing 
queries until django.apps.apps.ready or at least issue a RuntimeWarning. We 
would have to go through deprecation but I'm pretty sure this would uncover 
a lot of existing application bugs and prevent future ones. This is related 
to #25454  [1] and probably a 
large number of closed tickets."

We have this restriction in some places, for example: "Executing database 
queries with the ORM at import time in models modules will also trigger 
this exception. The ORM cannot function properly until all models are 
available."

We also have a warning about using the ORM in AppConfig.ready(): "Although 
you can access model classes as described above, avoid interacting with the 
database in your ready() implementation. This includes model methods that 
execute queries (save(), delete(), manager methods etc.), and also raw SQL 
queries via django.db.connection. Your ready() method will run during 
startup of every management command. For example, even though the test 
database configuration is separate from the production settings, manage.py 
test would still execute some queries against your *production* database!"

There's also a warning in the testing docs: "Finding data from your 
production database when running tests? If your code attempts to access the 
database when its modules are compiled, this will occur *before* the test 
database is set up, with potentially unexpected results. For example, if 
you have a database query in module-level code and a real database exists, 
production data could pollute your tests. *It is a bad idea to have such 
import-time database queries in your code* anyway - rewrite your code so 
that it doesn’t do this. This also applies to customized implementations of 
ready()."

What do you think? Prohibiting such queries might be too strict at this 
point as I guess some users might rely on them. I suppose warnings could be 
selectively silenced as/if needed. We could start with a warning and ask 
users to let us know if they believe they have a legitimate usage. If we 
don't hear anything, we could proceed with the deprecation.

Related tickets:

[0] https://code.djangoproject.com/ticket/26273
[1] https://code.djangoproject.com/ticket/25454

-- 
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/7f41ba58-09ed-4f78-b5ce-be6d7d5a6fc7%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.