← Back to team overview

yahoo-eng-team team mailing list archive

[Bug 1263639] [NEW] glance/tests/unit/test_migrations.py does not calculate the tmp db path properly for sqlite

 

Public bug reported:

The current method of resetting temp tests databases in the TestMigrations._reset_databases() method is:
    1) use urlparse.urlparse to first parse the connection string
    2) strip the "path" component of the result of step (1) of all "/" prefixes and use the result as the path to the database (to clear the database for the test).

For a sqlite connection, this works fine for the default connection string ("sqlite:///test_migrations.db" - i.e. create the test database in the "current" directory):
    - step 1 yields: ParseResult(scheme='sqlite', netloc='', path='/test_migrations.db', params='', query='', fragment='')
    - step 2 yeids: 'test_migrations.db'

But if the test database is specified to be another directory (e.g. "/tmp/test_migrations.db") then the "/" prefix stripping leads to incorrect path calculation:
    - Note: The connection string should be: "sqlite:////tmp/test_migrations.db"
    - step 1 yields: ParseResult(scheme='sqlite', netloc='', path='//tmp/test_migrations.db', params='', query='', fragment='')
    - step 2 yields: 'tmp/test_migrations.db'
This path doesn't exist in the current directory and so the subsequent "resetting" code does nothing. But when sqlalchemy actually connects to the DB (which is in /tmp/test_migrations.db) the test finds that the DB hasn't been reset and so fails.

The fix for this should be not to strip all leading "/"s from the "path"
component of the result of step (1) but rather to just strip the first
leading "/".

** Affects: glance
     Importance: Undecided
     Assignee: David Koo (kpublicmail)
         Status: In Progress

** Changed in: glance
     Assignee: (unassigned) => David Koo (kpublicmail)

** Changed in: glance
       Status: New => In Progress

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to Glance.
https://bugs.launchpad.net/bugs/1263639

Title:
  glance/tests/unit/test_migrations.py does not calculate the tmp db
  path properly for sqlite

Status in OpenStack Image Registry and Delivery Service (Glance):
  In Progress

Bug description:
  The current method of resetting temp tests databases in the TestMigrations._reset_databases() method is:
      1) use urlparse.urlparse to first parse the connection string
      2) strip the "path" component of the result of step (1) of all "/" prefixes and use the result as the path to the database (to clear the database for the test).

  For a sqlite connection, this works fine for the default connection string ("sqlite:///test_migrations.db" - i.e. create the test database in the "current" directory):
      - step 1 yields: ParseResult(scheme='sqlite', netloc='', path='/test_migrations.db', params='', query='', fragment='')
      - step 2 yeids: 'test_migrations.db'

  But if the test database is specified to be another directory (e.g. "/tmp/test_migrations.db") then the "/" prefix stripping leads to incorrect path calculation:
      - Note: The connection string should be: "sqlite:////tmp/test_migrations.db"
      - step 1 yields: ParseResult(scheme='sqlite', netloc='', path='//tmp/test_migrations.db', params='', query='', fragment='')
      - step 2 yields: 'tmp/test_migrations.db'
  This path doesn't exist in the current directory and so the subsequent "resetting" code does nothing. But when sqlalchemy actually connects to the DB (which is in /tmp/test_migrations.db) the test finds that the DB hasn't been reset and so fails.

  The fix for this should be not to strip all leading "/"s from the
  "path" component of the result of step (1) but rather to just strip
  the first leading "/".

To manage notifications about this bug go to:
https://bugs.launchpad.net/glance/+bug/1263639/+subscriptions


Follow ups

References