Skip to content
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

Upstream/1339 special os file stores #1358

Closed

Conversation

Jimmy-653
Copy link
Contributor

I'm not sure if that's the kind of special implementation you were aiming at.
It still needs for every special OSFileStore on each OS to be implemented.
I've created the empty OverlayOSFileStore class but didn't implement any OS because I'm not sure which informations are available and don't have a docker setup to test it.

Feel free to extend this PR with a completed OverlayOSFileStore (at least the linux version should be implemented to cover #1339) before merging it.

@coveralls
Copy link

coveralls commented Oct 14, 2020

Coverage Status

Coverage decreased (-0.07%) to 87.904% when pulling bf47ab2 on J-Jimmy:upstream/1339_special_os_file_stores into f131fa1 on oshi:master.

@dbwiddis dbwiddis added the hacktoberfest-accepted Accepted during Hacktoberfest label Oct 14, 2020
@dbwiddis
Copy link
Member

Thanks for the attempt, but after reviewing briefly there's several things I don't like.

  • I really (really, really) don't want to add another dependency in the oshi-core project. If there is significant value in it, I would welcome functionality in a new sub-project. But I don't quite yet see that here.
  • I'm not sure a new class/interface is the solution, if it mostly repeats information from an existing interface is the way to go. That said, if we do create a new class/interface, it should be sufficiently unique to require separate handling. I think there's a case to be made for a new system here, but I don't want to over-specialize on the implementation in Linux rather than the concept. We do try to deal with this sort of thing on the Disk/Partition level, and I could see a similar implementation here (or totally different for both) but I'm not sure what that would look like yet.

My initial thought on this was just to allow configuration of the "suppressed" filesystem types or paths , currently handled in the code with the constants PSEUDO_FS_TYPES, NETWORK_FS_TYPES, and TMP_FS_PATHS. Rather than hardcoding these lists, we could easily add an entry in the config file (which can be overridden using GlobalConfig.set() on startup) identifying those strings. A user could simply remove overlay from the list if they wanted to see those in the existing output.

If there's some statistic for overlay stats (perhaps a list of the filestores they overlay) then perhaps an additional method on the OSFileStore interface could be added.

@dbwiddis dbwiddis removed the hacktoberfest-accepted Accepted during Hacktoberfest label Oct 14, 2020
@dbwiddis
Copy link
Member

Closing in favor of #1359

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants