[Django] #16969: Avoid selecting production database prior to test database setup/teardown
Django
--------------------------------------+--------------------
Reporter: andreas_pelme | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: SVN
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
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_dict['NAME'] 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/16969>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: SVN
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by carljm):
* needs_better_patch: => 0
* has_patch: 0 => 1
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:1>
Django
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: SVN
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Comment (by aaugustin):
The test suite runs under Oracle with the patch applied.
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:2>
Django
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: SVN
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Comment (by aaugustin):
Under PostgreSQL, is it guaranteed that all users can always connect to
the `postgres` database? Otherwise, the patch is incorrect.
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:3>
Django
Reporter: andreas_pelme | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
Regarding PostgreSQL, the answer appears to be "Yes". From the
[http://www.postgresql.org/docs/9.1/interactive/app-initdb.html PostgreSQL
docs]:
"The `postgres` database is a default database meant for use by users,
utilities and third party applications."
Additional information in this [http://stackoverflow.com/questions/2370525
/default-database-named-postgres-on-postgresql-server stackoverflow
question].
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:4>
Django
Reporter: andreas_pelme | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
Here's an alternative patch. Should be thoroughly reviewed!
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:5>
Django
Reporter: andreas_pelme | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:6>
Django
Reporter: andreas_pelme | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
In my patch, then, what about subclassing `nodb_connection()` in the
Oracle backend and returning `self.connection`?
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:7>
Django
Reporter: andreas_pelme | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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()`).
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:8>
Django
Reporter: andreas_pelme | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
Pull request created (using `_nodb_connection()` private name):
https://github.com/django/django/pull/1788
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:9>
Django
Reporter: andreas_pelme | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Testing framework | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:10>
Django
Reporter: andreas_pelme | Owner: nobody
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Testing framework | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz <claude@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"e953c78eeb81ee69dccd356145563fd6f9e4c7b6"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:11>
Django
Reporter: andreas_pelme | Owner: nobody
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Testing framework | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz <claude@…>):
In [changeset:"7e714827ead50f77aa82394cc2511ff96ab67fa4"]:
{{{
#!CommitTicketReference repository=""
revision="7e714827ead50f77aa82394cc2511ff96ab67fa4"
Don't initialize PostGIS-specific stuff for non-db connections
Refs #16969.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:12>
Django
Reporter: andreas_pelme | Owner: nobody
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Testing framework | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"0086c9eb48101b6dabefa7f9195ac7d197780f09"]:
{{{
#!CommitTicketReference repository=""
revision="0086c9eb48101b6dabefa7f9195ac7d197780f09"
[1.7.x] Fixed #22417 -- Added additional documentation for refs #16969.
Thanks Jon Foster for the report.
Backport of 1b3a3fc1e4 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:13>
Django
Reporter: andreas_pelme | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:14>
Django
Reporter: andreas_pelme | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Testing framework | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Created ticket #24791 for that issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:15>
Django
Reporter: Andreas Pelme | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Testing framework | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by GitHub <noreply@…>):
In [changeset:"5afd8c69404d718d254399fb5804efe0aa24fa8a" 5afd8c6]:
{{{
#!CommitTicketReference repository=""
revision="5afd8c69404d718d254399fb5804efe0aa24fa8a"
Refs #16969 -- Added test for not initializing PostGIS-specific stuff for
non-db connections.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16969#comment:16>