Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deadlock when connection fails #97

Conversation

arnauorriols
Copy link

Wraps sqlite3.connect() in try/except
and stores the exception to raise it in check_raise_error()

Fixes #90

Wraps sqlite3.connect() in try/except
and stores the exception to raise it in check_raise_error()
@arnauorriols arnauorriols force-pushed the fix-deadlock-when-connection-fails branch from 04ab11b to 2e5b23a Compare June 21, 2019 00:30
@arnauorriols
Copy link
Author

There seems to be a race-condition, still deadlocking sometimes. I'll check tomorrow.

@@ -277,6 +287,6 @@ def test_tablenames(self):
self.assertEqual(SqliteDict.get_tablenames(fname), ['table1'])
with SqliteDict(fname,tablename='table2') as db2:
self.assertEqual(SqliteDict.get_tablenames(fname), ['table1','table2'])

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpenkov can we set up an automated git hook to check for these things?

Code with trailing whitespace should never have been merged.

attempt_funcs = [attempt_write,
attempt_update,
attempt_funcs = [attempt_write,
attempt_update,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hanging indent please (not vertical).

self.conn.execute(MAKE_TABLE)
self.conn.commit()
except sqlite3.OperationalError as e:
raise RuntimeError(str(e))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it better to raise RuntimeError than sqlite3.OperationalError here?

In my opinion, it's not a good idea because it hides useful information from the user.

@mpenkov mpenkov added the stale No recent activity from PR author label Jan 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale No recent activity from PR author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deadlock on exception when opening connection
3 participants