Opened 3 years ago
Last modified 2 years ago
#33685 new New feature
Support using service names for tests on PostgreSQL.
Reported by: | Shane Ambler | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | 4.0 |
Severity: | Normal | Keywords: | |
Cc: | Hasan Ramezani, Carlton Gibson | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In django 4.0 the postgresql DATABASE settings were changed to allow use of a .pg_service.conf and .pgpass file being defined in the OPTIONS value.
Using the old config works fine in both runserver and test.
Using the new config works for runserver but fails for test.
In settings I have :-
DATABASES = { 'default': { 'ENGINE': 'django.db.backends.postgresql', 'OPTIONS': { 'service': 'yfsm_pgdb', 'passfile': '.yfsm_pgpass', }, }, }
I have the user password entry allowing login to any database (using *
) I have also tried adding an entry with test_dbname
. I also tried adding an entry in .pg_service.conf as [test_yfsm_pgdb]
running tests I see django.db.utils.OperationalError: fe_sendauth: no password supplied
indicating that the password file is not used.
It would appear that when testing, the new OPTIONS settings are ignored.
Change History (18)
comment:1 by , 3 years ago
comment:2 by , 3 years ago
Component: | Testing framework → Documentation |
---|
comment:3 by , 3 years ago
Component: | Documentation → Testing framework |
---|
comment:4 by , 3 years ago
I failed to get my setting file right when I checked that, the use of the django_test
service name is part of the django project tests.
The use of postgresql service name still appears broken with testing
follow-up: 6 comment:5 by , 3 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
If you want to user PostgreSQL's password files, all required databases must be included, i.e. postgres
, dbname
, test_dbname
, e.g. the following configuration works for me
localhost:5432:postgres:postgres_username:SeCrEtPassworD localhost:5432:dbname:postgres_username:SeCrEtPassworD localhost:5432:test_dbname:postgres_username:SeCrEtPassworD
settings.py
:
DATABASES = { 'default': { 'ENGINE': 'django.db.backends.postgresql', 'HOST': 'localhost', 'PORT': 5432, 'NAME': 'dbname', 'USER': 'postgres_username', 'OPTIONS': { 'passfile': '.custompgpass', } }, }
follow-up: 8 comment:6 by , 3 years ago
Replying to Mariusz Felisiak:
If you want to user PostgreSQL's password files, all required databases must be included, i.e.
postgres
,dbname
,test_dbname
, e.g. the following configuration works for me
localhost:5432:postgres:postgres_username:SeCrEtPassworD localhost:5432:dbname:postgres_username:SeCrEtPassworD localhost:5432:test_dbname:postgres_username:SeCrEtPassworD
Yes, I tried both the test_dbname
and *
to match any dbname and it still fails.
settings.py
:
DATABASES = { 'default': { 'ENGINE': 'django.db.backends.postgresql', 'HOST': 'localhost', 'PORT': 5432, 'NAME': 'dbname', 'USER': 'postgres_username', 'OPTIONS': { 'passfile': '.custompgpass', } }, }
While you are maintaining both old and new config variations, all will continue to work for you.
If you remove the old, no longer documented, settings of HOST
PORT
NAME
USER
then your setup will work for runserver
but not test
The pg docs https://docs.djangoproject.com/en/4.0/ref/databases/#postgresql-notes-1 only documents the new method of specifying service
and passfile
in the OPTIONS
- this documented setup does not support running tests with postgresql databases.
comment:7 by , 3 years ago
Resolution: | invalid |
---|---|
Status: | closed → new |
comment:8 by , 3 years ago
Summary: | Using new postgresql settings fails when running tests → Tests crash when using only a service name on PostgreSQL. |
---|---|
Triage Stage: | Unreviewed → Accepted |
While you are maintaining both old and new config variations, all will continue to work for you.
If you remove the old, no longer documented, settings of
HOST
PORT
NAME
USER
then your setup will work forrunserver
but nottest
The pg docs https://docs.djangoproject.com/en/4.0/ref/databases/#postgresql-notes-1 only documents the new method of specifyingservice
andpassfile
in theOPTIONS
...
Using ['OPTIONS']['service']
and ['OPTIONS']['passfile']
is not a "new" or only supported way for the DATABASES
settings. It's a PostgreSQL-specific option that you may use that's why it's documented only in the "PostgreSQL notes". Using HOST
, PORT
, NAME
, USER
, and PASSWORD
is still fully supported for PostgreSQL, see docs.
... this documented setup does not support running tests with postgresql databases.
NAME
is used in many places for test setup, including cloning a test database, that's why it doesn't work with only service
specified.
comment:9 by , 3 years ago
Replying to Mariusz Felisiak:
Using
['OPTIONS']['service']
and['OPTIONS']['passfile']
is not a "new" or only supported way for theDATABASES
settings.
It looks like it has been available in "main" for a bit over a year, but it is still "new" as it has only been available in one public release. Yes, the old (or generic) method still works and will work properly if it is the only config docs that you find.
Using ['DATABASE']['NAME']
and ['DATABASE']['USER']
will work for runserver as well as test and they will override any service file settings.
We shouldn't need to duplicate login details in two places to get basic functionality.
I see having tests fail on the first tutorial as a big entry barrier for new users.
comment:10 by , 3 years ago
Severity: | Normal → Release blocker |
---|
It looks like it has been available in "main" for a bit over a year, but it is still "new" as it has only been available in one public release.
Right, good point, it's a new feature so this ticket should be marked as a release blocker.
follow-up: 12 comment:11 by , 3 years ago
Just a suggestion for whoever looks into this :
I expect that the service name is being passed to psycopg2, making altering the dbname awkward, in that case I think prepending 'test_' to the service name would be a viable solution. Then the docs would show two service entries to support testing in the example.
comment:12 by , 3 years ago
Replying to Shane Ambler:
Just a suggestion for whoever looks into this :
I expect that the service name is being passed to psycopg2, making altering the dbname awkward, in that case I think prepending 'test_' to the service name would be a viable solution. Then the docs would show two service entries to support testing in the example.
Agreed, I was thinking about the same solution. Unfortunately it's quite tricky to implement this duality :|
comment:13 by , 3 years ago
Cc: | added |
---|
I've tried to implement this but all mechanism related with creating/cloning test databases are based on the database name. It's feasible, however, my draft patch is quite complex and the risk of introducing regressions in a stable version is too great, IMO. I'd document that service names cannot be currently used for testing purposes and treat this as a new feature request.
For example:
-
docs/ref/databases.txt
diff --git a/docs/ref/databases.txt b/docs/ref/databases.txt index c270f56942..ca6362a7a6 100644
a b password from the `password file`_, you must specify them in the 165 165 Support for connecting by a service name, and specifying a password file 166 166 was added. 167 167 168 .. warning:: 169 170 Using a service name for testing purposes is not supported. This 171 :ticket:`may be implemented later <33685>`. 172 168 173 Optimizing PostgreSQL's configuration 169 174 ------------------------------------- 170 175
comment:14 by , 3 years ago
however, my draft patch is quite complex and the risk of introducing regressions in a stable version is too great,
Agree with you.
I'd document that service names cannot be currently used for testing purposes and treat this as a new feature request.
Sound good to me.
comment:15 by , 3 years ago
Severity: | Release blocker → Normal |
---|---|
Summary: | Tests crash when using only a service name on PostgreSQL. → Support using service names for tests on PostgreSQL. |
Type: | Bug → New feature |
comment:18 by , 2 years ago
Was messing around with this and found the immediate issue to be that the .pop()
call in get_connection_params()
was destructive to self.settings_dict
, since assigning settings_dict = self.settings_dict
means both still reference the same underlying dictionary. This means that after _nodb_cursor
is done creating the test database, acquiring the normal cursor will fail since ['OPTIONS']['service']
has been removed from [OPTIONS]
entirely. I was able to get tests to work by removing the destructive call to .pop()
and specifying service as None
specifically in the conn_params
for this case:
-
django/db/backends/postgresql/base.py
diff --git a/django/db/backends/postgresql/base.py b/django/db/backends/postgresql/base.py index 2758c402ab..be1c05f313 100644
a b class DatabaseWrapper(BaseDatabaseWrapper): 194 194 } 195 195 elif settings_dict["NAME"] is None: 196 196 # Connect to the default 'postgres' db. 197 settings_dict.get("OPTIONS", {}).pop("service", None) 198 conn_params = {"database": "postgres", **settings_dict["OPTIONS"]} 197 conn_params = {"database": "postgres", "service": None, **settings_dict["OPTIONS"]} 199 198 else: 200 199 conn_params = {**settings_dict["OPTIONS"]} 201 200
But to Mariusz Felisiak's point:
I've tried to implement this but all mechanism related with creating/cloning test databases are based on the database name. It's feasible, however, my draft patch is quite complex and the risk of introducing regressions in a stable version is too great, IMO. I'd document that service names cannot be currently used for testing purposes and treat this as a new feature request.
There might be more to this than meets the eye, especially if their are code paths other than the instance of DatabaseWrapper
created by _nodb_cursor
that use this branch of get_connection_params()
that I am not aware of.
Looking at the pull request for the OPTIONS change, I find that I can use the service name
django_test
. This does not appear to be documented anywhere.While I wouldn't initially agree with using one login for testing all projects, use of this pg service name needs to be documented.