Opened 7 years ago
Closed 5 years ago
#29043 closed Cleanup/optimization (wontfix)
test --keepdb says "Using existing test database" even if it's run for the first time
Reported by: | karyon | Owned by: | Hasan Ramezani |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 2.0 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The title says it, test --keepdb
says Using existing test database for alias
even if that option hasn't been used before and there is no db to reuse.
besides, in my project the full output is
Using existing test database for alias 'default'... Cache table 'evap_db_cache' already exists. System check identified no issues (0 silenced). <snip>
i think the second line is weird and provides no value, i think it can simply be removed.
Change History (8)
comment:1 by , 7 years ago
Description: | modified (diff) |
---|
comment:2 by , 7 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
comment:3 by , 5 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Owner: | changed from | to
Status: | new → assigned |
The patch needs tests. I don't know how to test it.
Any suggestion?
comment:5 by , 5 years ago
Patch needs improvement: | set |
---|
As per comments on the PR, I agree with Tim's original comment: the extra code added just isn't worth the benefit.
We catch exceptions from _execute_create_test_db()
. Maybe we could add some additional output in the if keepdb
branch there, just to clarify the existing message.
comment:6 by , 5 years ago
@Carlton Gibson, Thanks for the comment.
I checked Django database backends creation.py
and the exception in _execute_create_test_db()
just raise in base
and MySQL
backend.
In Postgres
backend, it checks the database assistance and doesn't raise an exception:
`
def _execute_create_test_db(self, cursor, parameters, keepdb=False): try: if keepdb and self._database_exists(cursor, parameters['dbname']): # If the database should be kept and it already exists, don't # try to create a new one. return
In Oracle
backend we don't have the _execute_create_test_db()
, Oracle backend has it's own _create_test_db
and doesn't call the _execute_create_test_db()
.
So, it seems your proposed solution
`
Maybe we could add some additional output in the if keepdb branch there, just to clarify the existing message.
'
will add extra code but less than my first solution.
I have another idea, we can clarify the message at the first point. here is the solution:
diff --git a/django/db/backends/base/creation.py b/django/db/backends/base/creation.py index f36d60a5fe..cdf092967a 100644 --- a/django/db/backends/base/creation.py +++ b/django/db/backends/base/creation.py @@ -45,9 +45,10 @@ class BaseDatabaseCreation: if keepdb: action = "Using existing" - self.log('%s test database for alias %s...' % ( + self.log('%s test database for alias %s%s...' % ( action, self._get_database_display_str(verbosity, test_database_name), + '(if exists)' if keepdb else '', )) # We could skip this call if keepdb is True, but we instead
What is your opinion about this?
comment:7 by , 5 years ago
Patch needs improvement: | unset |
---|
comment:8 by , 5 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
After evaluating proposed patches I have to say that it's not worth to add a complexity here.
I'm not sure that fixing this won't add more complexity than it's worth, but we could evaluate a patch.