-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disks filtering by mount points from a configurable parameter #572
base: master
Are you sure you want to change the base?
Disks filtering by mount points from a configurable parameter #572
Conversation
2e16092
to
c161b8b
Compare
Use a new system config variable 'serverinfo_disk_filter_paths' that can contains a list of paths (actually a simple string with ':' as delimiter). If defined, only disks with a corresponding mount point are shown. The paths list can contains special name (DOCROOT, DATADIR, TMPDIR and LOGSDIR) which are automatically converted (example: "serverinfo_disk_filter_paths" => "DATADIR:/var/log"). Signed-off-by: Grégory Marigot <gmarigot@teicee.com>
Signed-off-by: Grégory Marigot <gmarigot@teicee.com>
Signed-off-by: Grégory Marigot <gmarigot@teicee.com>
Signed-off-by: Grégory Marigot <gmarigot@teicee.com>
f4d79c3
to
1d8c635
Compare
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! |
Signed-off-by: Grégory Marigot <gmarigot@teicee.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your pull request 👍
@@ -70,7 +72,47 @@ public function getUptime(): int { | |||
} | |||
|
|||
public function getDiskInfo(): array { | |||
return $this->backend->getDiskInfo(); | |||
$disks = $this->backend->getDiskInfo(); | |||
$filters = $this->config->getSystemValue('serverinfo_disk_filter_paths', null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have two configuration systems in Nextcloud: SystemConfig and AppConfig
I'm using AppConfig here in serverinfo nowadays.
$filters = $this->config->getSystemValue('serverinfo_disk_filter_paths', null); | |
$filters = $this->config->getAppValue('disk_filter', ''); |
You can also use occ to configure it.
For example like https://github.com/nextcloud/serverinfo?tab=readme-ov-file#restricted-mode--nextcloud-28
return $this->backend->getDiskInfo(); | ||
$disks = $this->backend->getDiskInfo(); | ||
$filters = $this->config->getSystemValue('serverinfo_disk_filter_paths', null); | ||
if ($filters === null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ($filters === null) { | |
if ($filters === '') { |
If you follow my suggestion from above, don't forget to replace null with empty string.
return $disks; | ||
} | ||
// apply defined filters to restrict the list of disks according to their mount point | ||
$filtered_disks = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$filtered_disks = []; | |
$filteredDisks = []; |
CamelCase please ;)
// convert special filters to their corresponding paths | ||
switch ($filter) { | ||
case 'DOCROOT': $path = ''; | ||
if (isset($_SERVER['SCRIPT_FILENAME'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the docroot special filter because it might be empty.
A good alternative might be \OC::$SERVERROOT which always point to the nextcloud installation dir.
Here is my latest version from #426
Use a new system config variable
serverinfo_disk_filter_paths
that can contains a list of paths (actually a simple string with:
as delimiter). If defined, only disks with a corresponding mount point are shown.The paths list can contains special name (
DOCROOT
,DATADIR
,TMPDIR
andLOGSDIR
) which are automatically converted (example:"serverinfo_disk_filter_paths" => "DATADIR:/var/log"
).It works perfectly for my needs, I usually configure my Nextcloud instances with :
occ config:system:set --value="DATADIR" -- serverinfo_disk_filter_paths