Use sane environment by default #538
Conversation
- by default, the repository has debug enabled, meaning the get_debug_config is called overwriting the user config. That is not nice. Changed by default to false. In the future this can be a configuration flag? - when no config file is set, and debug is false, no configuration is loaded. This causes a lot of errors (namely key not founds). Given the repo includes a config file, added code to load it by default - when using the default config file, warn the user about that Some details can be made better, but for now, it is working perfectly on my docker environment. I am happy to provide further changes on this branch if you like.
|
@ambs thanks - looks like there's an error to fix. |
| @@ -781,7 +798,7 @@ def _make_image(self, image_request, image_info): | |||
|
|
|||
| sys.path.append(path.join(project_dp)) # to find any local resolvers | |||
|
|
|||
| app = create_app(debug=True) # or 'opj' | |||
| app = create_app(debug=False) # or 'opj' | |||
There was a problem hiding this comment.
this run_simple() code here isn't meant for production use. Not sure we should change the debug default to false in create_app() - it might look like this is an approved way to run loris in production.
@alexwlchan any thoughts?
There was a problem hiding this comment.
Yeah, I would leave the default as debug=true. Clear sign that this is only for local testing.
There was a problem hiding this comment.
The problem is that it isn't documented, and it is not clear why some flags in the config file are being overwritten by the get_debug_config method. This function is my main concern. I do not think it is a problem to keep the debug as true if we honour the data from the config file, and not rewrite it.
|
I fixed the problem. It's not a clean solution yet. |
|
@ambs I think updating get_debug_config to just set values if they're not present - would be ok. That way configuration in the config file isn't overwritten, as you point out. However, to update get_debug_config, you might need to update the tests, which use that function. You might need to add a new function to set the correct config for the tests. |
|
@bcail I am happy with that. It might take some more time to prepare a decent patch, but I will try to prepare it. |
|
Take a look at the CI configuration: https://github.com/loris-imageserver/loris/blob/development/.github/workflows/ci.yml. That shows how to set up the environment for running the tests. |
by default, the repository has debug enabled, meaning the get_debug_config is called
overwriting the user config. That is not nice. Changed by default to false.
In the future this can be a configuration flag?
when no config file is set, and debug is false, no configuration is loaded. This causes
a lot of errors (namely key not founds). Given the repo includes a config file, added
code to load it by default
when using the default config file, warn the user about that
Some details can be made better, but for now, it is working perfectly on my docker environment.
I am happy to provide further changes on this branch if you like.