Clean up old assets before downloading new ones#314
Clean up old assets before downloading new ones#314crazytonyli wants to merge 1 commit intotrunkfrom
Conversation
| // Cleanup errors are intentionally ignored so that the download process | ||
| // continues uninterrupted. Failures are expected when running for the | ||
| // first time, since the storage directories may not yet exist. | ||
| await onceEvery(.seconds(86_400)) { |
There was a problem hiding this comment.
I think it may be worth reconsidering the "once every x interval" implementation.
The "handle" of this timestamp is static, which means one timestamp applies to all sites. At the bare minimum, the timestamp should be stored per site (configuration.siteURL).
Since we'll be supporting multiple post types, the number of handles will be N(sites) x N(post types). I don't feel like UserDefaults is the right place to store that data.
What do you think about using the last modified date of the manifest.json as the timestamp, so that we don't need to store any more timestamps anywhere?
I struggle to understand the problem we aim to solve with these changes. Following the reproduction steps did not help me either; I always saw the activity indicator, not the progress bar. Would be willing to elaborate on the observed problem please? |
|
@dcalhoun Sorry, I misunderstood the code. I double checked. There is no issue with the current implementation. |
What?
Perform the cleanup step before the download assets step.
Why?
The cleanup step removes the assets that were just downloaded if the previous function call was one day ago.
To reproduce the issue locally: