Code review comment for lp:~stefanor/ibid/sqlalchemy-370772

> Can you just change the log message so something like "Exception occured while
> committing session after %s processor of %s plugin".

Agreed. I sort of skipped over that. r628

> Do all versions of SQLAlchemy default to using transactions and autoflush?

All supported versions. And not likely to change.

> The account ID won't be available if autoflush isn't on.

No, autoflush doesn't help, but yes we do need a manual flush. r629
Although logging factoid IDs is of arguable value.

> There's a problem we didn't think of, which is that processors add responses
> before knowing that their database changes succeeded. So the bot will still
> respond positively if the commit() by the dispatcher failed. Maybe we do need
> to commit() in the plugin itself.

Yes, that's certainly an issue. We even have the option of re-running failed
processors now (although we'd need to differentiate which are safe to re-run,
etc.)

> I'd prefer the except block in Identify.identify() to only catch
> IntegrityErrors.

Agreed. r631

The reason for that is SQLite can cause an OperationalError to be thrown if
the database is locked. The correct behaviour in that case is probably to sleep
and then try and commit again. (at least at the pysqlite level, that'd be the
right thing)