tags:

views:

438

answers:

3

The function code:

# Connect to the DB
try:
    dbi = MySQLdb.connect(host='localhost', \
                          user='user', \
                          passwd='pass', \
                          db='dbname', \
                          port=3309)

    print "Connected to DB ..."

except MySQLdb.Error, e:
    apiErr = 2
    apiErrMsg = "Error %d: %s" % (e.args[0], e.args[1])
    return

    # To prevent try..finally bug in python2.4,
    # one has to nest the "try: except:" part. 
try:
    try:
        sql = dbi.cursor()
        sql.execute("""
        SELECT *
        FROM table
        WHERE idClient =  %s
        """, (key, ))

        access = sql.fetchall()

        # [some more code here]           

    except MySQLdb.Error, e:
        apiErr = 2
        apiErrMsg = "Error %d: %s" % (e.args[0], e.args[1])
        return

finally:
    sql.close()
    dbi.close()

I understand that in a try .. except .. finally, the finally block will always execute. In the above code, I don't want the finally in the second try block to execute if there is an exception in the first try block. What am I doing wrong?

(Note: Using python 2.4)

Clarification: I am not aware if MySQLdb closes connections automatically when an error occurs. The problem I am facing with the above code is, when there is an error in establishing a connection (the first try block of the code), calling dbi.close() in the finally block raises "AttributeError: 'NoneType' object has no attribute 'close'" with reference to dbi ...

Solution: This worked as desired -

# define at the start 
dbi = None
sql = None

In the finally block,

if sql is not None:
    sql.close()
if dbi is not None:
    dbi.close()

Thanks to those who replied. I learned something new from all of you. (I'll try to phrase my questions more clearly the next time :).

+4  A: 

Don't use finally. If you don't want the code to always be executed then you should find another flow control structure that meets your needs.

One way to accomplish this behavior is to move the statements in your 'finally' block to the bottom of your 'try' block. That way they won't get executed when an exception is thrown but will get executed, after all other statements, otherwise.

EDIT:

After further discussion it appears that in your case you do actually want to use 'finally'. What I recommend is checking to see if your connection has already been closed before you attempt to close it.

Waylon Flinn
But then what happens when an error happens? Does MySQLdb close the mysql connections automatically if the programmer hasn't done it explicitly?
@Sam It won't but then what's the problem? It sounds as though you don't want to close the connection when an exception occurs as that's the only code in your finally block.
Waylon Flinn
I do want the connections to close (1) after the query has been executed (2) in case there was some unknown error.
@Sam take a look at dbr's suggestion of the ':else' statement. duffymo is also giving good advice.
Waylon Flinn
How do I check if the connection has already been closed?
Check this page for some documentation of the object: http://mysql-python.sourceforge.net/MySQLdb.html
Waylon Flinn
+5  A: 

I think in this case you do want to use finally, because you want to close those connections.

I disagree with the notion that you should have two try blocks in the same method.

I think the flaw in the design is acquiring the connection and performing the query in the same method. I would recommend separating the two. A service class or method knows about the unit of work. It should acquire the connection, pass it to another class that performs the query, and closes the connection when it's done. That way the query method can throw any exception it encounters and leave the cleanup to the class or method that's responsible for the connection.

duffymo
Yeah, makes sense.
+5  A: 

Use else: instead of finally:. See the Exception Handling part of the docs:

The try ... except statement has an optional else clause, which, when present, must follow all except clauses. It is useful for code that must be executed if the try clause does not raise an exception.

for arg in sys.argv[1:]:
    try:
        f = open(arg, 'r')
    except IOError:
        print 'cannot open', arg
    else:
        print arg, 'has', len(f.readlines()), 'lines'
        f.close()

..basically:

try:
    [code that might error]
except IOError:
    [This code is only ran when IOError is raised]
else:
    [This code is only ran when NO exception is raised]
finally:
    [This code is always run, both if an exception is raised or not]
dbr