Add per-user storage view#40477
Add per-user storage view#40477pako81 wants to merge 9 commits intoowncloud:masterfrom pako81:add_per_user_storage
Conversation
| \OC_Util::tearDownFS(); | ||
| \OC_Util::setupFS($user->getUID()); | ||
| $storage = \OC_Helper::getStorageInfo('/'); | ||
| $storageUsed = sprintf("%d MB", $storage['used'] / 1024 / 1024); |
There was a problem hiding this comment.
I think this will return an integer value, so instead of 3.45 MB it will return 3 MB. \sprint('%.2f MB', .....) might be better because it will round the division to 2 decimals.
There was a problem hiding this comment.
I have rounded it to 1 decimal since the personal setting page is doing the same You are using 4.1 MB of 10 MB (41.35 %), so we should align?
| \OC_Util::setupFS($user->getUID()); | ||
| $storage = \OC_Helper::getStorageInfo('/'); | ||
| $storageUsed = sprintf("%d MB", $storage['used'] / 1024 / 1024); | ||
| } catch (\Exception $ex) { |
There was a problem hiding this comment.
This will require a log entry, specially because the 0 MB storage should be rare. We don't expect users without any file, so a 0 MB storage would means that something has happened, and in this case we must know what happened.
Also note that we're catching all the exceptions. We should catch only specific exceptions that we know it could happen and we know how to handle them. If we have to catch any exception, it's important to know what's going on. We might think we're catching an storageNotAvailable exception, but in fact, we might hiding a different exception, so troubleshooting will be harder.
There was a problem hiding this comment.
added log entry, not sure however if this is enough based on your comment above.
|
We probably need to restore the FS of the initial user after the |
|
💥 Acceptance tests pipeline webUILogin-chrome-mariadb10.2-php7.4 failed. The build has been cancelled. |
|
@jvillafanez done. Not sure this is enough. |
| $this->log->error("Can't compute used storage for user " . $user->getUID() . ": " . $e->getMessage(), ['app' => 'settings']); | ||
| $storageUsed = "0 MB"; | ||
| } finally { | ||
| \OC_Util::tearDownFS(); |
There was a problem hiding this comment.
This seems the wrong place. Taking into account that this is a private function (can't be called from outside), the method is called at the end (chance of breaking things due to using the wrong user's FS is fairly low) and it's in a controller (there shouldn't be any code needed to access the FS after this), we could just ignore this and include some comments to warn that this method will setup a different FS.
|
Is it worthy to create a helper method somewhere in order to get the size of the root folder of the user's home without setting up a different FS? It seems less risky and there should be less overhead. |
|
@jvillafanez we can try, however I can see that i.e. the metrics app is doing the same https://github.com/owncloud/metrics/blob/master/lib/Metrics/QuotaMetrics.php#L62 |
|
Ok, then I think we just add some comments to warn that we're changing the FS user and we aren't reverting it back, at least for now. That should be good enough to finish this PR. |
3 participants
Description
Add per-user storage view
Related Issue
Motivation and Context
Make possible to visualise used storage on per-user basis
How Has This Been Tested?
Storage Usedwill display the amount of used storage for every user. This option can be also opted in/out underSettingsin order to align with all other available users' options.Types of changes
Checklist: