Skip to content

dbapi interface#3

Open
remdragon wants to merge 12 commits intodashawn888:masterfrom
remdragon:master
Open

dbapi interface#3
remdragon wants to merge 12 commits intodashawn888:masterfrom
remdragon:master

Conversation

@remdragon
Copy link

@remdragon remdragon commented Aug 11, 2017

#I found sqlite3worker extremely helpful to me, and made a number of enhancements that I wanted to share back with the project:

dbapi compatible interface that implements enough features to be a drop-in replacement for 3 different database abstraction libraries that I use. This can be used by replacing "import sqlite3" with "import sqlite3worker as sqlite3"

use Queue objects in place of uuid tokens, this simplifies several code constructs including the back-off delay while waiting for a response.

eliminate auto-commit and support commit calls directly giving user complete control of how often they want to commit.

catch exceptions and throw in caller's thread

eliminate need for "thread_running" attribute by using threading.Thread.join()

Try to protect the user from accidentally opening two different worker objects on the same database.

@dashawn888
Copy link
Owner

dashawn888 commented Aug 11, 2017 via email

renamed sqlite3_conn to _sqlite3_conn to match tests, fix bad LOGGER.debug() call,
@remdragon
Copy link
Author

remdragon commented Aug 11, 2017 via email

@remdragon
Copy link
Author

Shawn,

It looks like you have to manually merge the pull because of the merge conflicts caused by the fact that I forked from palantir instead of yours. Your auto-builder appears to be refusing to test because of the merge conflict if I'm reading that error message correctly.

@remdragon
Copy link
Author

All checks are passing. I had to change the tests because they were written to return strings in case of error and I changed the behavior to throw exceptions when errors occur. This may require changes to code that uses your library, but it will make error handling much cleaner and more pythonic.

…s so that we can have multiple instances of a Sqlite3Worker. This fixes bug with multiple dbapi connections not shutting down the thread on each other. Replaced OperationalError with ProgrammingError where it seemed more appropriate.

sqlite3worker_test.py: wrote test to reproduce an error that occurred in production when opening multiple dbapi connections to the same database from different threads
sqlite3worker_test.py: it helps when you actually commit the new tests you write
…e3.Row doesn't like our fake cursor, fix bug in Sqlite3WorkerExecuteScript, remove "no cover" pragmas that are now covered in testing, fix typo in Sqlite3Worker.commit(), dbapi interface now keeps and applies row_factory itself instead of relying on Sqlite3Worker to do it - this fixes bug where different connections have different row factories

sqlite3worker_test.py: add a bunch more tests to get code coverage to 100% and prevent regression of bugs fixed in sqlite3worker.py
@remdragon
Copy link
Author

It took longer than I had expected, but the version of the code I just committed is now in production ( for the last 24 hours ) as a drop-in replacement for sqlite3 using the dbapi interface in a multi-threaded application that was experiencing database locked errors. I will update here if I run into any further issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants