Opened 12 years ago

Closed 10 years ago

Last modified 13 months ago

#16969 closed Cleanup/optimization (fixed)

Avoid selecting production database prior to test database setup/teardown

Reported by: Andreas Pelme Owned by: nobody
Component: Testing framework Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am playing around with running my test suite with pytest and pytest-xdist. I would like to be able to run my tests in parallell, which would probably be a massive speedup on a decent multi-core CPU. To isolate tests properly, I am dynamically modifying the database name in the settings, to create a separate test database for each process.

However, when the test database is created, Django creates a cursor to the production database before setting up/destroying the test database. Since I am generating the database name dynamically in each process, this causes problems for me, since I do not know the name and can create it beforehand. I see no reason for doing this, so I would like to skip selecting the production database before creating/destroying the test database.

The behavior is when creating a cursor without specifying a database varies a bit across the backends:

  • sqlite - this is not a problem
  • mysql - it is possible to omit the database name
  • postgesql - a database name must be given, but rather than choosing the production name, the default "postgesql" database can be used
  • oracle - I am unable to test this since I do not have an Oracle database available

I have modified django/db/backends/creation.py to set connection.settings_dictNAME to None instead of the production database name during creation/destroy. I also made a small change in the postgres backend to use the "postgesql" database if None is supplied.

I have run the Django test suite for sqlite, mysql and postgesql and this does not seems to cause any problems.

Attachments (2)

test_db_creation.diff (2.2 KB ) - added by Andreas Pelme 12 years ago.
16969-2.diff (4.4 KB ) - added by Claude Paroz 10 years ago.

Download all attachments as: .zip

Change History (18)

by Andreas Pelme, 12 years ago

Attachment: test_db_creation.diff added

comment:1 by Carl Meyer, 12 years ago

Has patch: set
Triage Stage: UnreviewedAccepted

If its possible to avoid connecting to the production DB at all for test runs, I think we should do this. Needs someone with Oracle to test the behavior there.

comment:2 by Aymeric Augustin, 12 years ago

The test suite runs under Oracle with the patch applied.

comment:3 by Aymeric Augustin, 12 years ago

Under PostgreSQL, is it guaranteed that all users can always connect to the postgres database? Otherwise, the patch is incorrect.

comment:4 by Tim Graham, 10 years ago

Regarding PostgreSQL, the answer appears to be "Yes". From the PostgreSQL docs:

"The postgres database is a default database meant for use by users, utilities and third party applications."

Additional information in this stackoverflow question.

by Claude Paroz, 10 years ago

Attachment: 16969-2.diff added

comment:5 by Claude Paroz, 10 years ago

Here's an alternative patch. Should be thoroughly reviewed!

comment:6 by Shai Berger, 10 years ago

Oracle does not support "databases" in the sense that other backends do; the NAME parameter is actually part of selecting the server you connect to. The tests worked with the patch because the patch changed the generic create_test_db function, which the Oracle backend overrides. In fact, Oracle doesn't use the TEST_NAME at all (except for printing) -- it uses a TEST_USER and TEST_TBLSPACE instead.

To summarize: On Oracle, you connect as a user to a server. There is only one "database". You cannot avoid connecting to it, AFAIK.

comment:7 by Claude Paroz, 10 years ago

In my patch, then, what about subclassing nodb_connection() in the Oracle backend and returning self.connection?

comment:8 by Shai Berger, 10 years ago

There's no need for that, the Oracle backend already overrides _create_test_db() as well as _destroy_test_db(), so it wouldn't use nodb_connection().

On point that rises, though, is that nodb_connection() should probably be named as a private method (_nodb_connection()).

comment:9 by Claude Paroz, 10 years ago

Pull request created (using _nodb_connection() private name):
https://github.com/django/django/pull/1788

comment:10 by Tim Graham, 10 years ago

Triage Stage: AcceptedReady for checkin

comment:11 by Claude Paroz <claude@…>, 10 years ago

Resolution: fixed
Status: newclosed

In e953c78eeb81ee69dccd356145563fd6f9e4c7b6:

Fixed #16969 -- Don't connect to named database when possible

Thanks Andreas Pelme for the report and initial patch, and
Aymeric Augustin, Shai Berger and Tim Graham for the reviews.

comment:12 by Claude Paroz <claude@…>, 10 years ago

In 7e714827ead50f77aa82394cc2511ff96ab67fa4:

Don't initialize PostGIS-specific stuff for non-db connections

Refs #16969.

comment:13 by Tim Graham <timograham@…>, 10 years ago

In 0086c9eb48101b6dabefa7f9195ac7d197780f09:

[1.7.x] Fixed #22417 -- Added additional documentation for refs #16969.

Thanks Jon Foster for the report.

Backport of 1b3a3fc1e4 from master

comment:14 by Daniel Hahler, 9 years ago

This prevents Django from being able to run tests on Heroku, where you are not
allowed to connect to the "postgres" database, at least with the free tier.

Would it be an option to use an explicitly configured
DATABASES['default']['TEST']['NAME'] setting, instead of 'postgres' here?

Or could there be a new setting, like
DATABASES['default']['TEST']['CONNECT_NAME']?

The traceback, for reference (Django 1.8.1):

.heroku/python/lib/python2.7/site-packages/pytest_django/fixtures.py:53: 
>           db_cfg = setup_databases(verbosity=0, interactive=False)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.heroku/python/lib/python2.7/site-packages/django/test/runner.py:370: in setup_databases
    serialize=connection.settings_dict.get("TEST", {}).get("SERIALIZE", True),
.heroku/python/lib/python2.7/site-packages/django/db/backends/base/creation.py:354: in create_test_db
    self._create_test_db(verbosity, autoclobber, keepdb)
.heroku/python/lib/python2.7/site-packages/django/db/backends/base/creation.py:447: in _create_test_db
    with self._nodb_connection.cursor() as cursor:
.heroku/python/lib/python2.7/site-packages/django/db/backends/base/base.py:164: in cursor
    cursor = self.make_cursor(self._cursor())
.heroku/python/lib/python2.7/site-packages/django/db/backends/base/base.py:135: in _cursor
    self.ensure_connection()
.heroku/python/lib/python2.7/site-packages/django/db/backends/base/base.py:130: in ensure_connection
    self.connect()
.heroku/python/lib/python2.7/site-packages/django/db/utils.py:97: in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
.heroku/python/lib/python2.7/site-packages/django/db/backends/base/base.py:130: in ensure_connection
    self.connect()
.heroku/python/lib/python2.7/site-packages/django/db/backends/base/base.py:119: in connect
    self.connection = self.get_new_connection(conn_params)
.heroku/python/lib/python2.7/site-packages/django/db/backends/postgresql_psycopg2/base.py:172: in get_new_connection
    connection = Database.connect(**conn_params)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

    ...

        if dsn is None:
            if not items:
                raise TypeError('missing dsn and no parameters')
            else:
                dsn = " ".join(["%s=%s" % (k, _param_escape(str(v)))
                    for (k, v) in items])

>       conn = _connect(dsn, connection_factory=connection_factory, async=async)
E       OperationalError: FATAL:  permission denied for database "postgres"
E       DETAIL:  User does not have CONNECT privilege.

.heroku/python/lib/python2.7/site-packages/psycopg2/__init__.py:164: OperationalError

comment:15 by Tim Graham, 9 years ago

Created ticket #24791 for that issue.

comment:16 by GitHub <noreply@…>, 13 months ago

In 5afd8c6:

Refs #16969 -- Added test for not initializing PostGIS-specific stuff for non-db connections.

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