initc3 / HoneyBadgerMPC

Robust MPC-based confidentiality layer for blockchains
GNU General Public License v3.0
131 stars 64 forks source link

Review configuration mechanism #382

Closed sbellem closed 4 years ago

sbellem commented 4 years ago

The current configuration mechanism has some strength and weaknesses. One of its current strengths appears to be that it is very convenient as it happens automatically in the root __init__.py:

https://github.com/initc3/HoneyBadgerMPC/blob/0914c0269a5d3636b01f40557e8ea432f721bd04/honeybadgermpc/__init__.py#L23

This comes at a cost though: building documentation that has docstrings fails (see #378). Perhaps it could also be a cause to other problems in the future but for now that seems to be the only problem. So one may object that it is a very low cost in exchange for the convenience it provides. Perhaps. Nevertheless, conceptually speaking it seems that we could improve the current mechanism in place to not only restore the capability of building documentation but to also make the configuration mechanism clearer, more explicit, and re-usable as needed in the modules that require it.

Rough Planning Notes

Observations

The main observation one may make is that the "loading config" code that reads command line arguments does not need to be invoked by default in the project root __init__.py.

Questions

Configuration Mechanisms in other Projects

How do other projects do it? Which projects are similar in terms of structure and patterns so that it makes to look at how they handle the configuration mechanism?

sbellem commented 4 years ago

Review and understand how the current mechanism works

Applications making use of HbmpcConfig:

It seems that the modules that depend on HbmpcConfig are meant to invoked by a higher-level script likelaunch-tmuxlocal.sh which is responsible to read and pass the config file that is then loaded via the root __init__.py. So, all these modules that can be invoked by a script like launch-tmuxlocal.sh depend on the script itself passing the -f arg to the module which will then pass it to the load_config parser invoked in the root __init__.py.

One possible alternative would be to have the explicit HbmpcConfig.load_config() call in each relevant module under the script section (if __name__ == "__main__":) as it is already done in https://github.com/initc3/HoneyBadgerMPC/blob/0914c0269a5d3636b01f40557e8ea432f721bd04/apps/asynchromix/powermixing.py#L167-L170. The inconvenient with this approach would be that one would need to always be careful to add the load_config when writing a new module that requires the configuration step.