Fix Config loading code #294
Conversation
Also, remove unused functions and parameters
webserver/__init__.py
Outdated
| return app | ||
| if deploy_env: | ||
| print('Config file loaded!') | ||
| pprint(dict(app.config)) | ||
|
|
||
|
|
||
| def create_app(debug=None, config_path=None): |
There was a problem hiding this comment.
I don't think we have any code that calls create_app(config_path=...). Can we remove this argument?
webserver/__init__.py
Outdated
| return app | ||
| if not os.path.exists(config_file): | ||
| print("No config file generated. Retried %d times, exiting." % CONSUL_CONFIG_FILE_RETRY_COUNT) | ||
| sys.exit(-1) |
There was a problem hiding this comment.
app.config.from_pyfile will fail anyway if the file doesn't exist. Does that make the sys.exit unneeded?
webserver/__init__.py
Outdated
| def load_config(app): | ||
| """Load configuration file for specified Flask app""" | ||
| config_file = os.path.join(os.path.dirname(os.path.realpath(__file__)), "..", "config.py") | ||
| for _ in range(CONSUL_CONFIG_FILE_RETRY_COUNT): |
There was a problem hiding this comment.
in development I'd like it much more to have a failure message immediately if I forget to copy config.py.example to config.py rather than have to wait 10 seconds for the message to appear. Is there a way that we can support both of these cases?
Also, remove extraneous parameters and sys.exit
webserver/__init__.py
Outdated
| if deploy_env: | ||
| for _ in range(CONSUL_CONFIG_FILE_RETRY_COUNT): | ||
| if not os.path.exists(config_file): | ||
| time.sleep(1) | ||
|
|
||
| if not os.path.exists(config_file): | ||
| print("No config file generated. Retried %d times, exiting." % CONSUL_CONFIG_FILE_RETRY_COUNT) |
There was a problem hiding this comment.
This sentence is not strictly true in development any more. Do we care?
There was a problem hiding this comment.
Perhaps we could fix it by indenting one level?
Action points:
config_pathargumentcreate_app_with_configurationmethod, which is no longer used independentlyprintin production