Opened 10 months ago

Closed 6 months ago

Last modified 6 months ago

#22620 closed Cleanup/optimization (fixed)

"Multiple databases" chapter of docs says 'default':{} can start DATABASES setting; Django says no

Reported by: moc@… Owned by: nobody
Component: Documentation Version: 1.6
Severity: Normal Keywords: multiple_databases
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by timgraham)

Hi all. I refer to https://docs.djangoproject.com/en/1.5/topics/db/multi-db/ and I am using 1.5 but the later docs are the same. I quote from the docs:

"If the concept of a default database doesn’t make sense in the context of your project, you need to be careful to always specify the database that you want to use. Django requires that a default database entry be defined, but the parameters dictionary can be left blank if it will not be used."

So I did that and got the following:

"Internal Server Error: /admin/
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/core/handlers/base.py", line 115, in get_response
    response = callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/contrib/admin/sites.py", line 219, in wrapper
    return self.admin_view(view, cacheable)(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/utils/decorators.py", line 91, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/views/decorators/cache.py", line 89, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/contrib/admin/sites.py", line 196, in inner
    if not self.has_permission(request):
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/contrib/admin/sites.py", line 149, in has_permission
    return request.user.is_active and request.user.is_staff
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/utils/functional.py", line 204, in inner
    self._setup()
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/utils/functional.py", line 270, in _setup
    self._wrapped = self._setupfunc()
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/contrib/auth/middleware.py", line 18, in <lambda>
    request.user = SimpleLazyObject(lambda: get_user(request))
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/contrib/auth/middleware.py", line 10, in get_user
    request._cached_user = auth.get_user(request)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/contrib/auth/__init__.py", line 136, in get_user
    user_id = request.session[SESSION_KEY]
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/contrib/sessions/backends/base.py", line 44, in __getitem__
    return self._session[key]
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/contrib/sessions/backends/base.py", line 167, in _get_session
    self._session_cache = self.load()
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/contrib/sessions/backends/db.py", line 18, in load
    expire_date__gt=timezone.now()
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/models/manager.py", line 143, in get
    return self.get_query_set().get(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/models/query.py", line 398, in get
    num = len(clone)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/models/query.py", line 106, in __len__
    self._result_cache = list(self.iterator())
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/models/query.py", line 317, in iterator
    for row in compiler.results_iter():
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/models/sql/compiler.py", line 775, in results_iter
    for rows in self.execute_sql(MULTI):
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/models/sql/compiler.py", line 830, in execute_sql
    sql, params = self.as_sql()
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/models/sql/compiler.py", line 74, in as_sql
    out_cols = self.get_columns(with_col_aliases)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/models/sql/compiler.py", line 212, in get_columns
    col_aliases)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/models/sql/compiler.py", line 299, in get_default_columns
    r = '%s.%s' % (qn(alias), qn2(field.column))
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/models/sql/compiler.py", line 52, in quote_name_unless_alias
    r = self.connection.ops.quote_name(name)
  File "/usr/local/lib/python2.7/dist-packages/Django-1.5.4-py2.7.egg/django/db/backends/dummy/base.py", line 15, in complain
    raise ImproperlyConfigured("settings.DATABASES is improperly configured. "
ImproperlyConfigured: settings.DATABASES is improperly configured. Please supply the ENGINE value. Check settings documentation for more details."

I include the traceback just to show that nothing originated in code of mine. Now previously, and now in fact (I switched back), I have my project up and running fine with two databases and with the 'default' key value being the dictionary of parameters for the first database. So I have reason to suggest that perhaps the docs are wrong concerning the possibility of starting with 'default':{}. Maybe it's a legacy thing that is no longer viable.

If I could move on to another item on that very page... this is nit-picky I know but some of us are like that. Sorry. I'm talking about some lines at the very end of the page:

"For common setups with multiple databases, it isn’t useful to have these objects in more than one database... Therefore, it’s recommended:

either to run syncdb only for the default database;
..."

After reading it several times I concluded that by "these objects" was meant only the objects of the preceding paragraph, which include only Site, ContentType and Permission objects. However, I have just experimented and found that dropping from my second database all of the tables associated with ALL of the objects mentioned in the subsection "Behavior of contrib apps" causes no harm.

I therefore suggest that the language there could be clarified. Also, the mentioned option "either to run syncdb only for the default database" could use, say a parenthetic reference to the "$ ./manage.py sqlall sales | ./manage.py dbshell" approach... if that's what you have in mind for syncing the other database.

Attachments (1)

22620.diff (2.3 KB) - added by timgraham 6 months ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 10 months ago by moc@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I didn't mean to remain anonymous. I submitted this.

-Mike O'Connor
Shelton, WA

comment:2 Changed 10 months ago by timo

  • Resolution set to invalid
  • Status changed from new to closed

Thanks for the suggestions. I don't really see any issues with the points you've raised, but feel free to reopen this ticket if you want to provide a patch for anything that could be clarified.

For 1, I don't see an issue as the default database is being used to store sessions, hence the error. It is possible to configure Django so the default database isn't used, but you haven't done that.

For 2, I believe your initial interpretation is correct (there's the reference to "synchronizing these three models"). I think it'll be safer not to change this text.

For 3, I think the best approach is to use the --database flag to syncdb. Note that syncdb is replaced by migrate in 1.7 and this command doesn't have a similar flag, so I guess the approach will be different there.

comment:3 Changed 10 months ago by moc@…

  • Resolution invalid deleted
  • Status changed from closed to new
  • Version changed from 1.5 to 1.6

I'll deal with the "For n"'s of timo in turn:

"For 1, I don't see an issue as the default database is being used to store sessions, hence the error. It is possible to configure Django so the default database isn't used, but you haven't done that."

This about the ticket referring to the fact that the "Multiple databases" chapter clearly states and shows, with text and code, that you can, if you need to, start the DATABASES setting with 'default':{}... with an empty dictionary in place of the parameters for your first database, with 'mydb1':{usual params}, 'mydb2':{usual params} to follow.

So this ticket is of course about how the documentation of the "Multiple databases" chapter works or fails to work; it's not about how Django works. And the question is `if after following the instructions and code example of the 'default':{} approach, how is anyone to know FROM THIS CHAPTER that there is some other required step with which it "is possible to configure Django so the default database isn't used"--- that the reader might thus avoid the error?' Why isn't there an immediate mention of the need for this other configuration step, right next to the code example for the empty 'default' dictionary approach?

Perhaps a core developer can read the provided traceback, which pertains entirely to django internals, and immediately know what to do. But we do not acquire and use frameworks so that we can learn all of the source code and how it works.

On to the next For...

"For 2, I believe your initial interpretation is correct (there's the reference to "synchronizing these three models"). I think it'll be safer not to change this text."

Yes the ticket is nit-picky on that one, as it admits, but note especially that it says that I tested deleting ALL of the objects from my second database other than the tables from the models from one of my apps, the only app that uses the second database--- not just those three objects. There are no resultant problems. I did the deleting using "manage.py dbshell --database=mydb2", which opens the default sqlite2 database manager which allows me, with a ".tables" command to clearly see all of the tables and with SQL DROP statements to delete any one of them. Not only am I able to read and write to the two databases separately through views in different apps but I have implemented a "class MultiDBModelAdmin(admin.ModelAdmin):" and "class Mydb2AdminSite(AdminSite):" as advised in this chapter so as to have separate admin pages for each database--- and that works fine with all of the objects that are discussed in https://docs.djangoproject.com/en/1.6/topics/db/multi-db/#behavior-of-contrib-apps deleted from mydb2, not with just the three deleted. So maybe my initial interpretation is not correct. My test rather strongly suggests that it is not.

And now for the last For...

"For 3, I think the best approach is to use the ​--database flag to syncdb. Note that syncdb is replaced by migrate in 1.7 and this command doesn't have a similar flag, so I guess the approach will be different there."

About the latter tip, be advised that https://docs.djangoproject.com/en/1.7/topics/db/multi-db/#synchronizing-your-databases clearly shows a --database flag in use with migrate. However, timo is probably right about its demise because whereas https://docs.djangoproject.com/en/1.6/ref/django-admin/#syncdb shows the --database option https://docs.djangoproject.com/en/1.7/ref/django-admin/#django-admin-migrate does not. Something has to give there in one or the other of those chapters. Basically "I guess the approach will be different there" will have to be replaced by an actual approach.

About "the best approach is to use the ​--database flag to syncdb", strangely, the entire https://docs.djangoproject.com/en/1.6/topics/db/multi-db/#behavior-of-contrib-apps is offered in contradiction of the earlier https://docs.djangoproject.com/en/1.6/topics/db/multi-db/#synchronizing-your-databases section, which introduces the use of the --database flag with syncdb. In that earlier section it clearly states that:

"to synchronize all models onto all databases in our example (this is with the 'default' value of DATABASES containing a parameter dictionary, not empty), you would need to call:

$ ./manage.py syncdb
$ ./manage.py syncdb --database=users"

So it says that all models will go into all databases with this "--database flag to syncdb" recommendation. But that's precisely what behavior-of-contrib-apps tells us to avoid in part. My experience has been that those two syncs indeed put not only the three objects into each database but also most of the others mentioned in behavior-of-contrib-apps. Here's a cut a paste from an sqlite3 database manager terminal:

auth_group django_admin_log
auth_group_permissions django_content_type
auth_permission django_session
auth_user django_site
auth_user_groups ...
auth_user_user_permissions <plus my own apps' tables>

So syncdb --database=mydb2 isn't going to avoid the inclusion of any superfluous objects, hence the ticket's (timo-rejected) suggestion of referring to the "fine-grained" approach of synchronizing-your-databases that uses sqlall piped to dbshell.

-Mike

comment:4 Changed 10 months ago by timo

  • Triage Stage changed from Unreviewed to Accepted

I think it's beyond the scope of the documentation to describe how to configure Django without a default database (and I imagine it's not a very common use case), but we could add a note that with the default settings of storing sessions in the DB, it won't work.

I guess some improvements need to be made with respect to 2 and 3, though it seems rather complicated. I look forward to reviewing a patch if you are able to submit one.

For future reference, it would be better to submit a separate ticket for each individual issue, thanks.

comment:5 Changed 6 months ago by timgraham

  • Description modified (diff)
  • Has patch set

For (1), I added a sentence about using DATABASE_ROUTERS to avoid any queries to the default database if you want to run without one.

For (2), I added a similar sentence about using routers to avoid creating tables where you don't want them, but avoided any specific recommendation as I think it's best to let users figure out what tables they want and where.

For (3), after further investigation, migrate does support the --database flag and as far as I could tell the documentation is accurate with respect to how it works.

Note that for apps with migrations sqlall is replaced by sqlmigrate. Given the complexity of migrations, it seems like a bad idea to recommend that as a "fine-grained" approach the way sqlall was, so I removed that suggestion.

Feedback on the patch is welcome.

Changed 6 months ago by timgraham

comment:6 Changed 6 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 5ecead9ab993d3939068740bf195e2fff281c8ab:

Fixed #22620 -- Emphasized role of DATABASE_ROTERS in multi-db docs.

Thanks Mike O'Connor for the suggestions.

comment:7 Changed 6 months ago by Tim Graham <timograham@…>

In 403cc55f236d0877592f194ad34d6c1b5f0321ea:

[1.7.x] Fixed #22620 -- Emphasized role of DATABASE_ROTERS in multi-db docs.

Thanks Mike O'Connor for the suggestions.

Backport of 5ecead9ab9 from master

Note: See TracTickets for help on using tickets.
Back to Top