_create_test_db hides errors like 'source database "template1" is being accessed by other users' with --keepdb

[Daniel Hahler]

The _create_test_db method will hide errors like 'source database "template1" is being accessed by other users', and will assume that the test database exists already.

> …/pyenv/project/lib/python3.4/site-packages/psycopg2/__init__.py(165)connect()
    164     import ipdb; ipdb.set_trace()
--> 165     conn = _connect(dsn, connection_factory=connection_factory, async=async)
    166     if cursor_factory is not None:

ipdb> c
source database "template1" is being accessed by other users
DETAIL:  There are 3 other sessions using the database.

> …/pyenv/project/lib/python3.4/site-packages/django/db/backends/base/creation.py(458)_create_test_db()
    457                 # just return and skip it all.
--> 458                 if keepdb:
    459                     return test_database_name

Source reference: ​https://github.com/blueyed/django/blob/9e530b08d5858d7063d081b60ec86d24173e4df5/django/db/backends/base/creation.py#L146-L165

This will then result in an error when trying to connect to it, because it has not been created:

psycopg2.OperationalError: FATAL: database "test_project" does not exist

But instead the initial error should be displayed:

source database "template1" is being accessed by other users
DETAIL: There are 3 other sessions using the database.

To reproduce this:

• connect to the "template1" database
• run Django tests

I think the SQL could use CREATE DATABASE IF NOT EXISTS (in case IF NOT EXISTS) is supported by all backends (maybe that needs to be subclassed then), and then would not assume that an Exception can be ignored with keepdb.

An even better way would be to check if it exists, instead of trying to create it.

With pytest-django we're using the following code:

def test_database_exists_from_previous_run(connection):
    # Try to open a cursor to the test database
    test_db_name = connection.creation._get_test_db_name()

    # When using a real SQLite backend (via TEST_NAME), check if the file
    # exists, because it gets created automatically.
    if connection.settings_dict['ENGINE'] == 'django.db.backends.sqlite3':
        if not os.path.exists(test_db_name):
            return False

    orig_db_name = connection.settings_dict['NAME']
    connection.settings_dict['NAME'] = test_db_name

    # With SQLite memory databases the db never exists.
    if connection.settings_dict['NAME'] == ':memory:':
        return False

    try:
        connection.cursor()
        return True
    except Exception:  # TODO: Be more discerning but still DB agnostic.
        return False
    finally:
        connection.close()
        connection.settings_dict['NAME'] = orig_db_name

(Source: ​https://github.com/blueyed/pytest_django/blob/93fca47feea39016dd93e657a9328450e9b6e891/pytest_django/db_reuse.py#L11-L35)

[Daniel Hahler]

(just for reference: I've noticed this when making pytest-django use Django's keepdb method - ​https://github.com/pytest-dev/pytest-django/pull/261)

[Tim Graham]

Yes, this could be improved.

[Daniel Hahler]

Another example is "ProgrammingError: permission denied to create database".

I think an easy improvement would be to log the exception always.
And/or really check if the database exists already when skipping it for "keepdb".

Is it possible to use "CREATE DABASE ... IF NOT EXISTS" or something similar here?

[Mariusz Felisiak]

​PR

[Tim Graham <timograham@…>]

In 64264c9a:

Fixed #25406 -- Removed exception hiding in PostgreSQL test database creation during --keepdb.

Thanks Markus Holtermann and Tim Graham for reviews.

[GitHub <noreply@…>]

In e1253bc2:

Refs #25406 -- Removed exception hiding in MySQL test database creation during --keepdb.

Thanks Adam Johnson, Simon Charette and Tim Graham for reviews.