Opened 4 years ago

Closed 22 months ago

Last modified 4 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: master
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 4 years ago.
16969-2.diff (4.4 KB) - added by claudep 23 months ago.

Download all attachments as: .zip

Change History (17)

Changed 4 years ago by andreas_pelme

comment:1 Changed 4 years ago by carljm

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 Changed 4 years ago by aaugustin

The test suite runs under Oracle with the patch applied.

comment:3 Changed 4 years ago by aaugustin

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

comment:4 Changed 23 months ago by timo

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.

Changed 23 months ago by claudep

comment:5 Changed 23 months ago by claudep

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

comment:6 Changed 23 months ago by shai

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 Changed 23 months ago by claudep

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

comment:8 Changed 23 months ago by shai

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 Changed 23 months ago by claudep

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

comment:10 Changed 22 months ago by timo

  • Triage Stage changed from Accepted to Ready for checkin

comment:11 Changed 22 months ago by Claude Paroz <claude@…>

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

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 Changed 22 months ago by Claude Paroz <claude@…>

In 7e714827ead50f77aa82394cc2511ff96ab67fa4:

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

Refs #16969.

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

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 Changed 4 months ago by blueyed

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 Changed 4 months ago by timgraham

Created ticket #24791 for that issue.

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