[Django] #29043: test --keepdb says "Using existing test database" even if it's run for the first time
Django
first time
-----------------------------------------+------------------------
Reporter: karyon | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 2.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
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).
...
```
i think the second line is weird and provides no value, i think it can
simply be removed.
--
Ticket URL: <https://code.djangoproject.com/ticket/29043>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
first time
Reporter: karyon | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 2.0
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by karyon:
Old description:
> 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).
> ...
> ```
>
> i think the second line is weird and provides no value, i think it can
> simply be removed.
New description:
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29043#comment:1>
Django
first time
Reporter: karyon | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* type: Uncategorized => Cleanup/optimization
* component: Uncategorized => Database layer (models, ORM)
* stage: Unreviewed => Accepted
Comment:
I'm not sure that fixing this won't add more complexity than it's worth,
but we could evaluate a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/29043#comment:2>
Django
first time
Reporter: karyon | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Hasan Ramezani):
* status: new => assigned
* owner: nobody => Hasan Ramezani
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
The patch needs tests. I don't know how to test it.
Any suggestion?
--
Ticket URL: <https://code.djangoproject.com/ticket/29043#comment:3>
Django
first time
Reporter: karyon | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Hasan Ramezani):
* needs_tests: 1 => 0
Comment:
Tests added
--
Ticket URL: <https://code.djangoproject.com/ticket/29043#comment:4>
Django
first time
Reporter: karyon | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
Comment:
As per comments on the [https://github.com/django/django/pull/11437 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29043#comment:5>
Django
first time
Reporter: karyon | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Hasan Ramezani):
@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?
--
Ticket URL: <https://code.djangoproject.com/ticket/29043#comment:6>
Django
first time
Reporter: karyon | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Hasan Ramezani):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/29043#comment:7>
Django
first time
Reporter: karyon | Owner: Hasan
Type: | Ramezani
Component: Database layer | Version: 2.0
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* status: assigned => closed
* resolution: => wontfix
Comment:
After evaluating proposed patches I have to say that it's not worth to add
a complexity here.
--
Ticket URL: <https://code.djangoproject.com/ticket/29043#comment:8>