core: add 'core' config section, add disabled_modules/enabled_modules, use in hpi modules and hpi doctor

[karlicoss]

Example config:

class core:
    disabled_modules = [
        'my.pdfs',
        'my.body.*',
    ]

hpi doctor outputs:

✅ OK  : my.bluemaestro                
✅     - stats: {'measurements': {'count': '100+', 'last': datetime.datetime(2018, 7, 15, 4, 37)}}
🔲 SKIP: my.body                        module disabled in config
🔲 SKIP: my.body.blood                  module disabled in config
🔲 SKIP: my.body.weight                 module disabled in config
✅ OK  : my.calendar                   
✅ OK  : my.calendar.holidays          
❌ FAIL: my.coding.codeforces           loading failed; pass --verbose to print more information

Happy for any comments. Mainly I'm not sure about:

1. Naming: core config section -- hopefully makes sense?
2. Naming: active/enabled/disabled/on/off -- not sure what's the best? I'll give it a think, ideally would be nice to use some widely adopted convention. The most important is the config variable, the rest can be renamed later.
3. Including SKIP by default (they are still sort of spammy, especially if you're only using just a few modules). Perhaps it's better to add a flag (e.g. --all) and only output a summary by default (e.g. "20 modules skipped, use --all"

[seanbreckenridge]

I think class core makes sense.

active/enabled/disabled/on/off

I think enabled_modules and disabled_modules is good.

Including SKIP by default (they are still sort of spammy, especially if you're only using just a few modules). Perhaps it's better to add a flag (e.g. --all) and only output a summary by default (e.g. "20 modules skipped, use --all"

Would agree with this.

For hpi doctor, personally, I've just done this:

diff --git a/my/core/__main__.py b/my/core/__main__.py
index 6ba675b..4ce8fa6 100644
--- a/my/core/__main__.py
+++ b/my/core/__main__.py
@@ -171,10 +171,10 @@ def modules_check(args):
                 tb(e)
             continue
 
-        info(f"{color.GREEN}OK{color.RESET}  : {m:<30}")
         stats = getattr(mod, "stats", None)
         if stats is None:
             continue
+        info(f"{color.GREEN}OK{color.RESET}  : {m:<30}")
         from . import common
 
         common.QUICK_STATS = False

(which means if a module doesnt have stats and it loads properly, it prints nothing in hpi doctor)

Not saying the hpi doctor and enabling/disabling modules are the same thing, but its the main use.

For me currently:

• If module doesn't load, it shows 'failed' and the error message (pass --verbose)
• If module loads and it doesnt have a stats function, it doesn't print anything
• If module loads and it has stats, prints checkmark and computed stats.

Compare my hpi modules output to hpi doctor output (notice the my.food/my.google helper modules that are ignored, without modifying an ignore list)

I think the extension of that idea with the SKIP would be:

For hpi modules:

Print whether or not modules are enabled/disabled, but show all of them. Currently seems like you ignore those, maybe, the --all flag could be used here as well; if --all is passed to hpi modules, show all of them, even if they're disabled. If --all isn't passed, just show enabled modules. --all would probably be split in check marks/skipped boxes, like:

$ hpi modules
🔲 SKIP: my.body.weight                 module disabled in config
✅ OK  : my.calendar                   

For hpi doctor

• If --all not passed:
  • If module doesn't load, dont show anything (though, print a summary at the end of how many are failed/disabled, and mention the --all flag)
  • If module loads properly but no stats, dont display anything (sort of spammy, especially for possible helper .py files that aren't 'modules')
  • If module loads and has stats, show stats
• If --all passed:
  • If a module doesn't load:
    • if disabled by config file:
      • show SKIP
    • else
      • show 'FAIL, pass --verbose' error message
  • If a module loads but no stats, show it anyways (this is --all, after all)
  • If module loads and has stats, show stats

That way, if a module errors, it displays. But if a module loads properly and it has no stats, hpi doctor is a concise as possible, without you having to manually disable things in your config file. You always have the option to pass --all to increase verbosity.

I think hpi doctor not showing a bunch of errors (and just the 'ignoring 25 disabled modules' at the end) by default is nice when starting out modifying HPI, makes it less scary for a user and introduces them to the hpi doctor interface.

[karlicoss]

Thanks for the comments!

Currently seems like you ignore those,

Yeah, I wasn't sure yet what to do with these -- basically the issue is that only a subset of Python files are HPI modules. Some I'll probably leave there (e.g. my.core, and kython stuff, which probably belongs to the core too). The rest is trickier. E.g. if you have 'utils.py' within some module, you probably don't want it to show up in hpi modules or in hpi doctor --all. Then there are these options (as I see it):

1. Explicitly mark HPI modules so they are detected by doctor. Yep, like you suggested, def stats could be one of such 'markers'. And I guess if for some reason they don't want to implement stats, they can always just run hpi doctor against the specific module they want.

   In addition, hpi cli could have another mode (e.g. lint or something like that): if it thinks a file looks like an HPI module (e.g. the only file in a package, or uses my.config, etc), it could suggest adding stats. But maybe it's overthinking at this stage.

2. Or, explicitly mark the files that aren't HPI modules (e.g. stuff like common.py/utils.py/etc) so they don't show up in doctor. Feels somewhat wrong though.

   There could also be some hybrid approach, e.g. the 'main' module file lists the 'helper' files in a variable. Then doctor reads these variables, and excludes the listed files. But again, feels like it's stepping into the territory of order resolution/circular dependencies/etc -- perhaps it's not worth it.

That way, if a module errors, it displays. But if a module loads properly and it has no stats, hpi doctor is a concise as possible, without you having to manually disable things in your config file. You always have the option to pass --all to increase verbosity.

Yep, I like it, good idea! I guess also allows to defer the decision how to detect HPI modules and non-modules.

[seanbreckenridge]

E.g. if you have 'utils.py' within some module, you probably don't want it to show up in hpi modules or in hpi doctor --all

Yeah. By default it probably shouldn't show up?

I think all files should be listed when you pass hpi doctor or hpi modules the --all flag (if its implemented for both of those commands). If a module would be ignored by something in my.config.core.disabled_modules, it should still print and show SKIP when hpi modules is passed the --all flag.

explicitly mark
def stats could be one of such 'markers'.

Perhaps the concept of a 'marker' could also affect hpi modules?

Currently, I see the markers as:

• explicitly included in my.config.core.enabled_modules
• has a stats function

(e.g. stuff like common.py/utils.py/etc) so they don't show up in doctor. Feels somewhat wrong though.

Yeah, automatically ignoring a base filepath in new modules a user might create feels wrong. Ignoring core/kython makes sense, but if someone created my.modulename.utils and its just ignored because the base filepath matches, could lead to confusion/errors down the line. Feels like too much restriction and then having adding that to my.config.core.enabled_modules afterwards to force it to show is annoying, both logic-wise and for the user experience.

As long as the def stats is documented as one of the possible 'markers', and when you don't pass the --all flag, the --all flag is mentioned at the bottom of the summary, I think that would be the best for new users to introduce the system, as well as provide configuration options for someone whose added additional personal modules.

[karlicoss]

Perhaps the concept of a 'marker' could also affect hpi modules

Yep, doesn't hurt.

Implemented the way we discussed: if the module is suppressed, hpi doctor/modules doesn't list it by default, and it can be controlled via --all. User config takes the precedence, if the module is enabled, it's shown regardless stats, etc. If it's disabled in the config, it's not shown regardless stats, etc. It should also be much faster now, because it doesn't try importing the modules if they are suppressed.

Also I actually went ahead and added __NOT_HPI_MODULE__ flag, I feel like it might be useful to mark utility modules so they are reported differently in the CLI. If it doesn't prove useful, would be trivial to deprecate. And it already allowed to clean up the hardcoded mess in the function checking for ignored modules.