Don't catch exception within session

Bug #1262065 reported by ChangBo Guo(gcb)
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
oslo-incubator
Fix Released
Medium
Ben Nemec

Bug Description

In code review, some projects are using try/except within the context
managed by session. That's wrong, need add the note in doc string,
hope others put try/except in right place.

Error will be raised in session's __exit__ handler, and any try/except
within the context will drop the ROLLBACK, need catch the error out of
the context.

Solution:

Need catch exeption out of session , except developer want to recover from failure and continue commit part of stuff of one session.

Revision history for this message
ChangBo Guo(gcb) (glongwave) wrote :

[root@guochbo-10-7-0-151 test]# cat ./sqlalchemy_test.py
#!/usr/bin/env python
# This is a sqlalchemy sample code
# author: guochbo ChangBo Guo

import sqlalchemy;
from sqlalchemy import create_engine
from sqlalchemy import Table, Column, Integer, String
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import sessionmaker
from sqlalchemy.exc import IntegrityError

Base = declarative_base()

# Create engine
#connect='mysql://nova:nova@localhost/nova?charset=utf8'
connect='sqlite:///:memory:'
engine = create_engine(connect,echo=True)
Session = sessionmaker(bind=engine)
session = Session()

# Create one table
class User(Base):
    __tablename__ = 'users'

    id = Column(Integer, primary_key=True)
    name = Column(String(36))

    def __init__(self, id, name):
        self.id = id
        self.name = name

Base.metadata.create_all(engine)

user1= User(1, 'ed1')
user2 = User(1, 'ed2')
# duplicated entry will rollback
try:
    with session.begin(subtransactions=True):
        try:
            session.add(user1)
            session.add(user2)
        except Exception :
            print('get exception1 in with block')
except IntegrityError:
    print('get IntegrityError2 out of with block')

# catch exception in contex won't rollback
session1= Session()
try:
    with session1.begin(subtransactions=True):
        try:
            session1.add(user1)
            dict1={}
            dict1.pop('test')
            session1.add(user2)
        except Exception :
            print('get exception3 in with block')
except IntegrityError:
    print('get IntegrityError4 out of with block')

[root@guochbo-10-7-0-151 test]# ./sqlalchemy_test.py
2013-12-17 23:52:06,547 INFO sqlalchemy.engine.base.Engine PRAGMA table_info("users")
2013-12-17 23:52:06,548 INFO sqlalchemy.engine.base.Engine ()
2013-12-17 23:52:06,548 INFO sqlalchemy.engine.base.Engine
CREATE TABLE users (
        id INTEGER NOT NULL,
        name VARCHAR(36),
        PRIMARY KEY (id)
)

2013-12-17 23:52:06,548 INFO sqlalchemy.engine.base.Engine ()
2013-12-17 23:52:06,549 INFO sqlalchemy.engine.base.Engine COMMIT
2013-12-17 23:52:06,550 INFO sqlalchemy.engine.base.Engine BEGIN (implicit)
2013-12-17 23:52:06,550 INFO sqlalchemy.engine.base.Engine INSERT INTO users (id, name) VALUES (?, ?)
2013-12-17 23:52:06,550 INFO sqlalchemy.engine.base.Engine ((1, 'ed1'), (1, 'ed2'))
2013-12-17 23:52:06,551 INFO sqlalchemy.engine.base.Engine ROLLBACK
get IntegrityError2 out of with block
get exception3 in with block
2013-12-17 23:52:06,551 INFO sqlalchemy.engine.base.Engine BEGIN (implicit)
2013-12-17 23:52:06,552 INFO sqlalchemy.engine.base.Engine INSERT INTO users (id, name) VALUES (?, ?)
2013-12-17 23:52:06,552 INFO sqlalchemy.engine.base.Engine (1, 'ed1')

Revision history for this message
ChangBo Guo(gcb) (glongwave) wrote :

Last commet is an example, shows incorrect method won't rollback the commit.

Changed in oslo:
assignee: nobody → ChangBo Guo (guochbo)
status: New → In Progress
Changed in oslo:
assignee: ChangBo Guo (guochbo) → Ben Nemec (bnemec)
Ben Nemec (bnemec)
Changed in oslo:
importance: Undecided → Medium
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to oslo-incubator (master)

Reviewed: https://review.openstack.org/60144
Committed: https://git.openstack.org/cgit/openstack/oslo-incubator/commit/?id=9bc593e8af0b6c47383d97463de5b0980b78d106
Submitter: Jenkins
Branch: master

commit 9bc593e8af0b6c47383d97463de5b0980b78d106
Author: Chang Bo Guo <email address hidden>
Date: Wed Dec 4 19:05:37 2013 -0800

    Add docstring for exception handlers of session

    In code review, some projects are using try/except within the
    context managed by session. That's wrong, add the note in
    docstring, hope others put try/except in right place.

    Database Errors like IntegrityError will be raised in session's
    __exit__ handler, and any try/except within the context managed
    by session will not be triggered. And catching other non-database
    errors in the session will not trigger the ROLLBACK, so exception
    handlers should always be outside the session, except developer
    want to commit parts of stuff on purpose.

    Closes-Bug: #1262065

    Change-Id: I23223fbe9b6d9fa8e92e59894fb3b1fa195be159

Changed in oslo:
status: In Progress → Fix Committed
Changed in oslo:
milestone: none → icehouse-2
Thierry Carrez (ttx)
Changed in oslo:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in oslo:
milestone: icehouse-2 → 2014.1
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.