Not all db.sqal.session methods are wrapped by wrap_db_error

[Boris Pavlovic]

first(), all(), begin(), commit() and other public methods could produce amount of exceptions, that should be wrapped in exception in any case.

[Mark McLoughlin (markmc)]

Was there a patch posted for this?

Is it still an issue?

[Mike Bayer (zzzeek)]

part of what I'm looking into is to support oslo.db being able to intercept and accommodate DB-level exceptions at the Core level, rather than the ORM level. Wrapping ORM methods means the exceptions are being intercepted at any number of endpoints, when in fact the exceptions are generated up the tree at a much smaller number of locations. SQLAlchemy has a standard Core-level exception handling routine, so being able to take advantage of this means that exception-rewriting can in fact occur in exactly one place, and it will be hit for all DBAPI interactions that raise an exception.

Right now the exception-handling routine has a pluggable event hook in it, which allows inspection of the raw DBAPI exception. Theoretically it can be used to re-raise a new exception, though the hook should be modified slightly to support this fully. To use the hook as public API would therefore require SQLAlchemy 0.9 and probably an unreleased 0.9.X version. The same general methodology is possible with any 0.9.x, 0.8.x or 0.7.x version using a "wrapping" approach, that would require use of two underscored attributes, wrapping the _handle_dbapi_exception() method, which would make use of Engine._connection_cls in order to invoke. if SQLAlchemy releases a "blessed" API hook, oslo.db could use version detection to safely choose between blessed API vs. wrapping-on-known-SQLAlchemy-version.

Regardless of the timeframe / version requirements for this to happen, it's my recommendation that ultimately, DBAPI exception rewriting occur at the Core level using events which would eliminate the need for endpoint level exception catching. I'll be putting down my recommendations soon at some more well-defined place (like a wiki page).

[Ben Nemec (bnemec)]

Good stuff. :-)

I think a spec would be the best place to capture the details of this work now: https://github.com/openstack/oslo-specs/blob/master/specs/template.rst We've started using those so everyone has a reasonable way to view and comment on new feature work before it gets approved.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/105891
Committed: https://git.openstack.org/cgit/openstack/oslo.db/commit/?id=0a6c8a8a0eb8a2034758488bb4897c4c9d2a964a
Submitter: Jenkins
Branch: master

commit 0a6c8a8a0eb8a2034758488bb4897c4c9d2a964a
Author: Mike Bayer <email address hidden>
Date: Wed Jul 9 17:44:08 2014 -0400

    Implement new exception interception and filtering layer

    Replace @_wrap_db_error() with an event-based system
    that leverages the new SQLAlchemy handle_error() event.

    The internal logic of @_wrap_db_error() is replaced by a
    system of declarative filters, which is also expressed
    on the testing side using a new test framework for these
    conditions.

    partially implement bp: use-events-for-error-wrapping
    Closes-Bug: #1214341
    Change-Id: Ib8c7f8d5c6f44c49a53dd6411adbf8aaaecb74be

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/108935

[Viktor Serhieiev (vsergeyev)]

This bug also affects Keystone and Ironic, because these project checks for the sqlalchemy.exc.IntegrityError exception (instead of oslo.db.exception.DBDuplicateEntry), when they checks unit constrains in migration testing.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/108943
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=82de9e34d0cf7343aae77c78b0b645ac1765ac35
Submitter: Jenkins
Branch: master

commit 82de9e34d0cf7343aae77c78b0b645ac1765ac35
Author: Victor Sergeyev <email address hidden>
Date: Wed Jul 23 13:15:23 2014 +0300

    Catch oslo.db error instead of sqlalchemy error

    The latest oslo.db code wraps all sqlalchemy exceptions and re-raise
    them as oslo.db exceptions. But there is a single case in Ironic when
    we suppose to get sqlalchemy exception, so this will break unittest,
    when the new version of oslo.db will come.

    Added check for the new oslo.db exception, left the old check for the
    compatibility.

    Closes-Bug: #1214341
    Change-Id: Iae5599948d29778ac416edf31d3b1b2f7a8a54a9

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/108935
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=9231482360aa7765d7b4f0e887533aceaff59e53
Submitter: Jenkins
Branch: master

commit 9231482360aa7765d7b4f0e887533aceaff59e53
Author: Ilya Pekelny <email address hidden>
Date: Wed Jul 23 12:32:48 2014 +0300

    Catch correct oslo.db exception

    Now oslo.db wraps all the database exceptions into specific oslo.db
    exceptions. A test expects for IntegrityError where now returns
    DBDuplicateEntry. We should catch DBDuplicateEntry to keep test
    correctness.

    Change-Id: I4576f19c4fd3db6742a6a2fbbca0e0e5400b792d
    Closes-Bug: #1214341

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/111760

[Sean McGinnis (sean-mcginnis)]

Automatically unassigning due to inactivity.