feat(updater): download resume and progress logging #571
Conversation
joshtrichards
force-pushed
the
jtr/feat-download-progress-logging-and-resume
branch
2 times, most recently
from
d25c492 to
0038485
Compare
Under some circumstances, yes. But we're also already leaving Archive files around today whenever there are failures before/ during the extraction step (either until the next Updater run or forever if a manual update is used after the Updater fails). If those are of concern we should probably just add a background job to clean them up.
To keep this PR from introducing any new potential problems we were previously saving ourselves from, I'll re-add clean-up at the top of the |
Signed-off-by: Josh Richards <[email protected]>
joshtrichards
force-pushed
the
jtr/feat-download-progress-logging-and-resume
branch
from
0038485 to
d34724a
Compare
joshtrichards
changed the title
Signed-off-by: Josh Richards <[email protected]>
Signed-off-by: Josh Richards <[email protected]>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Josh Richards <[email protected]>
| if (file_exists($storageLocation . 'nextcloud/')) { | ||
| $this->silentLog('[info] extracted Archive location exists'); | ||
| $this->recursiveDelete($storageLocation . 'nextcloud/'); | ||
| } |
There was a problem hiding this comment.
Could we instead delete any file that is not $saveLocation to clean out old downloads?
It’s not duplicated index.php is built with make and contains the other file. Unless I misunderstood what you’re talking about? |
downloadUpdate()step #570What it does:
updater.log(in 2% increments until reaching 10% then 10% increments after that)updater.logupon completion (bytes downloaded, total time, average speed)TODO (before merging):
Example log (with a partial previous download detected):
Most material change this makes internally: We no longer delete the
/downloads/storage location if it exists. Instead we delete only the extracted archive contents sub-folder (/downloads/nextcloud/).This was obviously necessary for the resume to work (since we want the Archive zip).
We still clean-up
/downloads/nextcloud/(just not the whole/downloads/folder) in case the Updater fails at later steps and leaves the extracted contents around.Follow-up PR ideas:
finalize()step. The downside being that ~250MB would be consumed until the Updater finishes rather than being freed up at the extraction stage. I don't know how to decide whether that's reasonable or not for all the different environments we try to support. So I tried to keep this PR conservative. Technically that space already had be available in the environment because the extraction step itself requires space for the Archive + the extracted contents since both must co-exist until the extraction is successful. Therefore I think it'd be safe move that clean-up tofinalize(). But with the Updater being a very sensitive area I figured... maybe a follow-up PR tweaking that makes more sense. :)