Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#17786 closed Bug (fixed)

Exception when running the tests under Oracle

Reported by: Aymeric Augustin Owned by: Aymeric Augustin
Component: Testing framework Version: 1.4-beta-1
Severity: Release blocker Keywords:
Cc: anssi.kaariainen@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Destroying test database for alias 'default'...
Traceback (most recent call last):
  File "runtests.py", line 323, in <module>
    options.failfast, args)
  File "runtests.py", line 166, in django_tests
    failures = test_runner.run_tests(test_labels, extra_tests=extra_tests)
  File "/var/lib/jenkins/jobs/Django + Oracle/workspace/django/test/simple.py", line 383, in run_tests
    self.teardown_databases(old_config)
  File "/var/lib/jenkins/jobs/Django + Oracle/workspace/django/test/simple.py", line 351, in teardown_databases
    connection.creation.destroy_test_db(old_name, self.verbosity)
  File "/var/lib/jenkins/jobs/Django + Oracle/workspace/django/db/backends/creation.py", line 378, in destroy_test_db
    new_connection.creation._destroy_test_db(test_database_name, verbosity)
  File "/var/lib/jenkins/jobs/Django + Oracle/workspace/django/db/backends/oracle/creation.py", line 126, in _destroy_test_db
    self.connection.settings_dict["USER"] = self.remember['user']
KeyError: 'user'

This could be to be a consequence of r17411.

Attachments (1)

17786-second-try.patch (2.2 KB ) - added by Aymeric Augustin 12 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by Anssi Kääriäinen, 12 years ago

Cc: anssi.kaariainen@… added

This is almost definitely connected to r17411. That patch caused the drop database code to use a new connection for dropping the database. The Oracle code expects the dropping to be done by the same connection.

The right fix seems to be returning the "remember" dictionary from the _create_test_db(), and then again passing it to _destroy_test_db(). Or, you could get hacky and have a class level remember variable in the creation code, mapping connection alias to remember dictionary of that alias. It should work but as said, it is a little ugly... Even uglier is just assigning the creation from the old connection to the new temporary connection.

comment:2 by Aymeric Augustin, 12 years ago

Owner: changed from nobody to Aymeric Augustin
Triage Stage: UnreviewedAccepted

Yes, the problem has several solutions, it's mostly a matter of choosing the less ugly :)

Assigning to myself since I have an Oracle setup where I can test this.

comment:3 by Aymeric Augustin, 12 years ago

Resolution: fixed
Status: newclosed

In [17599]:

Fixed #17786 -- Exception when running the tests under Oracle, during the destruction of the test database.

comment:4 by Anssi Kääriäinen, 12 years ago

Resolution: fixed
Status: closedreopened

I don't think the patch is correct. You can have multiple Oracle connections with different username and password. So, the dictionary must be:
alias -> (username, password). Otherwise you will use the same username and password for all the connections, which can be incorrect.

by Aymeric Augustin, 12 years ago

Attachment: 17786-second-try.patch added

comment:5 by Aymeric Augustin, 12 years ago

Good catch. I think the patch I just attached is cleaner and works with multiple databases too. I'd appreciate a review before I commit something stupid again...

comment:6 by Anssi Kääriäinen, 12 years ago

That looks good to me.

There is an assumption the settings_dict is shared between connections. So, I think it is safe to store per-alias data there. It might be worth it to put a small comment about this assumption in django.db.backends.BaseDatabaseWrapper.__init__(). I think the assumption has been there long before this patch, but still a small comment can't hurt anybody. This is just nitpicking, not any must-fix issue.

Note that I haven't actually tested anything...

comment:7 by Aymeric Augustin, 12 years ago

I've run the multiple_databases tests under Oracle and everything seemed to behave properly.

comment:8 by Aymeric Augustin, 12 years ago

Resolution: fixed
Status: reopenedclosed

In [17601]:

Fixed #17786 (again) -- Ensured destruction of test databases works under Oracle, even with multiple databases, after r17411.

comment:9 by Anssi Kääriäinen, 12 years ago

Now I know what bothers me about the self.settings_dict. From the code you get a feeling the settings_dict is instance level, or class level at most. However, you are really modifying the settings.py DATABASES['alias'] dictionary. That makes it pretty easy to do accidental global changes if you start changing the self.settings_dict.

Anyhow, the above isn't this ticket's problem. And I don't know if "fixing" the above is worth more than a comment anyways.

I often hope the django.db.backends and especially the django.db.models.sql would have good README files explaining what is happening and why. I guess these would get accepted assuming somebody were to write the files...

Note: See TracTickets for help on using tickets.
Back to Top