Hide docker mounts from diskusage by default

[tillkrischer]

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

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [ ] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I have read the CONTRIBUTING document.
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.

[sktt]

Couldn't we suggest to the user to configure watchMountPoints instead? Perhaps we can give a notification on the terminal output when starting flood in lines of "You probably want to specify the mountpoints that you want to see reported disk usage for". For example in your case you'd just do watchMountPoints: ["/downloads"]

[tillkrischer]

Couldn't we suggest to the user to configure watchMountPoints instead? Perhaps we can give a notification on the terminal output when starting flood in lines of "You probably want to specify the mountpoints that you want to see reported disk usage for". For example in your case you'd just do watchMountPoints: ["/downloads"]

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.

[sktt]

Ah! ok that's even better, this way we know which partition to watch right?

[sktt]

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?

[tillkrischer]

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.

[sktt]

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 docker exec -ti rtorrent-flood vi config.js and docker restart rtorrent-flood. Which would be lost after an upgrade)

The option to store to make this configurable from the GUI would mean that we'd store this information in the database.

[tillkrischer]

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.

[MacTwister]

Another idea could be simply to add a new environment variable for the "watch mounts" setting (ie. comma separated value), like we do for FLOOD_SECRET?

Also the /data volume is hard-coded in Dockerfile therefore can not be changed internally in the container from the perspective of the app meaning having a default in config.docker.js for watchMountPoints: ["/data"] is a suitable in my opinion. @tillkrischer You'r talking about your volume mount point on the host which is not what the app can see. If a user did manage changed the default DB path, that would mean they'd had made local modification as-well, meaning they'd be able to change the config values as-well.

I had to create my own Dockerfile and config.js locally to build a new image so I could have custom watch mount config value, see below, which as easy and simple to do. Only s downside is I'd have to manually update my config.js during any update where config changed.

FROM jfurrow/flood-ui:latest

COPY config.custom.js ./config.js

[tillkrischer]

@tillkrischer You'r talking about your volume mount point on the host which is not what the app can see.

I'm actually mounting /downloads in flood, but only to match my rtorrent container, so the paths in the file picker are correct. Now that you mention it I think it's more of a niche configuration.

Also the /data volume is hard-coded in Dockerfile therefore can not be changed internally in the container from the perspective of the app meaning having a default in config.docker.js for watchMountPoints: ["/data"] is a suitable in my opinion.

I guess that is a reasonable default. It would work in my setup, since /data and /downloads are on the same partition.

Ultimately I think the best would still be a GUI option, maybe I can figure out how to implement that.

[jfurrow]

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.

[tillkrischer]

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. If I fail I will let you know, then you can merge this.

[jesec]

Resolved by https://github.com/jesec/flood/commit/ee83e8f3aab7f7f4d8aa87cb04eef3b0eee8fd79, https://github.com/jesec/flood/commit/b6ebee63517eca908785a1bdf37e9fd128b6fd80.

You may close the PR if it is no longer relevant.