Adds background job to cleanup all previews. #2152
Conversation
|
@rullzer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @LukasReschke, @DeepDiver1975 and @PVince81 to be potential reviewers. |
|
tests? |
|
Yes I still want that. but testing is a bitch because DI doesn't work for core stuff there 😕 |
rullzer
force-pushed
the
preview_cleanupjob
branch
3 times, most recently
from
fa61b98
to
059d5af
Compare
|
Tests added |
|
👍 |
Current coverage is 57.75% (diff: 75.00%)@@ master #2152 diff @@
==========================================
Files 1176 1178 +2
Lines 70737 70798 +61
Methods 7213 7220 +7
Messages 0 0
Branches 1212 1212
==========================================
+ Hits 40846 40890 +44
- Misses 29891 29908 +17
Partials 0 0
|
| $c->getRootFolder(), | ||
| $c->getLogger(), | ||
| $c->getJobList(), | ||
| new TimeFactory() |
There was a problem hiding this comment.
I think you should just register a service ITimeFactory::class, then CleanPreviewsBackgroundJob can be build by the 👾
There was a problem hiding this comment.
I would expect it to be able to auto inject an ITimeFactory already
There was a problem hiding this comment.
Only for apps, since it's only defined in DIContainer not in Server
There was a problem hiding this comment.
Nope won't work because it is not an App. So indeed no DIContainaer which means the DI magic fails.
There was a problem hiding this comment.
Yeah, but when you create it for ITimeFactory it should work
|
cc @icewind1991 |
|
@nickvergessen @rullzer What are the next steps here? |
|
DI fails here with the ITimeFactory after a quick test. I vote to get it in. we can redo DI magic later. |
| public function run(IOutput $output) { | ||
| $this->userManager->callForSeenUsers(function(IUser $user) { | ||
| $this->jobList->add(CleanPreviewsBackgroundJob::class, ['uid' => $user->getUID()]); | ||
| }); |
There was a problem hiding this comment.
Could we add a check if this was already added? Otherwise this is added on every update. Just add an entry to appconfig 😉
* A repair step that inserts a background job for each user * Each background job will delete for 15 seconds if it takes longer we reschedule. This is done so instances that don't use the system cron won't time out. * Added tests Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
rullzer
force-pushed
the
preview_cleanupjob
branch
from
059d5af
to
78a318d
Compare
rullzer
dismissed stale reviews from nickvergessen and MorrisJobke
Lets fix it separatly
|
Tested and still works 👍 |
reschedule. This is done so instances that don't use the system cron
won't time out.
CC: @MorrisJobke @nickvergessen @icewind1991 @LukasReschke