Report exceptions in SqliteMultithread. #28
Conversation
This is a first pass implementation, requesting feedback. Because of the asynchronous "fire and forget" nature of calling SqliteMultithread.execute(), there is no way for the calling thread to be made aware of an exception in the inner thread. We handle such exception gracefully, logging to level ERROR the inner exception, the exception only, *and* the outer stacktrace (the true offending code from the caller's thread). **This might have a slight performance impact.** For the ``select*`` family of methods, the inner exception is raised in by calling thread before returning, but for "fire and forget" calls of the ``execute*`` family of methods, such as ``db['key'] = 'value'``, we may only throw the exception: - on commit: somewhat mirroring asyncio's exceptions thrown when accessing the result of a Future. - on close or __del__, (which now awaits confirmation before returning, allowing exceptions to be caught). - at any subsequent statement after the time that the inner exception occurred. commit() is made blocking by default (awaiting confirmation), except by implied commit when using by autocommit=True.
|
Example, At this point, I don't appear to have a prompt, it returned early, and we're seeing the logging from the inner thread. I do have a prompt, no exception was raised from the main thread. I press return to see a prompt, and execute any subsequent statement, such as and here we get the pdb debugger, with the traceback of the original inner exception. We can see the original values of |
|
Oh wow. Lots of changes -- I think you've written more code than the original implementation put together :) Performance impact -- not a problem. Sqlitedict's purpose is simple API and straightforward "dict-like" compatibility, not HPC. Question: how does this PR affect backward compatibility? Are there situations (non-error, no exception) where sqlitedict will behave differently now? @jquast by the way, what do you use sqlitedict for? How did you find it? I'm adding you to project collaborators, your code is good. |
No changes, this could be a minor version bump. I worked hard towards 100% coverage to make sure of that.
I use it on http://github.com/jquast/x84/ where it stores user records, e-mail-like messages, or even high scores in tetris. I provide a "proxy" that communicates through the existing sub-process IPC event engine, https://github.com/jquast/x84/blob/master/x84/bbs/dbproxy.py#L16 ; I always said we could change sqlitedict out for something else later, but we haven't had to yet, it serves just fine. I might propose sqlitedict as an alternative for redis in https://github.com/skoczen/will -- I'm annoyed at having to install and run redis-server when i wrote a github api-based bot which did not use or require a database at all. In such cases I could care less about performance, and they only use the k/v part of redis, anyway.
I was probably googling "Python dict database" I transitioned from ZODB in the "x84" project some years ago, I have used many k/v db's like mongodb, berkely, or redis before. I wanted something along the DBProxy class I shared above, and users to not have to "Install and configure a database server first!". sqlitedict fit perfectly -- if I hadn't found it, I probably would have wrote it!
Wow, thanks! Happy to collaborate! |
|
On a separate note, I hope to address the data type issue separately and that this code path is not ever used, I may have it emit a "report a bug" kind of message. Once the data type issue is resolved this bug may not be reproducible except by mocking: It does however make TDD of #25 and #19 much easier when the test-runner doesn't lock up ! |
Report exceptions in SqliteMultithread.
|
Merging. Thanks again Jeff! I'll also merge Honza's "pickle keys" PR #26 and make a new release. I think these two warrant a major version bump. |
|
By the way @jquast , could you add a "coverage (coveralls) badge" into the README? |
|
missed your coveralls comment, will do. By the way, all of this work may be reverted in the 2.0 release. We probably should have done a "merge --squash" so that we could have done so easily! |
Closes Issue #24
This is a first pass implementation, requesting feedback.
Because of the asynchronous "fire and forget" nature of calling SqliteMultithread.execute(), there is no way for the calling thread to be made aware of an exception in the inner thread.
We handle such exception gracefully, logging to level ERROR the inner exception, the exception only, and the outer stacktrace (the true offending code from the caller's thread).
This might have a slight performance impact.
For the
select*family of methods, the inner exception is raised in by calling thread before returning, but for "fire and forget" calls of theexecute*family of methods, such asdb['key'] = 'value', we may only throw the exception:closeor__del__, (which now awaits confirmation before returning, allowing exceptions to be caught).commit()is made blocking by default (awaiting confirmation), except by implied commit when using byautocommit=True.For the patient, the work was recorded: