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

If there is no modified data, do not write. #54

Merged
merged 1 commit into from May 20, 2015

Conversation

2 participants
@rainwoodman
Contributor

rainwoodman commented May 15, 2015

This avoids an exception:

Exception ValueError: 'I/O operation on closed file' in <bound method CachingMiddleware.__del__ of  <tinydb.middlewares.CachingMiddleware object at 0x7f9c96610a90>> ignored

The case happens when the objects has been closed and del is invoked. Since the semantics guarantees
no writes after close, avoid storage.write when there are no changes in flush seems to be a reasonable fix.

If there is no modified data, do not write.
This avoids an exception:

    Exception ValueError: 'I/O operation on closed file' in <bound method CachingMiddleware.__del__ of  <tinydb.middlewares.CachingMiddleware object at 0x7f9c96610a90>> ignored

The case happens when the objects has been closed and __del__ is invoked. Since the semantics guarantees
no writes after close, avoid storage.write when there are no changes in flush seems to be a reasonable fix.
@msiemens

This comment has been minimized.

Show comment
Hide comment
@msiemens

msiemens May 18, 2015

Owner

Thank you for the PR! Do you have an idea how this could be tested?

Owner

msiemens commented May 18, 2015

Thank you for the PR! Do you have an idea how this could be tested?

@rainwoodman

This comment has been minimized.

Show comment
Hide comment
@rainwoodman

rainwoodman May 18, 2015

Contributor

This would reproduce it. I am not sure how to integrate into the test framework; since this is an igored exception in __del__.

from tinydb import TinyDB
from tinydb import where
from tinydb.storages import JSONStorage
from tinydb.middlewares import CachingMiddleware

class DB(TinyDB):
    def __init__(self, filename):
        TinyDB.__init__(self, filename, storage=CachingMiddleware(JSONStorage))
        self.foo = filename

def main():
    with DB("abcd.json") as db:
        for i in range(12):
            db.remove(where("key") == i)
            d = {'key':i,
                 'value':"1231",
                }
            db.insert(d)

main()
Contributor

rainwoodman commented May 18, 2015

This would reproduce it. I am not sure how to integrate into the test framework; since this is an igored exception in __del__.

from tinydb import TinyDB
from tinydb import where
from tinydb.storages import JSONStorage
from tinydb.middlewares import CachingMiddleware

class DB(TinyDB):
    def __init__(self, filename):
        TinyDB.__init__(self, filename, storage=CachingMiddleware(JSONStorage))
        self.foo = filename

def main():
    with DB("abcd.json") as db:
        for i in range(12):
            db.remove(where("key") == i)
            d = {'key':i,
                 'value':"1231",
                }
            db.insert(d)

main()
@msiemens

This comment has been minimized.

Show comment
Hide comment
@msiemens

msiemens May 20, 2015

Owner

Hmmm... The ignored exception is raised when garbage collecting the database object while the file handle has already been closed. It appears to me that on garbage collection the file handle is collected (and closed) first, followed by the storage/middleware. But this of course depends on the Python implementation and maybe even version¹, so I'll merge this PR without an accompanying test.

¹ I've tried to reliably force garbage collection of the database object, but haven't succeeded so far.

Owner

msiemens commented May 20, 2015

Hmmm... The ignored exception is raised when garbage collecting the database object while the file handle has already been closed. It appears to me that on garbage collection the file handle is collected (and closed) first, followed by the storage/middleware. But this of course depends on the Python implementation and maybe even version¹, so I'll merge this PR without an accompanying test.

¹ I've tried to reliably force garbage collection of the database object, but haven't succeeded so far.

msiemens added a commit that referenced this pull request May 20, 2015

Merge pull request #54 from rainwoodman/patch-1
CachingMiddleware: Don't write if no data has been modified

@msiemens msiemens merged commit cbcf58d into msiemens:master May 20, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@msiemens

This comment has been minimized.

Show comment
Hide comment
@msiemens

msiemens May 20, 2015

Owner

I've now released v2.3.2 which contains your changes 😃

Owner

msiemens commented May 20, 2015

I've now released v2.3.2 which contains your changes 😃

@rainwoodman rainwoodman deleted the rainwoodman:patch-1 branch May 20, 2015

@rainwoodman

This comment has been minimized.

Show comment
Hide comment
@rainwoodman

rainwoodman May 20, 2015

Contributor

Thanks!

Contributor

rainwoodman commented May 20, 2015

Thanks!

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