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 exit error TypeError("'NoneType' object is not callable",) using catch try block in close method #45

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pip-log.txt
# Unit test / coverage reports
.coverage
.tox
.cache

# sqlite databases
*.sqlite
Expand Down
22 changes: 20 additions & 2 deletions sqlitedict.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def __exit__(self, *exc_info):
self.close()

def __str__(self):
return "SqliteDict(%s)" % (self.conn.filename)
return "SqliteDict(%s)" % (self.filename)

def __repr__(self):
return str(self) # no need of something complex
Expand Down Expand Up @@ -233,12 +233,18 @@ def __setitem__(self, key, value):
self.conn.execute(ADD_ITEM, (key, encode(value)))

def __delitem__(self, key):
if self.flag == 'r':
raise RuntimeError('Refusing to delete from read-only SqliteDict')

if key not in self:
raise KeyError(key)
DEL_ITEM = 'DELETE FROM %s WHERE key = ?' % self.tablename
self.conn.execute(DEL_ITEM, (key,))

def update(self, items=(), **kwds):
if self.flag == 'r':
raise RuntimeError('Refusing to update read-only SqliteDict')

try:
items = [(k, encode(v)) for k, v in items.items()]
except AttributeError:
Expand All @@ -253,6 +259,9 @@ def __iter__(self):
return self.iterkeys()

def clear(self):
if self.flag == 'r':
raise RuntimeError('Refusing to clear read-only SqliteDict')

CLEAR_ALL = 'DELETE FROM %s;' % self.tablename # avoid VACUUM, as it gives "OperationalError: database schema has changed"
self.conn.commit()
self.conn.execute(CLEAR_ALL)
Expand Down Expand Up @@ -289,6 +298,9 @@ def close(self, do_log=True):

def terminate(self):
"""Delete the underlying database file. Use with care."""
if self.flag == 'r':
raise RuntimeError('Refusing to terminate read-only SqliteDict')

self.close()

if self.filename == ':memory:':
Expand All @@ -303,7 +315,13 @@ def terminate(self):

def __del__(self):
# like close(), but assume globals are gone by now (do not log!)
self.close(do_log=False)
try:
self.close(do_log=False)
except Exception:
# prevent error log flood in case of multiple SqliteDicts
# closed after connection lost (exceptions are always ignored
# in __del__ method.
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we print the exception to sys.stderr?
As in del docs

exceptions that occur during their execution are ignored, and a warning is printed to sys.stderr instead.

Copy link
Owner

Choose a reason for hiding this comment

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

No, getting rid of the flood of logged exceptions is the entire reason for this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we at least print a warning instead of an exception?


# Adding extra methods for python 2 compatibility (at import time)
if major_version == 2:
Expand Down
28 changes: 26 additions & 2 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ def test_as_str(self):
db = sqlitedict.SqliteDict()
# exercise
db.__str__()
# test when db closed
db.close()
db.__str__()

def test_as_repr(self):
"""Verify SqliteDict.__repr__()."""
Expand Down Expand Up @@ -94,17 +97,38 @@ def test_readonly(self):
fname = norm_file('tests/db/sqlitedict-override-test.sqlite')
orig_db = sqlitedict.SqliteDict(filename=fname)
orig_db['key'] = 'value'
orig_db['key_two'] = 2
orig_db.commit()
orig_db.close()

readonly_db = sqlitedict.SqliteDict(filename=fname, flag = 'r')
self.assertTrue(readonly_db['key'] == 'value')
self.assertTrue(readonly_db['key_two'] == 2)

def attempt_write():
readonly_db['key'] = ['new_value']

with self.assertRaises(RuntimeError):
attempt_write()
def attempt_update():
readonly_db.update(key = 'value2', key_two = 2.1)

def attempt_delete():
del readonly_db['key']

def attempt_clear():
readonly_db.clear()

def attempt_terminate():
readonly_db.terminate()

attempt_funcs = [attempt_write,
attempt_update,
attempt_delete,
attempt_clear,
attempt_terminate]

for func in attempt_funcs:
with self.assertRaises(RuntimeError):
func()

def test_overwrite_using_flag_w(self):
"""Re-opening of a database with flag='w' destroys only the target table."""
Expand Down