Ensure that configuration options are considered for logging config#2375
Merged
sphuber merged 1 commit intoJan 9, 2019
Conversation
The configuration knows various options to change the logging configuration, however, these were not respected for two reasons: * Logging configuration was not lazily evaluated * Globally configured options were not agglomerated The first problem was caused by the fact that the logging configuration is evaluated upon loading the `aiida` module, at which point the profile is not necessarily loaded yet, causing the `get_config_option` functions to return the option defaults. The solution is to have the dictionary lazily evaluated by using lambdas, which are resolved when `configure_logging` is called. Finally, we make sure this function is called each time the profile is set. The second problem arose from the fact that if a profile is defined, the `get_config_option` only returned the config value if explicitly set for that profile and otherwise it would return the option default. This means that if the option was defined globally for the configuration it was ignored. This is now corrected where if the current profile does not explicitly define a value for the option but it is globally defined, the global value is returned.
sphuber
force-pushed
the
fix_2372_configuration_logging_option_loading
branch
from
f325807 to
3638fda
Compare
giovannipizzi
approved these changes
Jan 9, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2372
The configuration knows various options to change the logging
configuration, however, these were not respected for two reasons:
The first problem was caused by the fact that the logging configuration
is evaluated upon loading the
aiidamodule, at which point the profileis not necessarily loaded yet, causing the
get_config_optionfunctionsto return the option defaults. The solution is to have the dictionary
lazily evaluated by using lambdas, which are resolved when
configure_loggingis called. Finally, we make sure this function iscalled each time the profile is set.
The second problem arose from the fact that if a profile is defined, the
get_config_optiononly returned the config value if explicitly set forthat profile and otherwise it would return the option default. This
means that if the option was defined globally for the configuration it
was ignored. This is now corrected where if the current profile does not
explicitly define a value for the option but it is globally defined, the
global value is returned.