Persistent session storage #614
Conversation
Codecov Report
@@ Coverage Diff @@
## master #614 +/- ##
==========================================
+ Coverage 77.41% 77.79% +0.37%
==========================================
Files 110 110
Lines 10252 10448 +196
Branches 1259 1416 +157
==========================================
+ Hits 7937 8128 +191
- Misses 1918 1921 +3
- Partials 397 399 +2
Continue to review full report at Codecov.
|
|
Ok, I've added validator to
I've added some unit tests to (1) confirm Ping @kevin-bates, @mwakaba2 and @Wh1isper for a second round of reviews. Thanks, y'all! |
| "`database_filepath` expected a file path, but the given path is a directory." | ||
| ) | ||
| # If the file exists, but it's empty, its a valid entry. | ||
| if os.stat(path).st_size == 0: |
There was a problem hiding this comment.
I'd say just try to connect to the file here and let the error propagate. I verified that you can connect to an empty file.
| @@ -47,14 +46,12 @@ def _validate_database_filepath(self, proposal): | |||
| raise TraitError( | |||
| "`database_filepath` expected a file path, but the given path is a directory." | |||
| ) | |||
| # If the file exists, but it's empty, its a valid entry. | |||
| if os.stat(path).st_size == 0: | |||
| return value | |||
| # Verify that database path is an SQLite 3 Database by checking its header. | |||
| with open(value, "rb") as f: | |||
There was a problem hiding this comment.
I meant also for the contents validation piece, rather than inspecting the file
There was a problem hiding this comment.
I think sqlite3 overwrites the file if it's not a valid database format, rather than raising an exception if the file has contents that are not sqlite data.
|
Thank you! Merging! |
Fixes #611
Makes the session database path a configurable trait and avoids overwriting the database if it exists.
Also, sets the database's isolation level to
Noneso that it autocommits all changes after every change.