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 scenario where db_hash = None on close #2

Merged
merged 2 commits into from Feb 2, 2020

Conversation

@ipmb
Copy link
Contributor

ipmb commented Jan 24, 2020

If the NAME was already patched, load_remote_db would never set
self.db_hash. This caused the comparison to always fail during close
and the DB to unnecessarily be uploaded.

This also introduces more logging using the django_s3_sqlite namespace
to make it easier to turn on/off for debugging.

Some helper functions were added to make the code more succinct while
also avoiding ever reading the file into memory more than once for a
given method.

Since we already have it, the MD5 sum is pushed to S3 to ensure the
integrity of the upload.

If the NAME was already patched, `load_remote_db` would never set
self.db_hash. This caused the comparison to always fail during `close`
and the DB to unnecessarily be uploaded.

This also introduces more logging using the `django_s3_sqlite` namespace
to make it easier to turn on/off for debugging.

Some helper functions were added to make the code more succinct while
also avoiding ever reading the file into memory more than once for a
given method.

Since we already have it, the MD5 sum is pushed to S3 to ensure the
integrity of the upload.
@ipmb

This comment has been minimized.

Copy link
Contributor Author

ipmb commented Jan 24, 2020

Sorry for lumping all these changes into one PR. If you'd like any of them pulled out, just let me know.

Thanks for this library!

@ipmb

This comment has been minimized.

Copy link
Contributor Author

ipmb commented Jan 24, 2020

Makes a big difference for us on performance:

2020-01-24 at 1 47 PM

@FlipperPA FlipperPA merged commit 756ee40 into FlipperPA:master Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.