Add Restore for Backups#666
Conversation
|
Hi @schlick7 - thanks for the interesting PR! In the past I'd considered doing something like this but wasn't sure if it was a good idea ... data is such a precious thing, and I thought that making restores too easy would somehow be a bad thing :-) Anyway, I'll add some comments. Responding to these comments may be out of your range of expertise, in which case we'd want someone to pick it up and run with it. |
There was a problem hiding this comment.
Overall this is good work (thank you!), but it could create a real problem if a bad upload is done. This needs a whack of automated tests to ensure that it handles weird cases well.
Given the complexity of the cases that need to be handled, I wonder if this is necessary ... but that's just me, and having a graceful restore would be nice.
| self._remove_excess_backups(settings.backup_count, settings.backup_dir) | ||
| return f | ||
|
|
||
| def restore_backup(self, app_config, settings, filename): |
There was a problem hiding this comment.
I feel that this code needs to handle a few different cases, all of which should be really checked in the test_service.py:
- if I upload a .db.gz that was not a lute db, then everything will break. The code should first check that the db is in fact a valid db. It would need to connect to the code and then check for at least a few tables and columns (probably not all, just a few things should suffice)
- if the user restores an old db, then the code has to run db migrations on it. I don't know if you'd need to do a total restart or not ... I don't think so, would need to check the startup code
- if the db is bad, or the migrations fail (?), then the old db should be kept in place, nad the error reported back to the user
This section needs a fair amount of checking before the backup can be automated like this. At least with the user handling this manually, we can say that it's out of the code's hands, and so don't have to handle it. If the code manages it, then it has to handle everything.
| DatabaseBackupFile(os.path.join(outdir, f)) | ||
| for f in os.listdir(outdir) | ||
| if re.match(r"(manual_)?lute_backup_", f) | ||
| if f.endswith(".db.gz") |
There was a problem hiding this comment.
This should be fine, but it implies that the user won't try to upload/restore a bad db :-)
| return DatabaseBackupFile(fullpath) | ||
|
|
||
| def _set_backup_dir(self, dbfilename, backup_dir): | ||
| "Preserve the local backup_dir after restore." |
There was a problem hiding this comment.
Given that this refers to settings, it likely belongs in the lute/settings somewhere, and then the backup code would refer to that. This keeps all of the setting logic/structure in the same place.
|
Another thing that I haven't really thought about is how the restore interacts with user media (images) -- right now, the backup is just the db backup, I think, but maybe it needs to somehow be expanded to include media -- and that could be come a space drain. |
|
I knocked out the couple easy ones. Confirming the database seems like it would require a lot of code to be added. Easily done with an LLM, but it will probably be a little messy. |
|
I just uploaded a first pass at a db validation before restoring. Not sure if it goes far enough for you or not. |
|
I’m away for three weeks, will check on return. This is a very tough feature! Cheers @schlick7 thanks. |
2 participants
This was coded entirely with an LLM. I tried to keep it to the point and not touch anything not needed.
I have tested it locally with various backup files from different systems and all seem to work fine. I would recommend further testing by others though to confirm.
My reason for wanting this is to smooth the transition from going between Termux when I travel and my Server when I'm home.
Summary
.db.gzfiles so uploaded backups are visible (I can switch this to renaming the uploads instead if preferred)manual_prefix so they are not auto-prunedbackup_dirafter cross-system restoresNotes
backup_dirpreservation is intended to avoid breaking backups after restoring a DB created on a different system with different folder paths