wmjie / ibm-db

Automatically exported from code.google.com/p/ibm-db
0 stars 0 forks source link

django_adapter: host/port connection options are obligatory #48

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
by default most of django database attributes have default values "" (see
django.conf.global_settings). 
Logic done in ibm_db_django.base is like this: 

             if (isinstance(database_user, types.StringType) or 
               isinstance(database_user, types.UnicodeType)):
                kwargs['user'] = database_user

this mean that if you don't specify in your settings user at all kwargs[]
will hold "user" with "" value.

This is especially odd for host and port options, since this term: 

if ((isinstance(database_port, types.StringType) or
                isinstance(database_port, types.UnicodeType)) and 
                (isinstance(database_host, types.StringType) or
                isinstance(database_host, types.UnicodeType))): 

will allways be executed - what makes these two params required. 

Workaround - one can define HOST/PORT vars = None.

Proposed solution: 
            if (isinstance(database_pass, types.StringType) or
               isinstance(database_pass, types.UnicodeType)):
-> 
            if database_pass:

for port/host:

            if ((isinstance(database_port, types.StringType) or
                isinstance(database_port, types.UnicodeType)) and 
                (isinstance(database_host, types.StringType) or
                isinstance(database_host, types.UnicodeType))):

->
            if database_port and host:

----------------

btw. simplified version of isinstance:
            if (isinstance(database_host, types.StringType) or
               isinstance(database_host, types.UnicodeType)):
->
            if isinstance(database_host, basestring):

no need for "import types".

Original issue reported on code.google.com by trebor74hr@gmail.com on 29 Mar 2010 at 11:41

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
>>>
if ((isinstance(database_port, types.StringType) or
                isinstance(database_port, types.UnicodeType)) and 
                (isinstance(database_host, types.StringType) or
                isinstance(database_host, types.UnicodeType))): 
    ......
<<<

This will only execute when both database_port and database_host must be of
type basestring.

we are doing this check not only to avoid None value, this check is for to 
avoid any
unexpected input (like True, False and other object).

your second alternative( isinstance(database_host, basestring) ) is really 
good. we
wouldn't gain any performance due to this, but it will make code more readable 
and
simple.

I will use your second alternative in our next release.

Original comment by rahul.pr...@in.ibm.com on 30 Mar 2010 at 11:42

GoogleCodeExporter commented 9 years ago
For this django settings:

    DATABASE_ENGINE = 'ibm_db_django'
    DATABASE_NAME = 'sample'           
    DATABASE_USER = 'user'             
    DATABASE_PASSWORD = 'password'     

The database sample is registred on my machine and is located on another 
machine.
I'll get error that there is no such database on localhost. This is because 
django
default values (global_settings.py):
    DATABASE_HOST = ''             # Set to empty string for localhost. Not used with
sqlite3.
    DATABASE_PORT = ''             # Set to empty string for default. Not used with
sqlite3.

which will trigger "if" term with host/port that I reported. I think this is not
correct. 

In order to avoid this we defined settings like this:

   DATABASE_ENGINE = 'ibm_db_django'
   DATABASE_NAME = 'sample'           
   DATABASE_USER = 'user'             
   DATABASE_PASSWORD = 'password'     
   DATABASE_HOST = None # workaround!!!

If you need to check values than I would suggest this: 

   if database_port and database_host:
      assert isinstance(database_host, basestring), "database_host should be string type"
      assert isinstance(database_port, basestring), "database_port should be string type"

Original comment by trebor74hr@gmail.com on 1 Apr 2010 at 2:39

GoogleCodeExporter commented 9 years ago
Fixed in ibm_db-1.0.2

Original comment by rahul.pr...@in.ibm.com on 29 Apr 2010 at 12:33