-
Notifications
You must be signed in to change notification settings - Fork 173
Hide docker mounts from diskusage by default #836
base: master
Are you sure you want to change the base?
Conversation
@@ -15,6 +15,24 @@ const CONFIG = { | |||
ssl: process.env.FLOOD_ENABLE_SSL === 'true' || process.env.FLOOD_ENABLE_SSL === true, | |||
sslKey: '/data/flood_ssl.key', | |||
sslCert: '/data/flood_ssl.cert', | |||
diskUsageService: { | |||
dontWatchMountPoints: [ |
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 think you can't just enumerate files our mountpoints here because they can be different for each distro. We need to find a cross-distro way to do that.
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.
Those are paths in the container, I don't know if the host distro affects this. I only tried on ubuntu.
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.
My bad, I missed it was for docker only.
Couldn't we suggest to the user to configure |
The config.js is part of the docker image. You can't edit it easily. (You can open a shell inside the container and edit using vi). Or you would have to build a custom image. Ideally I think this should be configurable in the GUI, but I don't know how to do that. |
Ah! ok that's even better, this way we know which partition to watch right? |
It seems that by default we're using /data as volume https://github.com/Flood-UI/flood/blob/master/Dockerfile#L46 perhaps that should be preset in config.docker.js instead? |
Well the user can call their mounts whatever they want. I'm doing "/downloads" for example. That's why I'm proposing the blacklist option. |
Just trying to figure out whether this could be solved without introducing extra configuration, which would be preferable. Another way of seeing this issue is that docker users who attach additional volumes to their container, don't have a straightforward method of adding those to the config. Or that docker users can't easily edit the config file. (something along the lines of The option to store to make this configurable from the GUI would mean that we'd store this information in the database. |
Hardcoding a bunch of paths in configuration file is certainly not great. And as @noraj points out it might not cover all docker implementations. I will try to make a GUI configurable version and do a new pull request. |
Another idea could be simply to add a new environment variable for the "watch mounts" setting (ie. comma separated value), like we do for Also the I had to create my own FROM jfurrow/flood-ui:latest
COPY config.custom.js ./config.js |
I'm actually mounting
I guess that is a reasonable default. It would work in my setup, since Ultimately I think the best would still be a GUI option, maybe I can figure out how to implement that. |
Totally agree, this is the best solution. I think it should be a per-user setting, and the values would be stored in the database. The user should be able to view all available mounts somewhere in the settings modal and disable/enable them at will. Are you up for tackling this? If not, I think this config solution would be fine for now. |
Yes I will spend some time on it today. |
Resolved by jesec/flood@ee83e8f, jesec/flood@b6ebee6. You may close the PR if it is no longer relevant. |
Description
I think the docker related mount points should be hidden by default since they don't add any information.
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
before:
after:
Types of changes
Checklist: