-
-
Notifications
You must be signed in to change notification settings - Fork 371
Add SQLiteStore #368
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
Merged
jakirkham
merged 71 commits into
zarr-developers:master
from
jakirkham:add_sqlite_store
Jan 22, 2019
Merged
Add SQLiteStore #368
Changes from all commits
Commits
Show all changes
71 commits
Select commit
Hold shift + click to select a range
8ef5dc1
Create an SQLite-backed mutable mapping
jakirkham e3e2c2e
Test SQLiteStore
jakirkham d60aaab
Export `SQLiteStore` to the top-level namespace
jakirkham ace251c
Include some SQLiteStore examples
jakirkham ecf18f7
Demonstrate the `SQLiteStore` in the tutorial
jakirkham 92a4d71
Provide API documentation for `SQLiteStore`
jakirkham efa9ccd
Make a release note for `SQLiteStore`
jakirkham 6f68451
Use unique extension for `SQLiteStore` files
jakirkham 9bbbde6
Only close SQLite database when requested
jakirkham 20ef384
Update docs to show how to close `SQLiteStore`
jakirkham a8f31cf
Ensure all SQL commands are capitalized
jakirkham 5ddc193
Simplify `SQLiteStore`'s `__delitem__` using `in`
jakirkham 0abcc1a
Drop no longer needed flake8 error suppression
jakirkham b339c09
Simplify `close` and use `flush`
jakirkham 8b8d289
Flush before pickling `SQLiteStore`
jakirkham 1cac5eb
Special case in-memory SQLite database
jakirkham b8e2d23
Drop unneeded empty `return` statement
jakirkham 4db7e14
Update docs/release.rst
alimanfoo 31a9af3
Update docs/release.rst
alimanfoo 9f5d02b
Correct default value for `check_same_thread`
jakirkham ac6827e
Flush after making any mutation to the database
jakirkham 8b35eb8
Skip flushing data when pickling `SQLiteStore`
jakirkham f8d3f03
Skip using `flush` in `close`
jakirkham 1abeba7
Implement `update` for `SQLiteStore`
jakirkham 4bbbeba
Rewrite `__setitem__` to use `update`
jakirkham f9481b8
Disable `check_same_thread` by default again
jakirkham 0188a60
Force some parameters to defaults
jakirkham 1af4446
Drop `flush` calls from `SQLiteStore`
jakirkham ca6b8a4
Drop the `flush` function from `SQLiteStore`
jakirkham eb4564b
Implement optimized `clear` for `SQLiteStore`
jakirkham 4f59451
Implement optimized `rmdir` for `SQLiteStore`
jakirkham 5ab54c0
Implement optimized `getsize` for `SQLiteStore`
jakirkham c386c72
Implement optimized `listdir` for `SQLiteStore`
jakirkham f9dfc06
Implement `rename` for `SQLiteStore`
jakirkham 8d4a8e2
Allow users to specify the SQLite table name
jakirkham 349a885
Randomize temporary table name
jakirkham 152ed0c
Merge `SELECT`s in `rename`
jakirkham 82e6522
Tidy `rename` SQL code a bit
jakirkham 3748982
Fuse away one `SELECT` in `listdir`
jakirkham 168843c
Only use `k` in `SQLiteStore`'s `__contains__`
jakirkham 8defc15
Fuse `SELECT`s in `SQLiteStore`'s `__contains__`
jakirkham 019b3e0
Cast `has` to `bool` in `SQLiteStore.__contains__`
jakirkham 3f74f25
Prefer using single quotes in more places
jakirkham dcb808f
Wrap SQL table creation text
jakirkham 9369862
Adjust wrapping of `SQLiteStore.clear`'s code
jakirkham b1c644a
Use parameters for SQL in `listdir`
jakirkham 1c3a4d7
Use parameters for SQL in `getsize`
jakirkham 059ec45
Use parameters for SQL in `rmdir`
jakirkham 4fd3b3d
Adjust formatting of `SQLiteStore.__contains__`
jakirkham ed9c3b0
Drop `SQLiteStore`'s implementation of `rename`
jakirkham 075eabc
Just name the SQL table "zarr"
jakirkham 06e0fec
Unwrap some lines to compact the code a bit
jakirkham a07ecba
Simplify `SQLiteStore.__contains__` code wrapping
jakirkham d3e4f3d
Check SQLite Cursor's rowcount for deletion
jakirkham 7bff163
Parenthesize operations to `?` in SQL
jakirkham 0c5fa9d
Check `rowcount` for values less than `1`
jakirkham 9aa5381
Parenthesize a few other SQL commands with `?`
jakirkham 1edf86a
Use one line for `SQLiteStore.rmdir`'s SQL
jakirkham 0618dbb
Use 1 line for `SQLiteStore.rmdir`'s SQL & params
jakirkham d55ac16
Update docs/release.rst
alimanfoo 996fd77
`TestSQLiteStore` -> `TestGroupWithSQLiteStore`
jakirkham d268144
Drop `else` in `for`/`else` for clarity
jakirkham 207565d
Ensure SQLite is new enough to enable threading
jakirkham 7e86d3e
Add spacing around `=`
jakirkham 043eec4
Merge 'zarr-developers/master' into 'jakirkham/add_sqlite_store'
jakirkham 0282fbf
Hold a lock for any DML operations in SQLiteStore
jakirkham c65f78f
Raise when pickling an in-memory SQLite database
jakirkham 505ac5f
Test in-memory SQLiteStore's separately
jakirkham 0bad6c5
Drop explicit setting of `sqlite3` defaults
jakirkham 0dc34bb
Adjust inheritance of `TestSQLiteStoreInMemory`
jakirkham f5b8913
Merge 'zarr-developers/master' into 'jakirkham/add_sqlite_store'
jakirkham File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any value in allowing this lock (or the lock type) to be specified by the user? For example, we've found this sort of thing to be useful at times when using dask-distributed for I/O operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alimanfoo and I were a little concerned by some of the wording in the docs about
sqlite3and thread safety as discussed here. Having this lock may be (overly) cautious. It's not clear what the right answer is here. Would be happy to hear other thoughts on this if you have any.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine for now. We can certainly revisit this down the road if additional functionality is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jhamman. SGTM