User Management backend - Base - Config file #37
Conversation
Added config file so the app will auto configured once the flask app starts.
|
Unfortunately GitHub makes it close to impossible to properly review PRs that depend on other PRs because it shows all the changes mixed together instead of letting you see only the changes in the relevant PR. So we'll need to merge the first PR before we can review the ones that depend on it (This is why I don't personally consider GitHub to be a serious code review platform). |
edison/config.py
Outdated
| if(len(config_dict) == 0): | ||
| # Iterating through all config.py members | ||
| for name, obj in inspect.getmembers(sys.modules[__name__]): | ||
| # We're interested only with the derived classes of the Config class |
There was a problem hiding this comment.
you're not really checking here is the class you've got is derived from Config, only that is is not the Config class itself
edison/config.py
Outdated
|
|
||
| config_dict = {} | ||
|
|
||
| # If config_dict is empty this function builds it dynamically |
There was a problem hiding this comment.
Given that the get_config_object function is AFAIK only called once when the Flesk process starts, do you really need to cache the classes in the config_dict? It seems to me you can simplify this code.
Also, please use docstrings to document functions and not comments.
I think the docs for this function need to be improved, its doing non-trivial things like use the inspect module and returning classes. The motivations and techniques for doing this need to be explained. Try to see of your peers that did not write this code can figure out why you wrote it and what it does...
2. replace comments with docstring
few changes in the docstring
remove space line
fix indentation in docstring
change inspect.isclass(Config) to issubclass(obj, Config)
fixed the wrong checking - if a class is subclass of config
fixes #27.
This PR depends on PR #36.
Added config file so the app will auto configured once the flask app starts.
@ahinoamta @RoniRush