Skip to content
This repository has been archived by the owner on Nov 28, 2023. It is now read-only.

Update records in SQLite via SQL instead of re-creating the database/file #11

Closed
wants to merge 1 commit into from

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented Jul 8, 2020

This moves the update from the filesystem layer into the SQL layer, thus
allowing multiple processes to coordinate. Datasette holds open SQLite
connections and was keeping references to the deleted files.

Since the Makefile recipe got more complicated, move it to a shell
script.

Resolves #10.

…file

This moves the update from the filesystem layer into the SQL layer, thus
allowing multiple processes to coordinate.  Datasette holds open SQLite
connections and was keeping references to the deleted files.

Since the Makefile recipe got more complicated, move it to a shell
script.

Resolves #10.
@tsibley tsibley requested a review from a team July 8, 2020 01:35
@tsibley
Copy link
Member Author

tsibley commented Jul 8, 2020

Since I fixed the sqlite-utils insert --truncate PR, I also pushed an alternate implementation to update-database-in-place-alternate: https://github.com/seattleflu/scan-switchboard/compare/update-database-in-place-alternate

Either implementation is totally fine by me but wanted to put both out there.

# room for an empty table after a successful delete but failed insert.
# -trs, 7 July 2020
if [[ -e "$database" ]]; then
# sqlite3 appears to exits non-zero on a SQL error only when the SQL
Copy link

Choose a reason for hiding this comment

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

typo: exit instead of exits

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

See the tiny typo and a question about confirming that the DELETE worked.

@kairstenfay kairstenfay self-requested a review July 8, 2020 16:12
@kairstenfay
Copy link
Contributor

Since I fixed the sqlite-utils insert --truncate PR, I also pushed an alternate implementation to update-database-in-place-alternate: https://github.com/seattleflu/scan-switchboard/compare/update-database-in-place-alternate

Either implementation is totally fine by me but wanted to put both out there.

Given that you fixed the --truncate PR, I am partial to your solution including it because it's much more elegant and uses less bash (which we know not everyone appreciates 🙂).

Copy link

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

I prefer the sqlite-utils --truncate solution. Sounds like they are open to making improvements to how they handle transactions 👍

@tsibley
Copy link
Member Author

tsibley commented Jul 8, 2020

Closing this in favor of #12 then. Thanks for all your comments and our group programming yesterday!

@tsibley tsibley closed this Jul 8, 2020
@tsibley tsibley deleted the update-database-in-place branch July 8, 2020 19:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deployed front-end doesn't reload to include latest data
3 participants