Conversation
Adds a permanent_delete option (default false, preserving current behavior). When enabled, async_delete_backup also calls trash_clear on the deleted backup and metadata file ids after deletefile, permanently purging them from Trash and freeing quota immediately. Closes ghotso#29
- async_delete_backup now tracks whether trash_clear succeeded for the backup and metadata files and logs a warning (instead of a misleading "Successfully deleted") if permanent purge failed for either. - _async_trash_clear logs which file (backup/metadata) failed to purge and validates file_id before calling trash_clear. - Clarify in strings.json/translations that enabling permanent_delete makes deletions unrecoverable.
|
Thanks - overall this looks good and matches #29. Please address before merge: inline comments on backup.py (warning text) and de.json (copy). No action needed from you: I’ll extend permanent_delete to the other existing deletefile paths, do related cleanup, and refresh the README in a follow-up on my side after merging your PR. Thanks for your contribution and help 🥳 |
There was a problem hiding this comment.
purge_failed can be true when only metadata trash_clear fails, after the backup file was already purged. This message then wrongly implies the backup itself may still be in Trash.
Please broaden to something like: "…permanently purging one or more related files from pCloud Trash failed…"
Optional: log which part failed (backup vs metadata) — _async_trash_clear already has file_kind.
There was a problem hiding this comment.
Quick DE polish before merge:
Speicherplatz instead of Kontingent
Label: dauerhaft/endgültig löschen reads clearer than Papierkorb überspringen
Please use the same polished label/description in both folder_path and options.init (they’re duplicated on purpose).
|
@jimisola Are you still working on this PR? Any updates? |
- Broaden purge-failed warning to "one or more related files" to avoid implying the backup itself is still recoverable when only the metadata trash_clear failed (per ghotso review comment). - DE label: "Backups dauerhaft löschen" (clearer than Papierkorb überspringen). - DE description: "Speicherplatz" instead of "Kontingent" (more natural).
|
Hi Michael, thanks for the feedback and for the kind words! Just pushed a follow-up commit (13890c2) addressing both:
Looking forward to your other improvements on the |
|
Is the PR ok? |
2 participants
Implements the scope proposed in #29.
Summary
permanent_deleteoption (defaultfalse, current behavior unchanged).async_delete_backupcallstrash_clearon the deleted backup and metadata file ids afterdeletefile, permanently purging them from pCloud Trash and freeing quota immediately.trash_clearfailures are tracked per file (backup/metadata); if either fails, the delete still succeeds (matching the existing metadata-delete error handling style) but logs a warning that the backup may still be recoverable in Trash, rather than silently logging "Successfully deleted"._async_trash_clearvalidates the file id before callingtrash_clear.PRD/PRD.mdupdated to document the optionaltrash_clearstep.Files changed
custom_components/pcloud_backup/const.py—CONF_PERMANENT_DELETE/DEFAULT_PERMANENT_DELETEcustom_components/pcloud_backup/api.py—async_trash_clear(file_id)custom_components/pcloud_backup/backup.py—async_delete_backup+_async_trash_clearhelpercustom_components/pcloud_backup/config_flow.py— shared options schema + both flow stepscustom_components/pcloud_backup/strings.json+translations/{en,de}.jsonPRD/PRD.mdTest plan
permanent_delete=false(default), delete a backup — confirm it lands in pCloud Trash as before.permanent_delete=true, delete a backup — confirmdeletefilesucceeds and the same file id is then purged from Trash (trash_clear), freeing quota immediately.pytest-homeassistant-custom-componentcoverage for this (and other) areas in a separate PR if useful.Note on the German translation
I'm not a native/fluent German speaker — the
translations/de.jsonstrings were drafted to the best of my ability (and machine-assisted) but please treat them as a starting point. Happy to update if you or another reviewer can suggest better wording.