Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that configuration options are considered for logging config #2375

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Jan 9, 2019

Fixes #2372

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.

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 sphuber force-pushed the fix_2372_configuration_logging_option_loading branch from f325807 to 3638fda Compare January 9, 2019 11:54
@coveralls
Copy link

coveralls commented Jan 9, 2019

Coverage Status

Coverage increased (+0.01%) to 68.983% when pulling 3638fda on sphuber:fix_2372_configuration_logging_option_loading into 9b6b41a on aiidateam:provenance_redesign.

@sphuber sphuber merged commit 65c60d5 into aiidateam:provenance_redesign Jan 9, 2019
@sphuber sphuber deleted the fix_2372_configuration_logging_option_loading branch January 9, 2019 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants