Code

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#10716 closed (fixed)

Syntax error and bug in db/backends/oracle/creation.py

Reported by: canarix Owned by: nobody
Component: Database layer (models, ORM) Version: 1.0
Severity: Keywords: oracle
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Playing with the django tests on an Oracle DB, I've found 2 bugs in the db/backends/oracle/creation.py file :

  • line 110, the line settings.DATABASE_NAME = TEST_DATABASE_NAME is missing. Without this line, Django could use the production DB instead of the test one (can be quite annoying).

This mod also impacts the mods on some other lines because of the "remember" dict.

  • line 247, there's 2 typos : it should be "def _test_database_user(self, settings):" instead of "def _test_database_user(self, ettings):" and DATABASE_USER instead of DATABASE_NAME

I saw the problem in the 1.0.2 version, but the SVN has the same errors.

Julien

I attached a patch for the 1.0.2 version

Attachments (3)

creation.py.102.patch (1.6 KB) - added by canarix 5 years ago.
Patch for 1.0.2
creation.py.SVN.patch (1.9 KB) - added by canarix 5 years ago.
Patch for the SVN version
10716-refix.diff (1.8 KB) - added by kmtracey 5 years ago.

Download all attachments as: .zip

Change History (11)

Changed 5 years ago by canarix

Patch for 1.0.2

Changed 5 years ago by canarix

Patch for the SVN version

comment:1 Changed 5 years ago by mtredinnick

  • milestone set to 1.1
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 5 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [10547]) Fixed #10716 -- Fixed a couple of typos in Oracle testing setup.

Thanks, canarix.

comment:3 Changed 5 years ago by mtredinnick

(In [10548]) [1.0.X] Fixed #10716 -- Fixed a couple of typos in Oracle testing setup.

Thanks, canarix.

Backport of r10547 from trunk.

comment:4 Changed 5 years ago by mboersma

  • Resolution fixed deleted
  • Status changed from closed to reopened

This "fix" actually prevents the test suite from running at all on Oracle here, and for Justin Bronn as well, so we're unable to know which tests are succeeding or failing any longer.

At this late date, this should be reverted.

Changed 5 years ago by kmtracey

comment:5 Changed 5 years ago by Alex

Why are we assigning to settings.DATABASE_* here, that *should* not be occuring. I understand that settings.TEST_* is still handled globally, but that shouldn't be the case for DATABASE_*.

comment:6 Changed 5 years ago by kmtracey

Added a patch that makes it possible to run tests on Oracle again.

First, the patch reverts the parts of the original fix that recorded TEST_DATABASE_NAME in settings.DATABASE_NAME. I really don't have much a clue when it comes to Oracle, but it appears that this change caused a problem because Oracle test database creation doesn't actually create a database, it creates a tablespace within the DATABASE_NAME. If you then overwrite DATABASE_NAME with the test database (tablespace for Oracle) name in settings, when running the tests an attempt is made to connect to the test tablespace as a database, but it's not a database, so the connect attempt for running the tests fails.

It's all a bit unclear to me but it appears things are set up so that by use of the test tablespace and a test user that uses that tablespace by default, all of the test table creation, etc. is isolated from the production DB. canarix, do you experience the test data actually interfering with the production DB? Or did the code just look wrong?

The fix to the ettings instead of settings typo also causes a problem as given the way the code is written test_ is being prepended to the database user name both when it is created and when it is destroyed. So given a database user of system, the code will create user test_system then attempt to destroy test_test_system at the end. Recording the test user name as settings.TEST_USER_NAME avoids that problem so I opted for that instead of reverting what seems to be a clear typo in the original code. But someone with more of a clue on Oracle should probably take a look at this.

comment:7 Changed 5 years ago by mboersma

  • Resolution set to fixed
  • Status changed from reopened to closed

Karen's new patch looks sane enough to me, and does work. I made similar changes to the 1.0.X code and checked this in to both.

comment:8 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.