From 4e4f744be99e08233c5f73955a84ab7c7440e82d Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Tue, 17 Aug 2021 22:05:04 +0200 Subject: [PATCH 1/7] Logging: add the `report` method and `REPORT` level to `logging` module The built-in `logging` module comes with a number of pre-set logging levels, such as `DEBUG` and `INFO`. AiiDA adds the `REPORT` level, which sits in between `INFO` and `WARNING` and is designed to add some extra granularity of control for messages. The report level is intended to be used for messages that are purely informational, and so not a warning, yet they should probably be shown by default. Traditionally, `INFO` log messages logged by module code are often a bit too verbose to always be showing by default. That is why AiiDA sets the default loglevel to `REPORT` allowing code, such as `WorkChain`s to emit informational messages that are shown by default, without also forcing all `INFO` messages to be shown, which are typically too verbose. To make the `REPORT` level behave like a built-in level, we hot-fix the `logging.Logger` class to also have a `report` method, such that it can be called as one would call `debug()` or `info()`. --- aiida/common/log.py | 27 ++++++++++--------------- tests/common/test_logging.py | 39 +++++++++++++++++++++++------------- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/aiida/common/log.py b/aiida/common/log.py index 0cc83df08f..e1c5429254 100644 --- a/aiida/common/log.py +++ b/aiida/common/log.py @@ -20,10 +20,20 @@ # Custom logging level, intended specifically for informative log messages reported during WorkChains. # We want the level between INFO(20) and WARNING(30) such that it will be logged for the default loglevel, however # the value 25 is already reserved for SUBWARNING by the multiprocessing module. - LOG_LEVEL_REPORT = 23 + +# Add the custom log level to the :mod:`logging` module and add a corresponding report logging method. logging.addLevelName(LOG_LEVEL_REPORT, 'REPORT') + +def report(self, msg, *args, **kwargs): + """Log a message at the ``REPORT`` level.""" + self.log(LOG_LEVEL_REPORT, msg, *args, **kwargs) + + +setattr(logging, 'REPORT', LOG_LEVEL_REPORT) +setattr(logging.Logger, 'report', report) + # Convenience dictionary of available log level names and their log level integer LOG_LEVELS = { logging.getLevelName(logging.NOTSET): logging.NOTSET, @@ -35,18 +45,9 @@ logging.getLevelName(logging.CRITICAL): logging.CRITICAL, } -# The AiiDA logger AIIDA_LOGGER = logging.getLogger('aiida') -# A logging filter that can be used to disable logging -class NotInTestingFilter(logging.Filter): - - def filter(self, record): - from aiida.manage import configuration - return not configuration.PROFILE.is_test_profile - - # The default logging dictionary for AiiDA that can be used in conjunction # with the config.dictConfig method of python's logging module def get_logging_config(): @@ -65,17 +66,11 @@ def get_logging_config(): 'datefmt': '%m/%d/%Y %I:%M:%S %p', }, }, - 'filters': { - 'testing': { - '()': NotInTestingFilter - } - }, 'handlers': { 'console': { 'level': 'DEBUG', 'class': 'logging.StreamHandler', 'formatter': 'halfverbose', - 'filters': ['testing'] }, }, 'loggers': { diff --git a/tests/common/test_logging.py b/tests/common/test_logging.py index 60ffcd3c70..09f51a5e51 100644 --- a/tests/common/test_logging.py +++ b/tests/common/test_logging.py @@ -7,21 +7,32 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### -"""Tests for the logging module.""" - +"""Tests for the :mod:`aiida.common.log` module.""" import logging -import unittest -class TestLogger(unittest.TestCase): - """Test global python logging module.""" +def test_logging_before_dbhandler_loaded(caplog): + """Test that logging still works even if no database is loaded. + + When a profile is loaded, the ``DbLogHandler`` logging handler is configured that redirects log messages to the + database. This should not break the logging functionality when no database has been loaded yet. + """ + msg = 'Testing a critical message' + logger = logging.getLogger() + logging.getLogger().critical(msg) + assert caplog.record_tuples == [(logger.name, logging.CRITICAL, msg)] + + +def test_log_report(caplog): + """Test that the ``logging`` module is patched such that the ``Logger`` class has the ``report`` method. + + The ``report`` method corresponds to a call to the :meth:``Logger.log`` method where the log level used is the + :data:`aiida.common.log.LOG_LEVEL_REPORT`. + """ + msg = 'Testing a report message' + logger = logging.getLogger() + + with caplog.at_level(logging.REPORT): # pylint: disable=no-member + logger.report(msg) - def test_logger(self): - """ - The python logging module is configured with a DbLogHandler - upon runtime, but we do not want any fired log messages to - crash when they reach the handler and no database is set yet - """ - # pylint: disable=no-self-use - logger = logging.getLogger('aiida') - logger.critical('Test critical log') + assert caplog.record_tuples == [(logger.name, logging.REPORT, msg)] # pylint: disable=no-member From 77e82a38a018b465e8ff75cad5a54e180b501db0 Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Tue, 17 Aug 2021 23:11:41 +0200 Subject: [PATCH 2/7] CLI: add `-v/--verbosity` option to all `verdi` commands The custom class used for the `verdi` command, `MostSimilarCommandGroup` is renamed to `VerdiCommandGroup`. The reason for this is that it adds new functionality, in addition to the existing functionality of suggesting command names in case the command is not found, to automatically add a `-v/--verbosity` option to all commands. This option provides a choice to set the log level for the AiiDA logger. This allows a user to control the verbosity of output that is emitted through the logging system in a way that is consistent across all `verdi` commands. Since through this method, the verbosity option is only added to sub commands, the top-level commands `verdi` itself won't inherit from this mechanism. Instead, the option is added manually and explicitly. Note that in this case the callback of the verbosity, which sets the log level on the profile, does not work, since for the top-level command the verbosity option is parsed before the profile is set on the context. That is why the processing of the verbosity is done explicitly in the command itself. By making the `-v` option available for all commands, the user can pass it at only level of the sub command stack. There is one side-effect however. Since `verdi` itself already had the `-v/--version` option, which clashes with the `-v/--verbosity`, the `-v` shorthand for the version option had to be removed. For consistency the `-h` shorthand for the help flag, which is the only other option that is also available across all commands, is also removed. Note that this change also fixes a bug that only made the command name suggestion functionality available at the top level but not for the sub commands. By overriding the ``group`` method to use the same class as the top-level `verdi` command, all subcommands should recursively have the same functionality. There is one subtlety where if the `cls` is set explicitly by one of the subcommands, that should be respected. It can't simply override it with `VerdiCommandGroup` because the subclass may have overridden the `get_command` as well with custom functionality. This puts the onus on the developer of subcommands with custom classes to make sure it inherits from `VerdiCommandGroup` if it is going to overwrite the `get_command` method and make sure it calls the super. Finally, the ``run_cli_command`` fixture, designed to help test CLI commands, needed to be updated to manually apply the ``VERBOSITY`` option. In testing, the sub command is direclty fetched from the module where it is defined, instead of getting it through the ``click`` API starting from the base ``verdi`` command. This means the functionality that applies the ``VERBOSITY`` option is not hit and so it has to be called manually. --- aiida/cmdline/commands/cmd_computer.py | 4 +- aiida/cmdline/commands/cmd_config.py | 8 +- aiida/cmdline/commands/cmd_verdi.py | 131 +++++++++++++---------- aiida/cmdline/params/options/__init__.py | 1 + aiida/cmdline/params/options/main.py | 50 ++++++++- aiida/cmdline/utils/pluginable.py | 7 +- docs/source/reference/command_line.rst | 4 +- docs/source/topics/cli.rst | 39 +++++++ tests/cmdline/commands/conftest.py | 7 ++ tests/cmdline/commands/test_verdi.py | 89 ++++++++------- 10 files changed, 233 insertions(+), 107 deletions(-) diff --git a/aiida/cmdline/commands/cmd_computer.py b/aiida/cmdline/commands/cmd_computer.py index ff0c88c343..a65c6adcad 100644 --- a/aiida/cmdline/commands/cmd_computer.py +++ b/aiida/cmdline/commands/cmd_computer.py @@ -14,7 +14,7 @@ import click import tabulate -from aiida.cmdline.commands.cmd_verdi import verdi +from aiida.cmdline.commands.cmd_verdi import verdi, VerdiCommandGroup from aiida.cmdline.params import options, arguments from aiida.cmdline.params.options.commands import computer as options_computer from aiida.cmdline.utils import echo @@ -538,7 +538,7 @@ def computer_delete(computer): echo.echo_success(f"Computer '{label}' deleted.") -class LazyConfigureGroup(click.Group): +class LazyConfigureGroup(VerdiCommandGroup): """A click group that will lazily load the subcommands for each transport plugin.""" def list_commands(self, ctx): diff --git a/aiida/cmdline/commands/cmd_config.py b/aiida/cmdline/commands/cmd_config.py index 74700772ff..17ae261e3d 100644 --- a/aiida/cmdline/commands/cmd_config.py +++ b/aiida/cmdline/commands/cmd_config.py @@ -12,17 +12,17 @@ import click -from aiida.cmdline.commands.cmd_verdi import verdi +from aiida.cmdline.commands.cmd_verdi import verdi, VerdiCommandGroup from aiida.cmdline.params import arguments from aiida.cmdline.utils import echo -class _DeprecateConfigCommandsGroup(click.Group): +class _DeprecateConfigCommandsGroup(VerdiCommandGroup): """Overloads the get_command with one that identifies deprecated commands.""" def get_command(self, ctx, cmd_name): """Override the default click.Group get_command with one that identifies deprecated commands.""" - cmd = click.Group.get_command(self, ctx, cmd_name) + cmd = super().get_command(ctx, cmd_name) if cmd is not None: return cmd @@ -37,7 +37,7 @@ def get_command(self, ctx, cmd_name): 'autofill.user.institution' ]: ctx.obj.deprecated_name = cmd_name - cmd = click.Group.get_command(self, ctx, '_deprecated') + cmd = super().get_command(ctx, '_deprecated') return cmd ctx.fail(f"'{cmd_name}' is not a verdi config command.") diff --git a/aiida/cmdline/commands/cmd_verdi.py b/aiida/cmdline/commands/cmd_verdi.py index 2cd299f164..20fcbd7c66 100644 --- a/aiida/cmdline/commands/cmd_verdi.py +++ b/aiida/cmdline/commands/cmd_verdi.py @@ -16,45 +16,73 @@ from aiida.cmdline.params import options, types GIU = ( - 'ABzY8%U8Kw0{@klyK?I~3`Ki?#qHQ&IIM|J;6yB`9_+{&w)p(JK}vokj-11jhve8xcx?dZ>+9nwrEF!x' - '*S>9A+EWYrR?6GA-u?jFa+et65GF@1+D{%8{C~xjt%>uVM4RTSS?j2M)XH%T#>M{K$lE2XGD`RS0T67213wbAs!SZmn+;(-m!>f(T@e%@oxd`yRBp9nu+9N`4xv8AS@O$CaQ;7FXzM=' - 'ug^$?3ta2551EDL`wK4|Cm%RnJdS#0UFwVweDkcfdNjtUv1N^iSQui#TL(q!FmIeKb!yW4' - '|L`@!@-4x6B6I^ptRdH+4o0ODM;1_f^}4@LMe@#_YHz0wQdq@d)@n)uYNtAb2OLo&fpBkct5{~3kbRag' - '^_5QG%qrTksHMXAYAQoz1#2wtHCy0}h?CJtzv&@Q?^9rd&02;isB7NJMMr7F@>$!ELj(sbwzIR4)rnch' - '=oVZrG;8)%R6}FUk*fv2O&!#ZA)$HloK9!es&4Eb+h=OIyWFha(8PPy9u?NqfkuPYg;GO1RVzBLX)7OR' - 'MM>1hEM`-96mGjJ+A!e-_}4X{M|4CkKE~uF4j+LW#6IsFa*_da_mLqzr)E<`%ikthkMO2>65cNMtpDE*VejqZV^MyewPJJAS*VM6jY;QY#g7gOKgPbFg{@;' - 'YDL6Gbxxr|2T&BQunB?PBetq?X>jW1hFF7' - '&>EaYkKYqIa_ld(Z@AJT+lJ(Pd;+?<&&M>A0agti19^z3n4Z6_WG}c~_+XHyJI_iau7+V$#YA$pJ~H)y' - 'HEVy1D?5^Sw`tb@{nnNNo=eSMZLf0>m^A@7f{y$nb_HJWgLRtZ?x2?*>SwM?JoQ>p|-1ZRU0#+{^' - 'UhK22+~oR9k7rh(GH9y|jm){jY9_xAI4N_EfU#4taTUXFY4a4l$v=N-+f+w&wuH;Z(6p6#=n8XwlZ' - ';*L&-rcL~T_vEm@#-Xi8&g06!MO+R(+9nwrEF!x*S>9A+EWYrR?6GA-u?jFa+et65GF@1+D{%' + '8{C~xjt%>uVM4RTSS?j2M)XH%T#>M{K$lE2XGD`RS0T67213wbAs!SZmn+;(-m!>f(T@e%@oxd`yRBp9nu+9N`4xv8AS@O$CaQ;7FXzM=ug^$?3ta2551EDL`wK4|Cm' + '%RnJdS#0UFwVweDkcfdNjtUv1N^iSQui#TL(q!FmIeKb!yW4|L`@!@-4x6' + 'B6I^ptRdH+4o0ODM;1_f^}4@LMe@#_YHz0wQdq@d)@n)uYNtAb2OLo&fpBkct5{~3kbRag^_5QG%qrTksHMXAYAQoz1#2wtHCy0}h?CJtzv&@Q?^9r' + 'd&02;isB7NJMMr7F@>$!ELj(sbwzIR4)rnch=oVZrG;8)%R6}FUk*fv2O&!#ZA)$HloK9!es&4Eb+h=OIyWFha(8PPy9u?NqfkuPYg;GO1RVzBLX)7' + 'ORMM>1hEM`-96mGjJ+A!e-_}4X{M|4CkKE~uF4j+LW#6IsFa*_da_mLqzr)E<`%ikthkMO2>65cNMtpDE*VejqZV^MyewPJJAS*VM6jY;QY' + '#g7gOKgPbFg{@;YDL6Gbxxr|2T&BQunB?PBetq?X>jW1hFF7&>EaYkKYqIa_ld(Z@AJT' + '+lJ(Pd;+?<&&M>A0agti19^z3n4Z6_WG}c~_+XHyJI_iau7+V$#YA$pJ~H)yHEVy1D?5^Sw`tb@{nnNNo=eSMZLf0>m^A@7f{y$nb_HJWgLRtZ?x2?*>SwM?JoQ>p|-1ZRU0#+{^UhK22+~o' + 'R9k7rh(GH9y|jm){jY9_xAI4N_EfU#4' + 'taTUXFY4a4l$v=N-+f+w&wuH;Z(6p6#=n8XwlZ;*L&-rcL~T_vEm@#-Xi8&g06!MO+R(`_. +The ``REPORT`` level is a log level that is defined and added by AiiDA that sits between the ``INFO`` and ``WARNING`` level, and is the default log level. + +The verbosity option is case-insensitive, i.e., ``--verbosity debug`` and ``--verbosity DEBUG`` are identical. +The option can be passed at any subcommand level, for example: + +.. code:: console + + verdi process list --verbosity debug + +is identical to + +.. code:: console + + verdi --verbosity debug process list + +When the option is specified multiple times, only the last value will be considered. +The default value for the logging level can be changed permanently through the configuration system. +For example, to set the default log level to ``WARNING``, + +.. code:: console + + verdi config set logging.aiida_loglevel WARNING + +This is identical to passing ``--verbosity WARNING`` manually to each invoked command. + +.. warning:: Setting the configuration option does not just affect the verbosity of the command line, but the logging for all of AiiDA. + For example, it also affects the verbosity of process reports, such as those of work chains. + If the log level is set above ``REPORT``, the reports of work chains will no longer be logged. + + .. _topics:cli:identifiers: Identifiers diff --git a/tests/cmdline/commands/conftest.py b/tests/cmdline/commands/conftest.py index baf4ad994c..2c3b30d99d 100644 --- a/tests/cmdline/commands/conftest.py +++ b/tests/cmdline/commands/conftest.py @@ -11,6 +11,8 @@ import click import pytest +from aiida.cmdline.commands.cmd_verdi import VerdiCommandGroup + @pytest.fixture def run_cli_command(): @@ -31,6 +33,11 @@ def _run_cli_command(command: click.Command, options: list = None, raises: bool """ import traceback + # We need to apply the ``VERBOSITY`` option. When invoked through the command line, this is done by the logic + # of the ``VerdiCommandGroup``, but when testing commands, the command is retrieved directly from the module + # which circumvents this machinery. + command = VerdiCommandGroup.add_verbosity_option(command) + runner = click.testing.CliRunner() result = runner.invoke(command, options or []) diff --git a/tests/cmdline/commands/test_verdi.py b/tests/cmdline/commands/test_verdi.py index ed3aa88204..09d0efada8 100644 --- a/tests/cmdline/commands/test_verdi.py +++ b/tests/cmdline/commands/test_verdi.py @@ -8,48 +8,61 @@ # For further information please visit http://www.aiida.net # ########################################################################### """Tests for `verdi`.""" -from click.testing import CliRunner +import click import pytest from aiida import get_version -from aiida.backends.testbase import AiidaTestCase from aiida.cmdline.commands import cmd_verdi @pytest.mark.usefixtures('config_with_profile') -class TestVerdi(AiidaTestCase): - """Tests for `verdi`.""" - - def setUp(self): - super().setUp() - self.cli_runner = CliRunner() - - def test_verdi_version(self): - """Regression test for #2238: verify that `verdi --version` prints the current version""" - result = self.cli_runner.invoke(cmd_verdi.verdi, ['--version']) - self.assertIsNone(result.exception, result.output) - self.assertIn(get_version(), result.output) - - def test_verdi_with_empty_profile_list(self): - """Regression test for #2424: verify that verdi remains operable even if profile list is empty""" - from aiida.manage.configuration import CONFIG - - # Run verdi command with updated CONFIG featuring an empty profile list - CONFIG.dictionary[CONFIG.KEY_PROFILES] = {} - result = self.cli_runner.invoke(cmd_verdi.verdi, []) - self.assertIsNone(result.exception, result.output) - - def test_invalid_cmd_matches(self): - """Test that verdi with an invalid command will return matches if somewhat close""" - result = self.cli_runner.invoke(cmd_verdi.verdi, ['usr']) - self.assertIn('is not a verdi command', result.output) - self.assertIn('The most similar commands are', result.output) - self.assertIn('user', result.output) - self.assertNotEqual(result.exit_code, 0) - - def test_invalid_cmd_no_matches(self): - """Test that verdi with an invalid command with no matches returns an appropriate message""" - result = self.cli_runner.invoke(cmd_verdi.verdi, ['foobar']) - self.assertIn('is not a verdi command', result.output) - self.assertIn('No similar commands found', result.output) - self.assertNotEqual(result.exit_code, 0) +def test_verdi_version(run_cli_command): + """Regression test for #2238: verify that `verdi --version` prints the current version""" + result = run_cli_command(cmd_verdi.verdi, ['--version']) + assert get_version() in result.output + + +@pytest.mark.usefixtures('config_with_profile') +def test_verdi_with_empty_profile_list(run_cli_command): + """Regression test for #2424: verify that verdi remains operable even if profile list is empty""" + from aiida.manage.configuration import CONFIG + + # Run verdi command with updated CONFIG featuring an empty profile list + CONFIG.dictionary[CONFIG.KEY_PROFILES] = {} + run_cli_command(cmd_verdi.verdi, []) + + +@pytest.mark.usefixtures('config_with_profile') +def test_invalid_cmd_matches(run_cli_command): + """Test that verdi with an invalid command will return matches if somewhat close""" + result = run_cli_command(cmd_verdi.verdi, ['usr'], raises=True) + assert 'is not a verdi command' in result.output + assert 'The most similar commands are' in result.output + assert 'user' in result.output + + +@pytest.mark.usefixtures('config_with_profile') +def test_invalid_cmd_no_matches(run_cli_command): + """Test that verdi with an invalid command with no matches returns an appropriate message""" + result = run_cli_command(cmd_verdi.verdi, ['foobar'], raises=True) + assert 'is not a verdi command' in result.output + assert 'No similar commands found' in result.output + + +def test_verbosity_options(): + """Recursively find all leaf commands of ``verdi`` and ensure they have the ``--verbosity`` option.""" + + def recursively_check_leaf_commands(ctx, command, leaf_commands): + """Recursively return the leaf commands of the given command.""" + try: + for subcommand in command.commands: + # We need to fetch the subcommand through the ``get_command``, because that is what the ``verdi`` + # command does when a subcommand is invoked on the command line. + recursively_check_leaf_commands(ctx, command.get_command(ctx, subcommand), leaf_commands) + except AttributeError: + # There are not subcommands so this is a leaf command, verify it has the verbosity option + assert 'verbosity' in [p.name for p in command.params], f'`{command.name} does not have verbosity option' + + leaf_commands = [] + ctx = click.Context(cmd_verdi.verdi) + recursively_check_leaf_commands(ctx, cmd_verdi.verdi, leaf_commands) From bd3d21425f930d45895dad6bb7859a5eeda8463f Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Tue, 17 Aug 2021 23:29:06 +0200 Subject: [PATCH 3/7] CLI: pipe `cmdline.utils.echo` functions through logger The utility functions in the `aiida.cmdline.utils.echo` module are designed to be used in CLI commands to print output to stdout or stderr streams. This is now changed to instead log the message to the `aiida.cmdline` logger. This logger was added in the previous commit. The reason is that the verbosity of this logger can be controlled through the `--verbosity` option that was added to all commands. To match the available log levels, the `echo_report` method was added, which, as the name suggests, logs messages at the `REPORT` level. The `echo` method, which was used to simply print a message without a particular prefix, logs also at the `REPORT` level, since that is the default log level in AiiDA. This makes sense since when developers used `echo` they probably want it to show by default, but it is not a warning and so should not be shown when the log level is set to `WARNING`. The `echo_highlight` method has been removed as it was essentially a thin wrapper around functionality that is already provided by `echo`. Instead of using the different `color` argument, one can use `echo` using the `fg` argument which is more consistent with the interface from `click`'s methods which are wrapped. All remaining direct calls of `click.echo` and `click.secho` in the code have been replaced by the relevant `cmdline.utils.echo` command to make sure that they are properly piped through the logging system and respect the `--verbosity` option. Note that a few instances remain, for example, in the `verdi completioncommand` command, but this is intentional as they should not be influenced by the `--verbosity` option. --- .../djsite/db/migrations/0024_dblog_update.py | 12 +- .../versions/041a79fc615f_dblog_cleaning.py | 11 +- aiida/cmdline/commands/cmd_code.py | 2 +- aiida/cmdline/commands/cmd_computer.py | 12 +- aiida/cmdline/commands/cmd_daemon.py | 12 +- aiida/cmdline/commands/cmd_data/cmd_remote.py | 12 +- aiida/cmdline/commands/cmd_status.py | 4 +- aiida/cmdline/utils/common.py | 28 ++- aiida/cmdline/utils/daemon.py | 13 +- aiida/cmdline/utils/echo.py | 208 ++++++++++-------- aiida/cmdline/utils/repository.py | 4 +- aiida/common/log.py | 32 ++- pyproject.toml | 2 + 13 files changed, 199 insertions(+), 153 deletions(-) diff --git a/aiida/backends/djsite/db/migrations/0024_dblog_update.py b/aiida/backends/djsite/db/migrations/0024_dblog_update.py index daf92ec6b2..eebfebe06c 100644 --- a/aiida/backends/djsite/db/migrations/0024_dblog_update.py +++ b/aiida/backends/djsite/db/migrations/0024_dblog_update.py @@ -10,7 +10,6 @@ # Generated by Django 1.11.16 on 2018-12-21 10:56 # pylint: disable=invalid-name """Migration for the update of the DbLog table. Addition of uuids""" - import sys import click @@ -20,6 +19,7 @@ from aiida.backends.djsite.db.migrations import upgrade_schema_version from aiida.backends.general.migrations.utils import dumps_json from aiida.common.utils import get_new_uuid +from aiida.cmdline.utils import echo from aiida.manage import configuration REVISION = '1.0.24' @@ -157,11 +157,11 @@ def export_and_clean_workflow_logs(apps, schema_editor): return if not configuration.PROFILE.is_test_profile: - click.echo( + echo.echo_warning( 'We found {} log records that correspond to legacy workflows and {} log records to correspond ' 'to an unknown entity.'.format(lwf_number, other_number) ) - click.echo( + echo.echo_warning( 'These records will be removed from the database and exported to JSON files to the current directory).' ) proceed = click.confirm('Would you like to proceed?', default=True) @@ -181,7 +181,7 @@ def export_and_clean_workflow_logs(apps, schema_editor): # If delete_on_close is False, we are running for the user and add additional message of file location if not delete_on_close: - click.echo(f'Exported legacy workflow logs to {filename}') + echo.echo(f'Exported legacy workflow logs to {filename}') # Now delete the records DbLog.objects.filter(objname__startswith=leg_workflow_prefix).delete() @@ -205,7 +205,7 @@ def export_and_clean_workflow_logs(apps, schema_editor): # If delete_on_close is False, we are running for the user and add additional message of file location if not delete_on_close: - click.echo(f'Exported unexpected entity logs to {filename}') + echo.echo(f'Exported unexpected entity logs to {filename}') # Now delete the records DbLog.objects.exclude(objname__startswith=node_prefix).exclude(objname__startswith=leg_workflow_prefix).delete() @@ -229,7 +229,7 @@ def export_and_clean_workflow_logs(apps, schema_editor): # If delete_on_close is False, we are running for the user and add additional message of file location if not delete_on_close: - click.echo('Exported entity logs that don\'t correspond to nodes to {}'.format(filename)) + echo.echo('Exported entity logs that don\'t correspond to nodes to {}'.format(filename)) # Now delete the records with schema_editor.connection.cursor() as cursor: diff --git a/aiida/backends/sqlalchemy/migrations/versions/041a79fc615f_dblog_cleaning.py b/aiida/backends/sqlalchemy/migrations/versions/041a79fc615f_dblog_cleaning.py index 33e45372b3..952bed3cac 100644 --- a/aiida/backends/sqlalchemy/migrations/versions/041a79fc615f_dblog_cleaning.py +++ b/aiida/backends/sqlalchemy/migrations/versions/041a79fc615f_dblog_cleaning.py @@ -27,6 +27,7 @@ from sqlalchemy.sql import text from aiida.backends.general.migrations.utils import dumps_json +from aiida.cmdline.utils import echo from aiida.manage import configuration # revision identifiers, used by Alembic. @@ -152,11 +153,11 @@ def export_and_clean_workflow_logs(connection): return if not configuration.PROFILE.is_test_profile: - click.echo( + echo.echo_warning( 'We found {} log records that correspond to legacy workflows and {} log records to correspond ' 'to an unknown entity.'.format(lwf_no_number, other_number) ) - click.echo( + echo.echo_warning( 'These records will be removed from the database and exported to JSON files to the current directory).' ) proceed = click.confirm('Would you like to proceed?', default=True) @@ -178,7 +179,7 @@ def export_and_clean_workflow_logs(connection): # If delete_on_close is False, we are running for the user and add additional message of file location if not delete_on_close: - click.echo(f'Exported legacy workflow logs to {filename}') + echo.echo(f'Exported legacy workflow logs to {filename}') # Now delete the records connection.execute( @@ -203,7 +204,7 @@ def export_and_clean_workflow_logs(connection): # If delete_on_close is False, we are running for the user and add additional message of file location if not delete_on_close: - click.echo(f'Exported unexpected entity logs to {filename}') + echo.echo(f'Exported unexpected entity logs to {filename}') # Now delete the records connection.execute( @@ -228,7 +229,7 @@ def export_and_clean_workflow_logs(connection): # If delete_on_close is False, we are running for the user and add additional message of file location if not delete_on_close: - click.echo('Exported entity logs that don\'t correspond to nodes to {}'.format(filename)) + echo.echo('Exported entity logs that don\'t correspond to nodes to {}'.format(filename)) # Now delete the records connection.execute( diff --git a/aiida/cmdline/commands/cmd_code.py b/aiida/cmdline/commands/cmd_code.py index cca99390f5..ee1662e1d1 100644 --- a/aiida/cmdline/commands/cmd_code.py +++ b/aiida/cmdline/commands/cmd_code.py @@ -177,7 +177,7 @@ def show(code, verbose): if verbose: table.append(['Calculations', len(code.get_outgoing().all())]) - click.echo(tabulate.tabulate(table)) + echo.echo(tabulate.tabulate(table)) @verdi_code.command() diff --git a/aiida/cmdline/commands/cmd_computer.py b/aiida/cmdline/commands/cmd_computer.py index a65c6adcad..66f1125e61 100644 --- a/aiida/cmdline/commands/cmd_computer.py +++ b/aiida/cmdline/commands/cmd_computer.py @@ -463,7 +463,7 @@ def computer_test(user, print_traceback, computer): with transport: num_tests += 1 - echo.echo_highlight('[OK]', color='success') + echo.echo('[OK]', fg='green') scheduler.set_transport(transport) @@ -486,16 +486,16 @@ def computer_test(user, print_traceback, computer): if not success: num_failures += 1 if message: - echo.echo_highlight('[Failed]: ', color='error', nl=False) + echo.echo('[Failed]: ', fg='red', nl=False) echo.echo(message) else: - echo.echo_highlight('[Failed]', color='error') + echo.echo('[Failed]', fg='red') else: if message: - echo.echo_highlight('[OK]: ', color='success', nl=False) + echo.echo('[OK]: ', fg='green', nl=False) echo.echo(message) else: - echo.echo_highlight('[OK]', color='success') + echo.echo('[OK]', fg='green') if num_failures: echo.echo_warning(f'{num_failures} out of {num_tests} tests failed') @@ -503,7 +503,7 @@ def computer_test(user, print_traceback, computer): echo.echo_success(f'all {num_tests} tests succeeded') except Exception as exception: # pylint:disable=broad-except - echo.echo_highlight('[FAILED]: ', color='error', nl=False) + echo.echo('[FAILED]: ', fg='red', nl=False) message = 'Error while trying to connect to the computer' if print_traceback: diff --git a/aiida/cmdline/commands/cmd_daemon.py b/aiida/cmdline/commands/cmd_daemon.py index bfe8e05294..eac5946f1a 100644 --- a/aiida/cmdline/commands/cmd_daemon.py +++ b/aiida/cmdline/commands/cmd_daemon.py @@ -72,7 +72,7 @@ def start(foreground, number): currenv = get_env_with_venv_bin() subprocess.check_output(command, env=currenv, stderr=subprocess.STDOUT) # pylint: disable=unexpected-keyword-arg except subprocess.CalledProcessError as exception: - click.secho('FAILED', fg='red', bold=True) + echo.echo('FAILED', fg='red', bold=True) echo.echo_critical(str(exception)) # We add a small timeout to give the pid-file a chance to be created @@ -105,8 +105,8 @@ def status(all_profiles): for profile in profiles: client = get_daemon_client(profile.name) delete_stale_pid_file(client) - click.secho('Profile: ', fg='red', bold=True, nl=False) - click.secho(f'{profile.name}', bold=True) + echo.echo('Profile: ', fg='red', bold=True, nl=False) + echo.echo(f'{profile.name}', bold=True) result = get_daemon_status(client) echo.echo(result) daemons_running.append(client.is_daemon_running) @@ -185,8 +185,8 @@ def stop(no_wait, all_profiles): client = get_daemon_client(profile.name) - click.secho('Profile: ', fg='red', bold=True, nl=False) - click.secho(f'{profile.name}', bold=True) + echo.echo('Profile: ', fg='red', bold=True, nl=False) + echo.echo(f'{profile.name}', bold=True) if not client.is_daemon_running: echo.echo('Daemon was not running') @@ -205,7 +205,7 @@ def stop(no_wait, all_profiles): if wait: if response['status'] == client.DAEMON_ERROR_NOT_RUNNING: - click.echo('The daemon was not running.') + echo.echo('The daemon was not running.') else: retcode = print_client_response_status(response) if retcode: diff --git a/aiida/cmdline/commands/cmd_data/cmd_remote.py b/aiida/cmdline/commands/cmd_data/cmd_remote.py index 9b2eef2770..4180cb4dae 100644 --- a/aiida/cmdline/commands/cmd_data/cmd_remote.py +++ b/aiida/cmdline/commands/cmd_data/cmd_remote.py @@ -48,11 +48,11 @@ def remote_ls(ls_long, path, datum): stat.filemode(metadata['attributes'].st_mode), metadata['attributes'].st_size, mtime.strftime('%d %b %Y %H:%M') ) - click.echo(pre_line, nl=False) + echo.echo(pre_line, nl=False) if metadata['isdir']: - click.echo(click.style(metadata['name'], fg='blue')) + echo.echo(metadata['name'], fg='blue') else: - click.echo(metadata['name']) + echo.echo(metadata['name']) @remote.command('cat') @@ -83,7 +83,5 @@ def remote_cat(datum, path): @arguments.DATUM(type=types.DataParamType(sub_classes=('aiida.data:remote',))) def remote_show(datum): """Show information for a RemoteData object.""" - click.echo('- Remote computer name:') - click.echo(f' {datum.computer.label}') - click.echo('- Remote folder full path:') - click.echo(f' {datum.get_remote_path()}') + echo.echo(f'- Remote computer name: {datum.computer.label}') + echo.echo(f'- Remote folder full path: {datum.get_remote_path()}') diff --git a/aiida/cmdline/commands/cmd_status.py b/aiida/cmdline/commands/cmd_status.py index f2750e4f9c..7ecc993934 100644 --- a/aiida/cmdline/commands/cmd_status.py +++ b/aiida/cmdline/commands/cmd_status.py @@ -155,8 +155,8 @@ def print_status(status, service, msg='', exception=None, print_traceback=False) :param msg: message string """ symbol = STATUS_SYMBOLS[status] - click.secho(f" {symbol['string']} ", fg=symbol['color'], nl=False) - click.secho(f"{service + ':':12s} {msg}") + echo.echo(f" {symbol['string']} ", fg=symbol['color'], nl=False) + echo.echo(f"{service + ':':12s} {msg}") if exception is not None: echo.echo_error(f'{type(exception).__name__}: {exception}') diff --git a/aiida/cmdline/utils/common.py b/aiida/cmdline/utils/common.py index 66e5821446..279e09b8bc 100644 --- a/aiida/cmdline/utils/common.py +++ b/aiida/cmdline/utils/common.py @@ -8,14 +8,13 @@ # For further information please visit http://www.aiida.net # ########################################################################### """Common utility functions for command line commands.""" -# pylint: disable=import-error - import os import sys -import click from tabulate import tabulate +from . import echo + def get_env_with_venv_bin(): """Create a clone of the current running environment with the AIIDA_PATH variable set directory of the config.""" @@ -383,10 +382,10 @@ def print_process_info(process): if not docstring: docstring = ['No description available'] - click.secho('Description:\n', fg='red', bold=True) + echo.echo('Description:\n', fg='red', bold=True) for line in docstring: - click.echo(f' {line.lstrip()}') - click.echo() + echo.echo(f' {line.lstrip()}') + echo.echo() print_process_spec(process.spec()) @@ -426,26 +425,26 @@ def build_entries(ports): max_width_type = max([len(entry[2]) for entry in inputs + outputs]) + 2 if process_spec.inputs: - click.secho('Inputs:', fg='red', bold=True) + echo.echo('Inputs:', fg='red', bold=True) for entry in inputs: if entry[1] == 'required': - click.secho(template.format(*entry, width_name=max_width_name, width_type=max_width_type), bold=True) + echo.echo(template.format(*entry, width_name=max_width_name, width_type=max_width_type), bold=True) else: - click.secho(template.format(*entry, width_name=max_width_name, width_type=max_width_type)) + echo.echo(template.format(*entry, width_name=max_width_name, width_type=max_width_type)) if process_spec.outputs: - click.secho('Outputs:', fg='red', bold=True) + echo.echo('Outputs:', fg='red', bold=True) for entry in outputs: if entry[1] == 'required': - click.secho(template.format(*entry, width_name=max_width_name, width_type=max_width_type), bold=True) + echo.echo(template.format(*entry, width_name=max_width_name, width_type=max_width_type), bold=True) else: - click.secho(template.format(*entry, width_name=max_width_name, width_type=max_width_type)) + echo.echo(template.format(*entry, width_name=max_width_name, width_type=max_width_type)) if process_spec.exit_codes: - click.secho('Exit codes:', fg='red', bold=True) + echo.echo('Exit codes:', fg='red', bold=True) for exit_code in sorted(process_spec.exit_codes.values(), key=lambda exit_code: exit_code.status): message = exit_code.message.capitalize() - click.secho('{:>{width_name}d}: {}'.format(exit_code.status, message, width_name=max_width_name)) + echo.echo('{:>{width_name}d}: {}'.format(exit_code.status, message, width_name=max_width_name)) def get_num_workers(): @@ -483,7 +482,6 @@ def check_worker_load(active_slots): :param active_slots: the number of currently active worker slots """ - from aiida.cmdline.utils import echo from aiida.common.exceptions import CircusCallError from aiida.manage.configuration import get_config diff --git a/aiida/cmdline/utils/daemon.py b/aiida/cmdline/utils/daemon.py index 56caf564bb..f78281c6ad 100644 --- a/aiida/cmdline/utils/daemon.py +++ b/aiida/cmdline/utils/daemon.py @@ -8,7 +8,6 @@ # For further information please visit http://www.aiida.net # ########################################################################### """Utility functions for command line commands related to the daemon.""" -import click from tabulate import tabulate from aiida.cmdline.utils import echo @@ -29,20 +28,20 @@ def print_client_response_status(response): return 1 if response['status'] == 'active': - click.secho('RUNNING', fg='green', bold=True) + echo.echo('RUNNING', fg='green', bold=True) return 0 if response['status'] == 'ok': - click.secho('OK', fg='green', bold=True) + echo.echo('OK', fg='green', bold=True) return 0 if response['status'] == DaemonClient.DAEMON_ERROR_NOT_RUNNING: - click.secho('FAILED', fg='red', bold=True) - click.echo('Try to run \'verdi daemon start --foreground\' to potentially see the exception') + echo.echo('FAILED', fg='red', bold=True) + echo.echo('Try to run `verdi daemon start --foreground` to potentially see the exception') return 2 if response['status'] == DaemonClient.DAEMON_ERROR_TIMEOUT: - click.secho('TIMEOUT', fg='red', bold=True) + echo.echo('TIMEOUT', fg='red', bold=True) return 3 # Unknown status, I will consider it as failed - click.echo(response['status']) + echo.echo_critical(response['status']) return -1 diff --git a/aiida/cmdline/utils/echo.py b/aiida/cmdline/utils/echo.py index 248a01a2db..0e8c5ba53b 100644 --- a/aiida/cmdline/utils/echo.py +++ b/aiida/cmdline/utils/echo.py @@ -7,22 +7,22 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### -""" Convenience functions for printing output from verdi commands """ - -from enum import IntEnum -from collections import OrderedDict +"""Convenience functions for logging output from ``verdi`` commands.""" +import collections +import enum import sys import yaml import click -__all__ = ( - 'echo', 'echo_info', 'echo_success', 'echo_warning', 'echo_error', 'echo_critical', 'echo_highlight', - 'echo_dictionary' -) +from aiida.common.log import AIIDA_LOGGER + +CMDLINE_LOGGER = AIIDA_LOGGER.getChild('cmdline') +__all__ = ('echo', 'echo_info', 'echo_success', 'echo_warning', 'echo_error', 'echo_critical', 'echo_dictionary') -class ExitCode(IntEnum): + +class ExitCode(enum.IntEnum): """Exit codes for the verdi command line.""" CRITICAL = 1 DEPRECATED = 80 @@ -33,138 +33,157 @@ class ExitCode(IntEnum): COLORS = { 'success': 'green', 'highlight': 'green', + 'debug': 'white', 'info': 'blue', + 'report': 'blue', 'warning': 'bright_yellow', 'error': 'red', 'critical': 'red', 'deprecated': 'red', } -BOLD = True # whether colors are used together with 'bold' -# pylint: disable=invalid-name -def echo(message, bold=False, nl=True, err=False): - """ - Print a normal message through click's echo function to stdout +def echo(message: str, fg: str = None, bold: bool = False, nl: bool = True, err: bool = False) -> None: + """Log a message to the cmdline logger. - :param message: the string representing the message to print - :param bold: whether to print the message in bold - :param nl: whether to print a newline at the end of the message - :param err: whether to print to stderr + .. note:: The message will be logged at the ``REPORT`` level. + + :param message: the message to log. + :param fg: if provided this will become the foreground color. + :param bold: whether to print the messaformat bold. + :param nl: whether to print a newlineaddhe end of the message. + :param err: whether to log to stderr. """ - click.secho(message, bold=bold, nl=nl, err=err) + message = click.style(message, fg=fg, bold=bold) + CMDLINE_LOGGER.report(message, extra=dict(nl=nl, err=err)) -def echo_info(message, bold=False, nl=True, err=False): - """ - Print an info message through click's echo function to stdout, prefixed with 'Info:' +def echo_debug(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: + """Log a debug message to the cmdline logger, prefixed with 'Debug:' - :param message: the string representing the message to print - :param bold: whether to print the message in bold - :param nl: whether to print a newline at the end of the message - :param err: whether to print to stderr + :param message: the message to log. + :param bold: whether to format the message in bold. + :param nl: whether to add a newline at the end of the message. + :param err: whether to log to stderr. + :param prefix: whether the message should be prefixed with a colored version of the log level. """ - click.secho('Info: ', fg=COLORS['info'], bold=True, nl=False, err=err) - click.secho(message, bold=bold, nl=nl, err=err) + prefix = click.style('Debug: ', fg=COLORS['debug'], bold=True) if prefix else '' + message = click.style(message, bold=bold) + CMDLINE_LOGGER.debug(prefix + message, extra=dict(nl=nl, err=err)) -def echo_success(message, bold=False, nl=True, err=False): - """ - Print a success message through click's echo function to stdout, prefixed with 'Success:' +def echo_info(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: + """Log an info message to the cmdline logger, prefixed with 'Info:' - :param message: the string representing the message to print - :param bold: whether to print the message in bold - include a newline character - :param nl: whether to print a newline at the end of the message - :param err: whether to print to stderr + :param message: the message to log. + :param bold: whether to format the message in bold. + :param nl: whether to add a newline at the end of the message. + :param err: whether to log to stderr. + :param prefix: whether the message should be prefixed with a colored version of the log level. """ - click.secho('Success: ', fg=COLORS['success'], bold=True, nl=False, err=err) - click.secho(message, bold=bold, nl=nl, err=err) + prefix = click.style('Info: ', fg=COLORS['info'], bold=True) if prefix else '' + message = click.style(message, bold=bold) + CMDLINE_LOGGER.info(prefix + message, extra=dict(nl=nl, err=err)) -def echo_warning(message, bold=False, nl=True, err=False): - """ - Print a warning message through click's echo function to stdout, prefixed with 'Warning:' +def echo_report(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: + """Log an report message to the cmdline logger, prefixed with 'Report:' - :param message: the string representing the message to print - :param bold: whether to print the message in bold - :param nl: whether to print a newline at the end of the message - :param err: whether to print to stderr + :param message: the message to log. + :param bold: whether to format the message in bold. + :param nl: whether to add a newline at the end of the message. + :param err: whether to log to stderr. + :param prefix: whether the message should be prefixed with a colored version of the log level. """ - click.secho('Warning: ', fg=COLORS['warning'], bold=True, nl=False, err=err) - click.secho(message, bold=bold, nl=nl, err=err) + prefix = click.style('Report: ', fg=COLORS['report'], bold=True) if prefix else '' + message = click.style(message, bold=bold) + CMDLINE_LOGGER.report(prefix + message, extra=dict(nl=nl, err=err)) -def echo_error(message, bold=False, nl=True, err=True): - """ - Print an error message through click's echo function to stdout, prefixed with 'Error:' +def echo_success(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: + """Log a success message to the cmdline logger, prefixed with 'Success:' - :param message: the string representing the message to print - :param bold: whether to print the message in bold - :param nl: whether to print a newline at the end of the message - :param err: whether to print to stderr + :param message: the message to log. + :param bold: whether to format the message in bold. + :param nl: whether to add a newline at the end of the message. + :param err: whether to log to stderr. + :param prefix: whether the message should be prefixed with a colored version of the log level. """ - click.secho('Error: ', fg=COLORS['error'], bold=True, nl=False, err=err) - click.secho(message, bold=bold, nl=nl, err=err) + prefix = click.style('Success: ', fg=COLORS['success'], bold=True) if prefix else '' + message = click.style(message, bold=bold) + CMDLINE_LOGGER.report(prefix + message, extra=dict(nl=nl, err=err)) + +def echo_warning(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: + """Log a warning message to the cmdline logger, prefixed with 'Warning:' -def echo_critical(message, bold=False, nl=True, err=True): + :param message: the message to log. + :param bold: whether to format the message in bold. + :param nl: whether to add a newline at the end of the message. + :param err: whether to log to stderr. + :param prefix: whether the message should be prefixed with a colored version of the log level. """ - Print an error message through click's echo function to stdout, prefixed with 'Critical:' - and then calls sys.exit with the given exit_status. + prefix = click.style('Warning: ', fg=COLORS['warning'], bold=True) if prefix else '' + message = click.style(message, bold=bold) + CMDLINE_LOGGER.warning(prefix + message, extra=dict(nl=nl, err=err)) + - This should be used to print messages for errors that cannot be recovered - from and so the script should be directly terminated with a non-zero exit - status to indicate that the command failed +def echo_error(message: str, bold: bool = False, nl: bool = True, err: bool = True, prefix: bool = True) -> None: + """Log an error message to the cmdline logger, prefixed with 'Error:' - :param message: the string representing the message to print - :param bold: whether to print the message in bold - :param nl: whether to print a newline at the end of the message - :param err: whether to print to stderr + :param message: the message to log. + :param bold: whether to format the message in bold. + :param nl: whether to add a newline at the end of the message. + :param err: whether to log to stderr. + :param prefix: whether the message should be prefixed with a colored version of the log level. """ - click.secho('Critical: ', fg=COLORS['critical'], bold=True, nl=False, err=err) - click.secho(message, bold=bold, nl=nl, err=err) - sys.exit(ExitCode.CRITICAL) + prefix = click.style('Error: ', fg=COLORS['error'], bold=True) if prefix else '' + message = click.style(message, bold=bold) + CMDLINE_LOGGER.error(prefix + message, extra=dict(nl=nl, err=err)) -def echo_highlight(message, nl=True, bold=True, color='highlight'): - """ - Print a highlighted message to stdout +def echo_critical(message: str, bold: bool = False, nl: bool = True, err: bool = True, prefix: bool = True) -> None: + """Log a critical error message to the cmdline logger, prefixed with 'Critical:' and exit with ``exit_status``. - :param message: the string representing the message to print - :param bold: whether to print the message in bold - :param nl: whether to print a newline at the end of the message - :param color: a color from COLORS + This should be used to print messages for errors that cannot be recovered from and so the script should be directly + terminated with a non-zero exit status to indicate that the command failed. + + :param message: the message to log. + :param bold: whether to format the message in bold. + :param nl: whether to add a newline at the end of the message. + :param err: whether to log to stderr. + :param prefix: whether the message should be prefixed with a colored version of the log level. """ - click.secho(message, bold=bold, nl=nl, fg=COLORS[color]) + prefix = click.style('Critical: ', fg=COLORS['critical'], bold=True) if prefix else '' + message = click.style(message, bold=bold) + CMDLINE_LOGGER.critical(prefix + message, extra=dict(nl=nl, err=err)) + sys.exit(ExitCode.CRITICAL) -# pylint: disable=redefined-builtin -def echo_deprecated(message, bold=False, nl=True, err=True, exit=False): - """ - Print an error message through click's echo function to stdout, prefixed with 'Deprecated:' - and then calls sys.exit with the given exit_status. +def echo_deprecated(message: str, bold: bool = False, nl: bool = True, err: bool = True, exit: bool = False) -> None: + """Log an error message to the cmdline logger, prefixed with 'Deprecated:' exiting with the given ``exit_status``. This should be used to indicate deprecated commands. - :param message: the string representing the message to print - :param bold: whether to print the message in bold - :param nl: whether to print a newline at the end of the message - :param err: whether to print to stderr + :param message: the message to log. + :param bold: whether to format the message in bold. + :param nl: whether to add a newline at the end of the message. + :param err: whether to log to stderr. :param exit: whether to exit after printing the message """ - click.secho('Deprecated: ', fg=COLORS['deprecated'], bold=True, nl=False, err=err) - click.secho(message, bold=bold, nl=nl, err=err) + # pylint: disable=redefined-builtin + prefix = click.style('Deprecated: ', fg=COLORS['deprecated'], bold=True) + echo_warning(prefix + message, bold=bold, nl=nl, err=err, prefix=False) if exit: sys.exit(ExitCode.DEPRECATED) def echo_formatted_list(collection, attributes, sort=None, highlight=None, hide=None): - """Print a collection of entries as a formatted list, one entry per line. + """Log a collection of entries as a formatted list, one entry per line. :param collection: a list of objects - :param attributes: a list of attributes to print for each entry in the collection + :param attributes: a list of attributes to log for each entry in the collection :param sort: optional lambda to sort the collection :param highlight: optional lambda to highlight an entry in the collection if it returns True :param hide: optional lambda to skip an entry if it returns True @@ -182,9 +201,9 @@ def echo_formatted_list(collection, attributes, sort=None, highlight=None, hide= values = [getattr(entry, attribute) for attribute in attributes] if highlight and highlight(entry): - click.secho(template.format(symbol='*', *values), fg=COLORS['highlight']) + echo(click.style(template.format(symbol='*', *values), fg=COLORS['highlight'])) else: - click.secho(template.format(symbol=' ', *values)) + echo(click.style(template.format(symbol=' ', *values))) def _format_dictionary_json_date(dictionary, sort_keys=True): @@ -214,14 +233,13 @@ def _format_yaml_expanded(dictionary, sort_keys=True): return yaml.dump(dictionary, sort_keys=sort_keys, default_flow_style=False) -VALID_DICT_FORMATS_MAPPING = OrderedDict( +VALID_DICT_FORMATS_MAPPING = collections.OrderedDict( (('json+date', _format_dictionary_json_date), ('yaml', _format_yaml), ('yaml_expanded', _format_yaml_expanded)) ) def echo_dictionary(dictionary, fmt='json+date', sort_keys=True): - """ - Print the given dictionary to stdout in the given format + """Log the given dictionary to stdout in the given format :param dictionary: the dictionary :param fmt: the format to use for printing diff --git a/aiida/cmdline/utils/repository.py b/aiida/cmdline/utils/repository.py index ffe9dbcf03..c507488fb8 100644 --- a/aiida/cmdline/utils/repository.py +++ b/aiida/cmdline/utils/repository.py @@ -8,7 +8,7 @@ # For further information please visit http://www.aiida.net # ########################################################################### """Utility functions for command line commands operating on the repository.""" -import click +from aiida.cmdline.utils import echo def list_repository_contents(node, path, color): @@ -22,4 +22,4 @@ def list_repository_contents(node, path, color): for entry in node.list_objects(path): bold = bool(entry.file_type == FileType.DIRECTORY) - click.secho(entry.name, bold=bold, fg='blue' if color and entry.file_type == FileType.DIRECTORY else None) + echo.echo(entry.name, bold=bold, fg='blue' if color and entry.file_type == FileType.DIRECTORY else None) diff --git a/aiida/common/log.py b/aiida/common/log.py index e1c5429254..433fcd6bbf 100644 --- a/aiida/common/log.py +++ b/aiida/common/log.py @@ -48,6 +48,30 @@ def report(self, msg, *args, **kwargs): AIIDA_LOGGER = logging.getLogger('aiida') +class ClickHandler(logging.Handler): + """Handler for writing to the console using click.""" + + def emit(self, record): + """Emit log record via click. + Can make use of special attributes 'nl' (whether to add newline) and 'err' (whether to print to stderr), which + can be set via the 'extra' dictionary parameter of the logging methods. + """ + import click + try: + msg = self.format(record) + try: + nl = record.nl + except AttributeError: + nl = True + try: + err = record.err + except AttributeError: + err = False + click.echo(msg, err=err, nl=nl) + except Exception: # pylint: disable=broad-except + self.handleError(record) + + # The default logging dictionary for AiiDA that can be used in conjunction # with the config.dictConfig method of python's logging module def get_logging_config(): @@ -68,10 +92,12 @@ def get_logging_config(): }, 'handlers': { 'console': { - 'level': 'DEBUG', 'class': 'logging.StreamHandler', 'formatter': 'halfverbose', }, + 'click': { + 'class': 'aiida.common.log.ClickHandler', + } }, 'loggers': { 'aiida': { @@ -79,6 +105,10 @@ def get_logging_config(): 'level': lambda: get_config_option('logging.aiida_loglevel'), 'propagate': False, }, + 'aiida.cmdline': { + 'handlers': ['click'], + 'propagate': False, + }, 'plumpy': { 'handlers': ['console'], 'level': lambda: get_config_option('logging.plumpy_loglevel'), diff --git a/pyproject.toml b/pyproject.toml index 4d848496b8..eab42b60b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,6 +40,8 @@ good-names = [ "j", "k", "pk", + "fg", + "nl", "TemplatereplacerCalculation", "ArithmeticAddCalculation", "MultiplyAddWorkChain" From 2b9725d48a4235eef95c53c40ffe3dc045744ed7 Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Wed, 18 Aug 2021 08:42:08 +0200 Subject: [PATCH 4/7] CLI: replace `echo_info` by `echo_report` In the previous commit, the `echo` utility functions were changed to go through the logger system, instead of directly using `click.echo` to print to stdout and stderr. Since the default log level is `REPORT` in AiiDA, messages logged with `echo_info` will no longer be shown by default, unless the log level is set to `INFO` or `DEBUG`. To return to the previous behavior, all `echo_info` instances are replaced by the new `echo_report`, unless the existing message really corresponds to info that is a bit verbose and can be hidden by default. Calls to `info` for the `MIGRATE_LOGGER`, `EXPORT_LOGGER` and the `DELETE_LOGGER` are changed to `report`. The current implementation using these loggers assumed that the default log level would be `info` and so these messages should be shown by default. This choice was made since this functionality is mostly called through the command line. With the current change where the default is now `report` these messages would no longer show up by default. --- .../0032_remove_legacy_workflows.py | 2 +- .../1b8ed3425af9_remove_legacy_workflows.py | 2 +- aiida/cmdline/commands/cmd_archive.py | 10 +++---- aiida/cmdline/commands/cmd_calcjob.py | 2 +- aiida/cmdline/commands/cmd_computer.py | 28 +++++++++++-------- aiida/cmdline/commands/cmd_data/cmd_show.py | 8 +++--- aiida/cmdline/commands/cmd_database.py | 2 +- aiida/cmdline/commands/cmd_group.py | 8 +++--- aiida/cmdline/commands/cmd_node.py | 4 +-- aiida/cmdline/commands/cmd_plugin.py | 6 ++-- aiida/cmdline/commands/cmd_process.py | 6 ++-- aiida/cmdline/commands/cmd_profile.py | 8 +++--- aiida/cmdline/commands/cmd_setup.py | 2 +- aiida/cmdline/commands/cmd_status.py | 2 +- aiida/cmdline/params/options/interactive.py | 6 ++-- aiida/cmdline/utils/common.py | 12 ++++---- aiida/manage/external/postgres.py | 4 +-- aiida/tools/graph/deletions.py | 10 +++---- aiida/tools/importexport/archive/migrators.py | 12 ++++---- aiida/tools/importexport/dbexport/main.py | 8 +++--- aiida/transports/cli.py | 4 +-- tests/cmdline/commands/test_profile.py | 4 +-- .../params/options/test_interactive.py | 2 +- 23 files changed, 78 insertions(+), 74 deletions(-) diff --git a/aiida/backends/djsite/db/migrations/0032_remove_legacy_workflows.py b/aiida/backends/djsite/db/migrations/0032_remove_legacy_workflows.py index 71ceb5b2d6..85fad93682 100644 --- a/aiida/backends/djsite/db/migrations/0032_remove_legacy_workflows.py +++ b/aiida/backends/djsite/db/migrations/0032_remove_legacy_workflows.py @@ -68,7 +68,7 @@ def export_workflow_data(apps, _): # If delete_on_close is False, we are running for the user and add additional message of file location if not delete_on_close: - echo.echo_info(f'Exported workflow data to {filename}') + echo.echo_report(f'Exported workflow data to {filename}') class Migration(migrations.Migration): diff --git a/aiida/backends/sqlalchemy/migrations/versions/1b8ed3425af9_remove_legacy_workflows.py b/aiida/backends/sqlalchemy/migrations/versions/1b8ed3425af9_remove_legacy_workflows.py index f5daf0bac6..2b0eed82a1 100644 --- a/aiida/backends/sqlalchemy/migrations/versions/1b8ed3425af9_remove_legacy_workflows.py +++ b/aiida/backends/sqlalchemy/migrations/versions/1b8ed3425af9_remove_legacy_workflows.py @@ -91,7 +91,7 @@ def export_workflow_data(connection): # If delete_on_close is False, we are running for the user and add additional message of file location if not delete_on_close: - echo.echo_info(f'Exported workflow data to {filename}') + echo.echo_report(f'Exported workflow data to {filename}') def upgrade(): diff --git a/aiida/cmdline/commands/cmd_archive.py b/aiida/cmdline/commands/cmd_archive.py index 073809ba77..88ce38548b 100644 --- a/aiida/cmdline/commands/cmd_archive.py +++ b/aiida/cmdline/commands/cmd_archive.py @@ -401,7 +401,7 @@ def _gather_imports(archives, webpages) -> List[Tuple[str, bool]]: if webpages is not None: for webpage in webpages: try: - echo.echo_info(f'retrieving archive URLS from {webpage}') + echo.echo_report(f'retrieving archive URLS from {webpage}') urls = get_valid_import_links(webpage) except Exception as error: echo.echo_critical( @@ -434,7 +434,7 @@ def _import_archive(archive: str, web_based: bool, import_kwargs: dict, try_migr archive_path = archive if web_based: - echo.echo_info(f'downloading archive: {archive}') + echo.echo_report(f'downloading archive: {archive}') try: with urllib.request.urlopen(archive) as response: temp_folder.create_file_from_filelike(response, 'downloaded_archive.zip') @@ -444,13 +444,13 @@ def _import_archive(archive: str, web_based: bool, import_kwargs: dict, try_migr archive_path = temp_folder.get_abs_path('downloaded_archive.zip') echo.echo_success('archive downloaded, proceeding with import') - echo.echo_info(f'starting import: {archive}') + echo.echo_report(f'starting import: {archive}') try: import_data(archive_path, **import_kwargs) except IncompatibleArchiveVersionError as exception: if try_migration: - echo.echo_info(f'incompatible version detected for {archive}, trying migration') + echo.echo_report(f'incompatible version detected for {archive}, trying migration') try: migrator = get_migrator(detect_archive_type(archive_path))(archive_path) archive_path = migrator.migrate( @@ -459,7 +459,7 @@ def _import_archive(archive: str, web_based: bool, import_kwargs: dict, try_migr except Exception as exception: _echo_exception(f'an exception occurred while migrating the archive {archive}', exception) - echo.echo_info('proceeding with import of migrated archive') + echo.echo_report('proceeding with import of migrated archive') try: import_data(archive_path, **import_kwargs) except Exception as exception: diff --git a/aiida/cmdline/commands/cmd_calcjob.py b/aiida/cmdline/commands/cmd_calcjob.py index 94bb585cce..f32d365c17 100644 --- a/aiida/cmdline/commands/cmd_calcjob.py +++ b/aiida/cmdline/commands/cmd_calcjob.py @@ -46,7 +46,7 @@ def calcjob_gotocomputer(calcjob): echo.echo_critical('no remote work directory for this calcjob, maybe the daemon did not submit it yet') command = transport.gotocomputer_command(remote_workdir) - echo.echo_info('going to the remote work directory...') + echo.echo_report('going to the remote work directory...') os.system(command) diff --git a/aiida/cmdline/commands/cmd_computer.py b/aiida/cmdline/commands/cmd_computer.py index 66f1125e61..4d4f6413ea 100644 --- a/aiida/cmdline/commands/cmd_computer.py +++ b/aiida/cmdline/commands/cmd_computer.py @@ -236,8 +236,8 @@ def computer_setup(ctx, non_interactive, **kwargs): else: echo.echo_success(f'Computer<{computer.pk}> {computer.label} created') - echo.echo_info('Note: before the computer can be used, it has to be configured with the command:') - echo.echo_info(f' verdi computer configure {computer.transport_type} {computer.label}') + echo.echo_report('Note: before the computer can be used, it has to be configured with the command:') + echo.echo_report(f' verdi computer configure {computer.transport_type} {computer.label}') @verdi_computer.command('duplicate') @@ -289,8 +289,8 @@ def computer_duplicate(ctx, computer, non_interactive, **kwargs): is_configured = computer.is_user_configured(orm.User.objects.get_default()) if not is_configured: - echo.echo_info('Note: before the computer can be used, it has to be configured with the command:') - echo.echo_info(f' verdi computer configure {computer.transport_type} {computer.label}') + echo.echo_report('Note: before the computer can be used, it has to be configured with the command:') + echo.echo_report(f' verdi computer configure {computer.transport_type} {computer.label}') @verdi_computer.command('enable') @@ -308,9 +308,11 @@ def computer_enable(computer, user): if not authinfo.enabled: authinfo.enabled = True - echo.echo_info(f"Computer '{computer.label}' enabled for user {user.get_full_name()}.") + echo.echo_report(f"Computer '{computer.label}' enabled for user {user.get_full_name()}.") else: - echo.echo_info(f"Computer '{computer.label}' was already enabled for user {user.first_name} {user.last_name}.") + echo.echo_report( + f"Computer '{computer.label}' was already enabled for user {user.first_name} {user.last_name}." + ) @verdi_computer.command('disable') @@ -330,9 +332,11 @@ def computer_disable(computer, user): if authinfo.enabled: authinfo.enabled = False - echo.echo_info(f"Computer '{computer.label}' disabled for user {user.get_full_name()}.") + echo.echo_report(f"Computer '{computer.label}' disabled for user {user.get_full_name()}.") else: - echo.echo_info(f"Computer '{computer.label}' was already disabled for user {user.first_name} {user.last_name}.") + echo.echo_report( + f"Computer '{computer.label}' was already disabled for user {user.first_name} {user.last_name}." + ) @verdi_computer.command('list') @@ -344,14 +348,14 @@ def computer_list(all_entries, raw): from aiida.orm import Computer, User if not raw: - echo.echo_info('List of configured computers') - echo.echo_info("Use 'verdi computer show COMPUTERLABEL' to display more detailed information") + echo.echo_report('List of configured computers') + echo.echo_report("Use 'verdi computer show COMPUTERLABEL' to display more detailed information") computers = Computer.objects.all() user = User.objects.get_default() if not computers: - echo.echo_info("No computers configured yet. Use 'verdi computer setup'") + echo.echo_report("No computers configured yet. Use 'verdi computer setup'") sort = lambda computer: computer.label highlight = lambda comp: comp.is_user_configured(user) and comp.is_user_enabled(user) @@ -432,7 +436,7 @@ def computer_test(user, print_traceback, computer): if user is None: user = orm.User.objects.get_default() - echo.echo_info(f'Testing computer<{computer.label}> for user<{user.email}>...') + echo.echo_report(f'Testing computer<{computer.label}> for user<{user.email}>...') try: authinfo = computer.get_authinfo(user) diff --git a/aiida/cmdline/commands/cmd_data/cmd_show.py b/aiida/cmdline/commands/cmd_data/cmd_show.py index 05648c671d..a6c99c1def 100644 --- a/aiida/cmdline/commands/cmd_data/cmd_show.py +++ b/aiida/cmdline/commands/cmd_data/cmd_show.py @@ -66,7 +66,7 @@ def _show_jmol(exec_name, trajectory_list, **kwargs): subprocess.check_output([exec_name, handle.name]) except subprocess.CalledProcessError: # The program died: just print a message - echo.echo_info(f'the call to {exec_name} ended with an error.') + echo.echo_error(f'the call to {exec_name} ended with an error.') except OSError as err: if err.errno == 2: echo.echo_critical( @@ -97,7 +97,7 @@ def _show_xcrysden(exec_name, object_list, **kwargs): subprocess.check_output([exec_name, '--xsf', tmpf.name]) except subprocess.CalledProcessError: # The program died: just print a message - echo.echo_info(f'the call to {exec_name} ended with an error.') + echo.echo_error(f'the call to {exec_name} ended with an error.') except OSError as err: if err.errno == 2: echo.echo_critical( @@ -159,7 +159,7 @@ def _show_vesta(exec_name, structure_list): subprocess.check_output([exec_name, tmpf.name]) except subprocess.CalledProcessError: # The program died: just print a message - echo.echo_info(f'the call to {exec_name} ended with an error.') + echo.echo_error(f'the call to {exec_name} ended with an error.') except OSError as err: if err.errno == 2: echo.echo_critical( @@ -190,7 +190,7 @@ def _show_vmd(exec_name, structure_list): subprocess.check_output([exec_name, tmpf.name]) except subprocess.CalledProcessError: # The program died: just print a message - echo.echo_info(f'the call to {exec_name} ended with an error.') + echo.echo_error(f'the call to {exec_name} ended with an error.') except OSError as err: if err.errno == 2: echo.echo_critical( diff --git a/aiida/cmdline/commands/cmd_database.py b/aiida/cmdline/commands/cmd_database.py index c2b4f085c6..35d674f3e9 100644 --- a/aiida/cmdline/commands/cmd_database.py +++ b/aiida/cmdline/commands/cmd_database.py @@ -132,7 +132,7 @@ def detect_duplicate_uuid(table, apply_patch): echo.echo_critical(f'integrity check failed: {str(exception)}') else: for message in messages: - echo.echo_info(message) + echo.echo_report(message) if apply_patch: echo.echo_success('integrity patch completed') diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index 353520b4e0..ab01f33140 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -107,7 +107,7 @@ def group_delete(group, delete_nodes, dry_run, force, verbose, **traversal_rules if not (force or dry_run): click.confirm(f'Are you sure you want to delete {group}?', abort=True) elif dry_run: - echo.echo_info(f'Would have deleted {group}.') + echo.echo_report(f'Would have deleted {group}.') if delete_nodes: @@ -341,10 +341,10 @@ def group_list( table.append([projection_lambdas[field](group[0]) for field in projection_fields]) if not all_entries: - echo.echo_info('To show groups of all types, use the `-a/--all` option.') + echo.echo_report('To show groups of all types, use the `-a/--all` option.') if not table: - echo.echo_info('No groups found matching the specified criteria.') + echo.echo_report('No groups found matching the specified criteria.') else: echo.echo(tabulate(table, headers=projection_header)) @@ -361,7 +361,7 @@ def group_create(group_label): if created: echo.echo_success(f"Group created with PK = {group.pk} and label '{group.label}'.") else: - echo.echo_info(f"Group with label '{group.label}' already exists: {group}.") + echo.echo_report(f"Group with label '{group.label}' already exists: {group}.") @verdi_group.command('copy') diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 9286a0e765..f4e11d332e 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -533,7 +533,7 @@ def comment_show(user, nodes): if not comments: valid_users = ', '.join(set(comment.user.email for comment in all_comments)) echo.echo_warning(f'no comments found for user {user}') - echo.echo_info(f'valid users found for Node<{node.pk}>: {valid_users}') + echo.echo_report(f'valid users found for Node<{node.pk}>: {valid_users}') else: comments = all_comments @@ -548,7 +548,7 @@ def comment_show(user, nodes): echo.echo('\n'.join(comment_msg)) if not comments: - echo.echo_info('no comments found') + echo.echo_report('no comments found') @verdi_comment.command('remove') diff --git a/aiida/cmdline/commands/cmd_plugin.py b/aiida/cmdline/commands/cmd_plugin.py index ec93f887f1..cb4bbd3dd4 100644 --- a/aiida/cmdline/commands/cmd_plugin.py +++ b/aiida/cmdline/commands/cmd_plugin.py @@ -34,12 +34,12 @@ def plugin_list(entry_point_group, entry_point): from aiida.plugins.entry_point import get_entry_point_names, load_entry_point if entry_point_group is None: - echo.echo_info('Available entry point groups:') + echo.echo_report('Available entry point groups:') for group in sorted(ENTRY_POINT_GROUP_TO_MODULE_PATH_MAP.keys()): echo.echo(f'* {group}') echo.echo('') - echo.echo_info('Pass one of the groups as an additional argument to show the registered plugins') + echo.echo_report('Pass one of the groups as an additional argument to show the registered plugins') return if entry_point: @@ -63,6 +63,6 @@ def plugin_list(entry_point_group, entry_point): echo.echo(f'* {registered_entry_point}') echo.echo('') - echo.echo_info('Pass the entry point as an argument to display detailed information') + echo.echo_report('Pass the entry point as an argument to display detailed information') else: echo.echo_error(f'No plugins found for group {entry_point_group}') diff --git a/aiida/cmdline/commands/cmd_process.py b/aiida/cmdline/commands/cmd_process.py index b65ff11748..13ca601598 100644 --- a/aiida/cmdline/commands/cmd_process.py +++ b/aiida/cmdline/commands/cmd_process.py @@ -293,7 +293,7 @@ def _print(communicator, body, sender, subject, correlation_id): # pylint: disa echo.echo(f'Process<{sender}> [{subject}|{correlation_id}]: {body}') communicator = get_manager().get_communicator() - echo.echo_info('watching for broadcasted messages, press CTRL+C to stop...') + echo.echo_report('watching for broadcasted messages, press CTRL+C to stop...') for process in processes: @@ -309,7 +309,7 @@ def _print(communicator, body, sender, subject, correlation_id): # pylint: disa sleep(2) except (SystemExit, KeyboardInterrupt): echo.echo('') # add a new line after the interrupt character - echo.echo_info('received interrupt, exiting...') + echo.echo_report('received interrupt, exiting...') try: communicator.close() except RuntimeError: @@ -373,7 +373,7 @@ def process_actions(futures_map, infinitive, present, past, wait=False, timeout= echo.echo_error(f'got unexpected response when {present} Process<{process.pk}>: {result}') if wait and scheduled: - echo.echo_info(f"waiting for process(es) {','.join([str(proc.pk) for proc in scheduled.values()])}") + echo.echo_report(f"waiting for process(es) {','.join([str(proc.pk) for proc in scheduled.values()])}") for future in futures.as_completed(scheduled.keys(), timeout=timeout): process = scheduled[future] diff --git a/aiida/cmdline/commands/cmd_profile.py b/aiida/cmdline/commands/cmd_profile.py index 338254bf0a..846d53ef78 100644 --- a/aiida/cmdline/commands/cmd_profile.py +++ b/aiida/cmdline/commands/cmd_profile.py @@ -34,10 +34,10 @@ def profile_list(): # to be able to see the configuration directory, for instance for those who have set `AIIDA_PATH`. This way # they can at least verify that it is correctly set. from aiida.manage.configuration.settings import AIIDA_CONFIG_FOLDER - echo.echo_info(f'configuration folder: {AIIDA_CONFIG_FOLDER}') + echo.echo_report(f'configuration folder: {AIIDA_CONFIG_FOLDER}') echo.echo_critical(str(exception)) else: - echo.echo_info(f'configuration folder: {config.dirpath}') + echo.echo_report(f'configuration folder: {config.dirpath}') if not config.profiles: echo.echo_warning('no profiles configured: run `verdi setup` to create one') @@ -55,7 +55,7 @@ def profile_show(profile): if profile is None: echo.echo_critical('no profile to show') - echo.echo_info(f'Profile: {profile.name}') + echo.echo_report(f'Profile: {profile.name}') data = sorted([(k.lower(), v) for k, v in profile.dictionary.items()]) echo.echo(tabulate.tabulate(data)) @@ -130,7 +130,7 @@ def profile_delete(force, include_config, include_database, include_database_use echo.echo_warning('this operation cannot be undone, ', nl=False) if not force and not click.confirm('are you sure you want to continue?'): - echo.echo_info(f'deleting of `{profile.name} cancelled.') + echo.echo_report(f'deleting of `{profile.name} cancelled.') continue get_config().delete_profile( diff --git a/aiida/cmdline/commands/cmd_setup.py b/aiida/cmdline/commands/cmd_setup.py index 6d90dc6d1b..28f52ef8ad 100644 --- a/aiida/cmdline/commands/cmd_setup.py +++ b/aiida/cmdline/commands/cmd_setup.py @@ -77,7 +77,7 @@ def setup( echo.echo_success(f'created new profile `{profile.name}`.') # Migrate the database - echo.echo_info('migrating the database.') + echo.echo_report('migrating the database.') manager = get_manager() backend = manager._load_backend(schema_check=False, repository_check=False) # pylint: disable=protected-access diff --git a/aiida/cmdline/commands/cmd_status.py b/aiida/cmdline/commands/cmd_status.py index 7ecc993934..9d9d60acad 100644 --- a/aiida/cmdline/commands/cmd_status.py +++ b/aiida/cmdline/commands/cmd_status.py @@ -71,7 +71,7 @@ def verdi_status(print_traceback, no_rmq): if profile is None: print_status(ServiceStatus.WARNING, 'profile', 'no profile configured yet') - echo.echo_info('Configure a profile by running `verdi quicksetup` or `verdi setup`.') + echo.echo_report('Configure a profile by running `verdi quicksetup` or `verdi setup`.') return try: diff --git a/aiida/cmdline/params/options/interactive.py b/aiida/cmdline/params/options/interactive.py index dfb3a69b3e..7c3113e97e 100644 --- a/aiida/cmdline/params/options/interactive.py +++ b/aiida/cmdline/params/options/interactive.py @@ -141,7 +141,7 @@ def prompt_func(self, ctx): def ctrl_help(self): """control behaviour when help is requested from the prompt""" - echo.echo_info(self.format_help_message()) + echo.echo(self.format_help_message()) def format_help_message(self): """ @@ -199,8 +199,8 @@ def safely_convert(self, value, param, ctx): def simple_prompt_loop(self, ctx, param, value): """Prompt until successful conversion. dispatch control sequences.""" if not hasattr(ctx, 'prompt_loop_info_printed'): - echo.echo_info(f'enter "{self.CHARACTER_PROMPT_HELP}" for help') - echo.echo_info(f'enter "{self.CHARACTER_IGNORE_DEFAULT}" to ignore the default and set no value') + echo.echo_report(f'enter "{self.CHARACTER_PROMPT_HELP}" for help') + echo.echo_report(f'enter "{self.CHARACTER_IGNORE_DEFAULT}" to ignore the default and set no value') ctx.prompt_loop_info_printed = True while 1: diff --git a/aiida/cmdline/utils/common.py b/aiida/cmdline/utils/common.py index 279e09b8bc..4e22a195d4 100644 --- a/aiida/cmdline/utils/common.py +++ b/aiida/cmdline/utils/common.py @@ -52,7 +52,7 @@ def print_last_process_state_change(process_type=None): :param process_type: optional process type for which to get the latest state change timestamp. Valid process types are either 'calculation' or 'work'. """ - from aiida.cmdline.utils.echo import echo_info + from aiida.cmdline.utils.echo import echo_report from aiida.common import timezone from aiida.common.utils import str_timedelta from aiida.engine.utils import get_process_state_change_timestamp @@ -60,12 +60,12 @@ def print_last_process_state_change(process_type=None): timestamp = get_process_state_change_timestamp(process_type) if timestamp is None: - echo_info('last time an entry changed state: never') + echo_report('last time an entry changed state: never') else: timedelta = timezone.delta(timestamp, timezone.now()) formatted = format_local_time(timestamp, format_str='at %H:%M:%S on %Y-%m-%d') relative = str_timedelta(timedelta, negative_to_zero=True, max_num_fields=1) - echo_info(f'last time an entry changed state: {relative} ({formatted})') + echo_report(f'last time an entry changed state: {relative} ({formatted})') def get_node_summary(node): @@ -385,7 +385,7 @@ def print_process_info(process): echo.echo('Description:\n', fg='red', bold=True) for line in docstring: echo.echo(f' {line.lstrip()}') - echo.echo() + echo.echo('') print_process_spec(process.spec()) @@ -503,6 +503,6 @@ def check_worker_load(active_slots): echo.echo_warning(f'{percent_load * 100:.0f}% of the available daemon worker slots have been used!') echo.echo_warning("Increase the number of workers with 'verdi daemon incr'.\n") else: - echo.echo_info(f'Using {percent_load * 100:.0f}% of the available daemon worker slots') + echo.echo_report(f'Using {percent_load * 100:.0f}% of the available daemon worker slots') else: - echo.echo_info('No active daemon workers') + echo.echo_report('No active daemon workers') diff --git a/aiida/manage/external/postgres.py b/aiida/manage/external/postgres.py index 0a6ff8f937..c418d712d7 100644 --- a/aiida/manage/external/postgres.py +++ b/aiida/manage/external/postgres.py @@ -120,7 +120,7 @@ def check_dbuser(self, dbuser): return dbuser, not self.dbuser_exists(dbuser) create = True while create and self.dbuser_exists(dbuser): - echo.echo_info(f'Database user "{dbuser}" already exists!') + echo.echo_warning(f'Database user "{dbuser}" already exists!') if not click.confirm('Use it? '): dbuser = click.prompt('New database user name: ', type=str, default=dbuser) else: @@ -169,7 +169,7 @@ def check_db(self, dbname): return dbname, not self.db_exists(dbname) create = True while create and self.db_exists(dbname): - echo.echo_info(f'database {dbname} already exists!') + echo.echo_warning(f'database {dbname} already exists!') if not click.confirm('Use it (make sure it is not used by another profile)?'): dbname = click.prompt('new name', type=str, default=dbname) else: diff --git a/aiida/tools/graph/deletions.py b/aiida/tools/graph/deletions.py index 5334413d54..57f785e9c2 100644 --- a/aiida/tools/graph/deletions.py +++ b/aiida/tools/graph/deletions.py @@ -70,7 +70,7 @@ def _missing_callback(_pks: Iterable[int]): pks_set_to_delete = get_nodes_delete(pks, get_links=False, missing_callback=_missing_callback, **traversal_rules)['nodes'] - DELETE_LOGGER.info('%s Node(s) marked for deletion', len(pks_set_to_delete)) + DELETE_LOGGER.report('%s Node(s) marked for deletion', len(pks_set_to_delete)) if pks_set_to_delete and DELETE_LOGGER.level == logging.DEBUG: builder = QueryBuilder().append( @@ -87,20 +87,20 @@ def _missing_callback(_pks: Iterable[int]): DELETE_LOGGER.debug(f' {uuid} {pk} {short_type_string} {label}') if dry_run is True: - DELETE_LOGGER.info('This was a dry run, exiting without deleting anything') + DELETE_LOGGER.report('This was a dry run, exiting without deleting anything') return (pks_set_to_delete, False) # confirm deletion if callable(dry_run) and dry_run(pks_set_to_delete): - DELETE_LOGGER.info('This was a dry run, exiting without deleting anything') + DELETE_LOGGER.report('This was a dry run, exiting without deleting anything') return (pks_set_to_delete, False) if not pks_set_to_delete: return (pks_set_to_delete, True) - DELETE_LOGGER.info('Starting node deletion...') + DELETE_LOGGER.report('Starting node deletion...') delete_nodes_and_connections(pks_set_to_delete) - DELETE_LOGGER.info('Deletion of nodes completed.') + DELETE_LOGGER.report('Deletion of nodes completed.') return (pks_set_to_delete, True) diff --git a/aiida/tools/importexport/archive/migrators.py b/aiida/tools/importexport/archive/migrators.py index 1446cc7ef2..08ba07f041 100644 --- a/aiida/tools/importexport/archive/migrators.py +++ b/aiida/tools/importexport/archive/migrators.py @@ -124,7 +124,7 @@ def migrate( if out_compression not in allowed_compressions: raise ValueError(f'Output compression must be in: {allowed_compressions}') - MIGRATE_LOGGER.info('Reading archive version') + MIGRATE_LOGGER.report('Reading archive version') current_version = self._retrieve_version() # compute the migration pathway @@ -141,10 +141,10 @@ def migrate( prev_version = MIGRATE_FUNCTIONS[prev_version][0] if not pathway: - MIGRATE_LOGGER.info('No migration required') + MIGRATE_LOGGER.report('No migration required') return None - MIGRATE_LOGGER.info('Migration pathway: %s', ' -> '.join(pathway + [version])) + MIGRATE_LOGGER.report('Migration pathway: %s', ' -> '.join(pathway + [version])) # perform migrations if work_dir is not None: @@ -162,7 +162,7 @@ def _perform_migration( """Perform the migration(s) in the work directory, compress (if necessary), then move to the out_path (if not None). """ - MIGRATE_LOGGER.info('Extracting archive to work directory') + MIGRATE_LOGGER.report('Extracting archive to work directory') extracted = Path(work_dir) / 'extracted' extracted.mkdir(parents=True) @@ -185,7 +185,7 @@ def _perform_migration( # re-compress archive if out_compression != 'none': - MIGRATE_LOGGER.info(f"Re-compressing archive as '{out_compression}'") + MIGRATE_LOGGER.report(f"Re-compressing archive as '{out_compression}'") migrated = work_dir / 'compressed' else: migrated = extracted @@ -199,7 +199,7 @@ def _perform_migration( if out_path is not None: # move to final location - MIGRATE_LOGGER.info('Moving archive to: %s', out_path) + MIGRATE_LOGGER.report('Moving archive to: %s', out_path) self._move_file(migrated, Path(out_path)) return Path(out_path) if out_path else migrated diff --git a/aiida/tools/importexport/dbexport/main.py b/aiida/tools/importexport/dbexport/main.py index e56c6fff49..4de87db391 100644 --- a/aiida/tools/importexport/dbexport/main.py +++ b/aiida/tools/importexport/dbexport/main.py @@ -250,7 +250,7 @@ def export( ) else: exported_entity_pks = defaultdict(set) - EXPORT_LOGGER.info('No entities were found to export') + EXPORT_LOGGER.report('No entities were found to export') # write mappings of groups to the nodes they contain if exported_entity_pks[GROUP_ENTITY_NAME]: @@ -270,14 +270,14 @@ def export( writer=writer_context ) - EXPORT_LOGGER.info('Finalizing Export...') + EXPORT_LOGGER.report('Finalizing Export...') # summarize export export_summary = '\n - '.join(f'{name:<6}: {len(pks)}' for name, pks in exported_entity_pks.items()) if exported_entity_pks: - EXPORT_LOGGER.info('Exported Entities:\n - ' + export_summary + '\n') + EXPORT_LOGGER.report('Exported Entities:\n - ' + export_summary + '\n') # TODO - # EXPORT_LOGGER.info('Writer Information:\n %s', writer.export_info) + # EXPORT_LOGGER.report('Writer Information:\n %s', writer.export_info) return writer diff --git a/aiida/transports/cli.py b/aiida/transports/cli.py index f4d9951d50..3c5e85073d 100644 --- a/aiida/transports/cli.py +++ b/aiida/transports/cli.py @@ -40,9 +40,9 @@ def configure_computer_main(computer, user, **kwargs): user = user or orm.User.objects.get_default() - echo.echo_info(f'Configuring computer {computer.label} for user {user.email}.') + echo.echo_report(f'Configuring computer {computer.label} for user {user.email}.') if user.email != get_manager().get_profile().default_user: - echo.echo_info('Configuring different user, defaults may not be appropriate.') + echo.echo_report('Configuring different user, defaults may not be appropriate.') computer.configure(user=user, **kwargs) echo.echo_success(f'{computer.label} successfully configured for {user.email}') diff --git a/tests/cmdline/commands/test_profile.py b/tests/cmdline/commands/test_profile.py index 7eb262eeb3..2d69a8897c 100644 --- a/tests/cmdline/commands/test_profile.py +++ b/tests/cmdline/commands/test_profile.py @@ -73,7 +73,7 @@ def test_list(self): result = self.cli_runner.invoke(cmd_profile.profile_list) self.assertClickSuccess(result) - self.assertIn(f'Info: configuration folder: {self.config.dirpath}', result.output) + self.assertIn(f'Report: configuration folder: {self.config.dirpath}', result.output) self.assertIn(f'* {self.profile_list[0]}', result.output) self.assertIn(self.profile_list[1], result.output) @@ -87,7 +87,7 @@ def test_setdefault(self): result = self.cli_runner.invoke(cmd_profile.profile_list) self.assertClickSuccess(result) - self.assertIn(f'Info: configuration folder: {self.config.dirpath}', result.output) + self.assertIn(f'Report: configuration folder: {self.config.dirpath}', result.output) self.assertIn(f'* {self.profile_list[1]}', result.output) self.assertClickSuccess(result) diff --git a/tests/cmdline/params/options/test_interactive.py b/tests/cmdline/params/options/test_interactive.py index 0b436e0cad..1743c088f0 100644 --- a/tests/cmdline/params/options/test_interactive.py +++ b/tests/cmdline/params/options/test_interactive.py @@ -368,7 +368,7 @@ def test_after_callback_invalid(self): self.assertIn('Invalid value', result.output) self.assertIn('invalid', result.output) - def test_after_callback_wrong_typ(self): + def test_after_callback_wrong_type(self): """ scenario: InteractiveOption with a user callback action: invoke with invalid value of wrong type From ec81d0b40f9d50c8c332b8c5f52bac061b56918f Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Mon, 23 Aug 2021 21:03:32 +0200 Subject: [PATCH 5/7] CLI: remove `VERBOSE` option and replace with `VERBOSITY` The `VERBOSE` option is obsolete since all commands now automatically have the `VERBOSITY` option exposed. The `VERBOSE` option was mostly used to configure a logger (most notably the `DELETE_LOGGER`) but this is no longer necessary. Since those loggers are children of the main `AIIDA_LOGGER`, whose level is controlled through the `VERBOSITY` option, the child logger will inherit its value. Some commands used the `VERBOSE` option more as a boolean switch. To reproduce this behavior, a utiltiy function `is_verbose` is added to the `cmdline` module, which will return `True` if the loglevel of the `CMDLINE_LOGGER` is equal or smaller than the `INFO` level and `False` otherwise. The `verdi archive` commands used an ad-hoc implementation of the verbosity option. This has been removed and they now also rely on the logging level that is set on the `AIIDA_LOGGER` through the global `VERBOSITY` option. --- aiida/backends/sqlalchemy/manage.py | 14 ++--- aiida/cmdline/__init__.py | 1 + aiida/cmdline/commands/cmd_archive.py | 56 +++++------------- aiida/cmdline/commands/cmd_code.py | 15 ++--- aiida/cmdline/commands/cmd_database.py | 12 ++-- aiida/cmdline/commands/cmd_devel.py | 7 +-- aiida/cmdline/commands/cmd_group.py | 9 +-- aiida/cmdline/commands/cmd_node.py | 9 +-- aiida/cmdline/params/options/__init__.py | 1 - aiida/cmdline/params/options/main.py | 6 +- aiida/cmdline/utils/__init__.py | 2 + aiida/cmdline/utils/common.py | 15 +++++ tests/cmdline/commands/test_database.py | 4 +- tests/cmdline/commands/test_node.py | 73 +++++++++--------------- 14 files changed, 89 insertions(+), 135 deletions(-) diff --git a/aiida/backends/sqlalchemy/manage.py b/aiida/backends/sqlalchemy/manage.py index d593b6bb7e..1538a1b9e1 100755 --- a/aiida/backends/sqlalchemy/manage.py +++ b/aiida/backends/sqlalchemy/manage.py @@ -9,10 +9,10 @@ # For further information please visit http://www.aiida.net # ########################################################################### """Simple wrapper around the alembic command line tool that first loads an AiiDA profile.""" - import alembic import click +from aiida.cmdline import is_verbose from aiida.cmdline.params import options @@ -51,18 +51,18 @@ def alembic_revision(message): @alembic_cli.command('current') -@options.VERBOSE() -def alembic_current(verbose): +@options.VERBOSITY() +def alembic_current(): """Show the current revision.""" - execute_alembic_command('current', verbose=verbose) + execute_alembic_command('current', verbose=is_verbose()) @alembic_cli.command('history') @click.option('-r', '--rev-range') -@options.VERBOSE() -def alembic_history(rev_range, verbose): +@options.VERBOSITY() +def alembic_history(rev_range): """Show the history for the given revision range.""" - execute_alembic_command('history', rev_range=rev_range, verbose=verbose) + execute_alembic_command('history', rev_range=rev_range, verbose=is_verbose()) @alembic_cli.command('upgrade') diff --git a/aiida/cmdline/__init__.py b/aiida/cmdline/__init__.py index b997337142..c239c20bdc 100644 --- a/aiida/cmdline/__init__.py +++ b/aiida/cmdline/__init__.py @@ -46,6 +46,7 @@ 'WorkflowParamType', 'dbenv', 'format_call_graph', + 'is_verbose', 'only_if_daemon_running', 'with_dbenv', ) diff --git a/aiida/cmdline/commands/cmd_archive.py b/aiida/cmdline/commands/cmd_archive.py index 88ce38548b..84c967c8f2 100644 --- a/aiida/cmdline/commands/cmd_archive.py +++ b/aiida/cmdline/commands/cmd_archive.py @@ -10,6 +10,7 @@ # pylint: disable=too-many-arguments,import-error,too-many-locals,broad-except """`verdi archive` command.""" from enum import Enum +import logging from typing import List, Tuple import traceback import urllib.request @@ -22,6 +23,7 @@ from aiida.cmdline.params.types import GroupParamType, PathOrUrl from aiida.cmdline.utils import decorators, echo from aiida.common.links import GraphTraversalRules +from aiida.common.log import AIIDA_LOGGER EXTRAS_MODE_EXISTING = ['keep_existing', 'update_existing', 'mirror', 'none', 'ask'] EXTRAS_MODE_NEW = ['import', 'none'] @@ -82,13 +84,6 @@ def inspect(archive, version, meta_data): type=click.Choice(['zip', 'zip-uncompressed', 'zip-lowmemory', 'tar.gz', 'null']), ) @options.FORCE(help='Overwrite output file if it already exists.') -@click.option( - '-v', - '--verbosity', - default='INFO', - type=click.Choice(['DEBUG', 'INFO', 'WARNING', 'CRITICAL']), - help='Control the verbosity of console logging' -) @options.graph_traversal_rules(GraphTraversalRules.EXPORT.value) @click.option( '--include-logs/--exclude-logs', @@ -113,7 +108,7 @@ def inspect(archive, version, meta_data): @decorators.with_dbenv() def create( output_file, codes, computers, groups, nodes, archive_format, force, input_calc_forward, input_work_forward, - create_backward, return_backward, call_calc_backward, call_work_backward, include_comments, include_logs, verbosity + create_backward, return_backward, call_calc_backward, call_work_backward, include_comments, include_logs ): """ Export subsets of the provenance graph to file for sharing. @@ -127,7 +122,7 @@ def create( # pylint: disable=too-many-branches from aiida.common.log import override_log_formatter_context from aiida.common.progress_reporter import set_progress_bar_tqdm, set_progress_reporter - from aiida.tools.importexport import export, ExportFileFormat, EXPORT_LOGGER + from aiida.tools.importexport import export, ExportFileFormat from aiida.tools.importexport.common.exceptions import ArchiveExportError entities = [] @@ -170,11 +165,10 @@ def create( elif archive_format == 'null': export_format = 'null' - if verbosity in ['DEBUG', 'INFO']: - set_progress_bar_tqdm(leave=(verbosity == 'DEBUG')) + if AIIDA_LOGGER.level <= logging.INFO: + set_progress_bar_tqdm(leave=(AIIDA_LOGGER.level == logging.DEBUG)) else: set_progress_reporter(None) - EXPORT_LOGGER.setLevel(verbosity) try: with override_log_formatter_context('%(message)s'): @@ -202,18 +196,12 @@ def create( # version inside the function when needed. help='Archive format version to migrate to (defaults to latest version).', ) -@click.option( - '--verbosity', - default='INFO', - type=click.Choice(['DEBUG', 'INFO', 'WARNING', 'CRITICAL']), - help='Control the verbosity of console logging' -) -def migrate(input_file, output_file, force, in_place, archive_format, version, verbosity): +def migrate(input_file, output_file, force, in_place, archive_format, version): """Migrate an export archive to a more recent format version.""" from aiida.common.log import override_log_formatter_context from aiida.common.progress_reporter import set_progress_bar_tqdm, set_progress_reporter from aiida.tools.importexport import detect_archive_type, EXPORT_VERSION - from aiida.tools.importexport.archive.migrators import get_migrator, MIGRATE_LOGGER + from aiida.tools.importexport.archive.migrators import get_migrator if in_place: if output_file: @@ -225,11 +213,10 @@ def migrate(input_file, output_file, force, in_place, archive_format, version, v 'no output file specified. Please add --in-place flag if you would like to migrate in place.' ) - if verbosity in ['DEBUG', 'INFO']: - set_progress_bar_tqdm(leave=(verbosity == 'DEBUG')) + if AIIDA_LOGGER.level <= logging.INFO: + set_progress_bar_tqdm(leave=(AIIDA_LOGGER.level == logging.DEBUG)) else: set_progress_reporter(None) - MIGRATE_LOGGER.setLevel(verbosity) if version is None: version = EXPORT_VERSION @@ -241,15 +228,14 @@ def migrate(input_file, output_file, force, in_place, archive_format, version, v with override_log_formatter_context('%(message)s'): migrator.migrate(version, output_file, force=force, out_compression=archive_format) except Exception as error: # pylint: disable=broad-except - if verbosity == 'DEBUG': + if AIIDA_LOGGER.level <= logging.DEBUG: raise echo.echo_critical( 'failed to migrate the archive file (use `--verbosity DEBUG` to see traceback): ' f'{error.__class__.__name__}:{error}' ) - if verbosity in ['DEBUG', 'INFO']: - echo.echo_success(f'migrated the archive to version {version}') + echo.echo_success(f'migrated the archive to version {version}') class ExtrasImportCode(Enum): @@ -313,19 +299,11 @@ class ExtrasImportCode(Enum): show_default=True, help='Force migration of archive file archives, if needed.' ) -@click.option( - '-v', - '--verbosity', - default='INFO', - type=click.Choice(['DEBUG', 'INFO', 'WARNING', 'CRITICAL']), - help='Control the verbosity of console logging' -) @options.NON_INTERACTIVE() @decorators.with_dbenv() @click.pass_context def import_archive( - ctx, archives, webpages, group, extras_mode_existing, extras_mode_new, comment_mode, migration, non_interactive, - verbosity + ctx, archives, webpages, group, extras_mode_existing, extras_mode_new, comment_mode, migration, non_interactive ): """Import data from an AiiDA archive file. @@ -334,15 +312,11 @@ def import_archive( # pylint: disable=unused-argument from aiida.common.log import override_log_formatter_context from aiida.common.progress_reporter import set_progress_bar_tqdm, set_progress_reporter - from aiida.tools.importexport.dbimport.utils import IMPORT_LOGGER - from aiida.tools.importexport.archive.migrators import MIGRATE_LOGGER - if verbosity in ['DEBUG', 'INFO']: - set_progress_bar_tqdm(leave=(verbosity == 'DEBUG')) + if AIIDA_LOGGER.level <= logging.INFO: + set_progress_bar_tqdm(leave=(AIIDA_LOGGER.level == logging.DEBUG)) else: set_progress_reporter(None) - IMPORT_LOGGER.setLevel(verbosity) - MIGRATE_LOGGER.setLevel(verbosity) all_archives = _gather_imports(archives, webpages) diff --git a/aiida/cmdline/commands/cmd_code.py b/aiida/cmdline/commands/cmd_code.py index ee1662e1d1..a66212922b 100644 --- a/aiida/cmdline/commands/cmd_code.py +++ b/aiida/cmdline/commands/cmd_code.py @@ -9,7 +9,6 @@ ########################################################################### """`verdi code` command.""" from functools import partial -import logging import click import tabulate @@ -144,11 +143,11 @@ def code_duplicate(ctx, code, non_interactive, **kwargs): @verdi_code.command() @arguments.CODE() -@options.VERBOSE() @with_dbenv() -def show(code, verbose): +def show(code): """Display detailed information for a code.""" from aiida.repository import FileType + from aiida.cmdline import is_verbose table = [] table.append(['PK', code.pk]) @@ -174,7 +173,7 @@ def show(code, verbose): table.append(['Prepend text', code.get_prepend_text()]) table.append(['Append text', code.get_append_text()]) - if verbose: + if is_verbose(): table.append(['Calculations', len(code.get_outgoing().all())]) echo.echo(tabulate.tabulate(table)) @@ -182,20 +181,16 @@ def show(code, verbose): @verdi_code.command() @arguments.CODES() -@options.VERBOSE() @options.DRY_RUN() @options.FORCE() @with_dbenv() -def delete(codes, verbose, dry_run, force): +def delete(codes, dry_run, force): """Delete a code. Note that codes are part of the data provenance, and deleting a code will delete all calculations using it. """ from aiida.common.log import override_log_formatter_context - from aiida.tools import delete_nodes, DELETE_LOGGER - - verbosity = logging.DEBUG if verbose else logging.INFO - DELETE_LOGGER.setLevel(verbosity) + from aiida.tools import delete_nodes node_pks_to_delete = [code.pk for code in codes] diff --git a/aiida/cmdline/commands/cmd_database.py b/aiida/cmdline/commands/cmd_database.py index 35d674f3e9..c70aeba485 100644 --- a/aiida/cmdline/commands/cmd_database.py +++ b/aiida/cmdline/commands/cmd_database.py @@ -197,28 +197,28 @@ def detect_invalid_nodes(): @verdi_database.command('summary') -@options.VERBOSE() -def database_summary(verbose): +def database_summary(): """Summarise the entities in the database.""" + from aiida.cmdline import is_verbose from aiida.orm import QueryBuilder, Node, Group, Computer, Comment, Log, User data = {} # User query_user = QueryBuilder().append(User, project=['email']) data['Users'] = {'count': query_user.count()} - if verbose: + if is_verbose(): data['Users']['emails'] = query_user.distinct().all(flat=True) # Computer query_comp = QueryBuilder().append(Computer, project=['label']) data['Computers'] = {'count': query_comp.count()} - if verbose: + if is_verbose(): data['Computers']['labels'] = query_comp.distinct().all(flat=True) # Node count = QueryBuilder().append(Node).count() data['Nodes'] = {'count': count} - if verbose: + if is_verbose(): node_types = QueryBuilder().append(Node, project=['node_type']).distinct().all(flat=True) data['Nodes']['node_types'] = node_types process_types = QueryBuilder().append(Node, project=['process_type']).distinct().all(flat=True) @@ -227,7 +227,7 @@ def database_summary(verbose): # Group query_group = QueryBuilder().append(Group, project=['type_string']) data['Groups'] = {'count': query_group.count()} - if verbose: + if is_verbose(): data['Groups']['type_strings'] = query_group.distinct().all(flat=True) # Comment diff --git a/aiida/cmdline/commands/cmd_devel.py b/aiida/cmdline/commands/cmd_devel.py index 06f4e18f03..21d127c752 100644 --- a/aiida/cmdline/commands/cmd_devel.py +++ b/aiida/cmdline/commands/cmd_devel.py @@ -11,7 +11,6 @@ import sys from aiida.cmdline.commands.cmd_verdi import verdi -from aiida.cmdline.params import options from aiida.cmdline.utils import decorators, echo @@ -21,8 +20,7 @@ def verdi_devel(): @verdi_devel.command('check-load-time') -@options.VERBOSE() -def devel_check_load_time(verbose): +def devel_check_load_time(): """Check for common indicators that slowdown `verdi`. Check for environment properties that negatively affect the responsiveness of the `verdi` command line interface. @@ -37,8 +35,7 @@ def devel_check_load_time(verbose): loaded_aiida_modules = [key for key in sys.modules if key.startswith('aiida.')] aiida_modules_str = '\n- '.join(sorted(loaded_aiida_modules)) - if verbose: - echo.echo(f'aiida modules loaded:\n- {aiida_modules_str}') + echo.echo_info(f'aiida modules loaded:\n- {aiida_modules_str}') manager = get_manager() diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index ab01f33140..46d0b86339 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -8,7 +8,6 @@ # For further information please visit http://www.aiida.net # ########################################################################### """`verdi group` commands""" -import logging import click from aiida.common.exceptions import UniquenessError @@ -93,17 +92,13 @@ def group_remove_nodes(group, nodes, clear, force): ) @options.graph_traversal_rules(GraphTraversalRules.DELETE.value) @options.DRY_RUN() -@options.VERBOSE() @with_dbenv() -def group_delete(group, delete_nodes, dry_run, force, verbose, **traversal_rules): +def group_delete(group, delete_nodes, dry_run, force, **traversal_rules): """Delete a group and (optionally) the nodes it contains.""" from aiida.common.log import override_log_formatter_context - from aiida.tools import delete_group_nodes, DELETE_LOGGER + from aiida.tools import delete_group_nodes from aiida import orm - verbosity = logging.DEBUG if verbose else logging.INFO - DELETE_LOGGER.setLevel(verbosity) - if not (force or dry_run): click.confirm(f'Are you sure you want to delete {group}?', abort=True) elif dry_run: diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index f4e11d332e..9be18aff3e 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -8,7 +8,6 @@ # For further information please visit http://www.aiida.net # ########################################################################### """`verdi node` command.""" -import logging import shutil import pathlib @@ -280,12 +279,11 @@ def extras(nodes, keys, fmt, identifier, raw): @verdi_node.command('delete') @click.argument('identifier', nargs=-1, metavar='NODES') -@options.VERBOSE() @options.DRY_RUN() @options.FORCE() @options.graph_traversal_rules(GraphTraversalRules.DELETE.value) @with_dbenv() -def node_delete(identifier, dry_run, verbose, force, **traversal_rules): +def node_delete(identifier, dry_run, force, **traversal_rules): """Delete nodes from the provenance graph. This will not only delete the nodes explicitly provided via the command line, but will also include @@ -294,10 +292,7 @@ def node_delete(identifier, dry_run, verbose, force, **traversal_rules): """ from aiida.common.log import override_log_formatter_context from aiida.orm.utils.loaders import NodeEntityLoader - from aiida.tools import delete_nodes, DELETE_LOGGER - - verbosity = logging.DEBUG if verbose else logging.INFO - DELETE_LOGGER.setLevel(verbosity) + from aiida.tools import delete_nodes pks = [] diff --git a/aiida/cmdline/params/options/__init__.py b/aiida/cmdline/params/options/__init__.py index 65f2992601..a2c585fc5b 100644 --- a/aiida/cmdline/params/options/__init__.py +++ b/aiida/cmdline/params/options/__init__.py @@ -104,7 +104,6 @@ 'USER_FIRST_NAME', 'USER_INSTITUTION', 'USER_LAST_NAME', - 'VERBOSE', 'VERBOSITY', 'VISUALIZATION_FORMAT', 'WAIT', diff --git a/aiida/cmdline/params/options/main.py b/aiida/cmdline/params/options/main.py index 4a74905415..861b3af5f9 100644 --- a/aiida/cmdline/params/options/main.py +++ b/aiida/cmdline/params/options/main.py @@ -30,8 +30,8 @@ 'OLDER_THAN', 'ORDER_BY', 'ORDER_DIRECTION', 'PAST_DAYS', 'PAUSED', 'PORT', 'PREPEND_TEXT', 'PRINT_TRACEBACK', 'PROCESS_LABEL', 'PROCESS_STATE', 'PROFILE', 'PROFILE_ONLY_CONFIG', 'PROFILE_SET_DEFAULT', 'PROJECT', 'RAW', 'REPOSITORY_PATH', 'SCHEDULER', 'SILENT', 'TIMEOUT', 'TRAJECTORY_INDEX', 'TRANSPORT', 'TRAVERSAL_RULE_HELP_STRING', - 'TYPE_STRING', 'USER', 'USER_EMAIL', 'USER_FIRST_NAME', 'USER_INSTITUTION', 'USER_LAST_NAME', 'VERBOSE', - 'VERBOSITY', 'VISUALIZATION_FORMAT', 'WAIT', 'WITH_ELEMENTS', 'WITH_ELEMENTS_EXCLUSIVE', 'active_process_states', + 'TYPE_STRING', 'USER', 'USER_EMAIL', 'USER_FIRST_NAME', 'USER_INSTITUTION', 'USER_LAST_NAME', 'VERBOSITY', + 'VISUALIZATION_FORMAT', 'WAIT', 'WITH_ELEMENTS', 'WITH_ELEMENTS_EXCLUSIVE', 'active_process_states', 'graph_traversal_rules', 'valid_calc_job_states', 'valid_process_states' ) @@ -553,8 +553,6 @@ def set_log_level(ctx, __, value): FREQUENCY = OverridableOption('-F', '--frequency', 'frequency', type=click.INT) -VERBOSE = OverridableOption('-v', '--verbose', is_flag=True, default=False, help='Be more verbose in printing output.') - TIMEOUT = OverridableOption( '-t', '--timeout', diff --git a/aiida/cmdline/utils/__init__.py b/aiida/cmdline/utils/__init__.py index 9562427ead..e7c4eff64f 100644 --- a/aiida/cmdline/utils/__init__.py +++ b/aiida/cmdline/utils/__init__.py @@ -16,11 +16,13 @@ # pylint: disable=wildcard-import from .ascii_vis import * +from .common import * from .decorators import * __all__ = ( 'dbenv', 'format_call_graph', + 'is_verbose', 'only_if_daemon_running', 'with_dbenv', ) diff --git a/aiida/cmdline/utils/common.py b/aiida/cmdline/utils/common.py index 4e22a195d4..f27d04096c 100644 --- a/aiida/cmdline/utils/common.py +++ b/aiida/cmdline/utils/common.py @@ -8,6 +8,7 @@ # For further information please visit http://www.aiida.net # ########################################################################### """Common utility functions for command line commands.""" +import logging import os import sys @@ -15,6 +16,20 @@ from . import echo +__all__ = ('is_verbose',) + + +def is_verbose(): + """Return whether the configured logging verbosity is considered verbose, i.e., equal or lower to ``INFO`` level. + + .. note:: This checks the effective logging level that is set on the ``CMDLINE_LOGGER``. This means that it will + consider the logging level set on the parent ``AIIDA_LOGGER`` if not explicitly set on itself. The level of the + main logger can be manipulated from the command line through the ``VERBOSITY`` option that is available for all + commands. + + """ + return echo.CMDLINE_LOGGER.getEffectiveLevel() <= logging.INFO + def get_env_with_venv_bin(): """Create a clone of the current running environment with the AIIDA_PATH variable set directory of the config.""" diff --git a/tests/cmdline/commands/test_database.py b/tests/cmdline/commands/test_database.py index 019ab40737..e5d79167b2 100644 --- a/tests/cmdline/commands/test_database.py +++ b/tests/cmdline/commands/test_database.py @@ -182,9 +182,9 @@ def tests_database_version(run_cli_command, manager): @pytest.mark.usefixtures('clear_database_before_test') def tests_database_summary(aiida_localhost, run_cli_command): - """Test the ``verdi database summary -v`` command.""" + """Test the ``verdi database summary`` command with the ``-verbosity`` option.""" from aiida import orm node = orm.Dict().store() - result = run_cli_command(cmd_database.database_summary, ['--verbose']) + result = run_cli_command(cmd_database.database_summary, ['--verbosity', 'info']) assert aiida_localhost.label in result.output assert node.node_type in result.output diff --git a/tests/cmdline/commands/test_node.py b/tests/cmdline/commands/test_node.py index f651f603b7..29b0712823 100644 --- a/tests/cmdline/commands/test_node.py +++ b/tests/cmdline/commands/test_node.py @@ -16,6 +16,7 @@ import gzip from click.testing import CliRunner +import pytest from aiida import orm from aiida.backends.testbase import AiidaTestCase @@ -597,55 +598,37 @@ def test_rehash_invalid_entry_point(self): self.assertIsNotNone(result.exception) -class TestVerdiDelete(AiidaTestCase): +@pytest.mark.parametrize( + 'options', ( + ['--verbosity', 'info'], + ['--verbosity', 'info', '--force'], + ['--create-forward'], + ['--call-calc-forward'], + ['--call-work-forward'], + ['--force'], + ) +) +@pytest.mark.usefixtures('clear_database_before_test') +def test_node_delete_basics(run_cli_command, options): """ - Tests for the ``verdi node delete`` command. - These test do not test the delete functionality, just that the command internal - logic does not create any problems before the call to the function. - For the actual functionality, see: - * source: manage.database.delete.nodes.py - * test: backends.tests.test_nodes.py + Testing the correct translation for the `--force` and `--verbosity` options. + This just checks that the calls do not except and that in all cases with the + force flag there is no messages. """ + from aiida.common.exceptions import NotExistent - def setUp(self): - self.cli_runner = CliRunner() + node = orm.Data().store() + pk = node.pk - def test_basics(self): - """ - Testing the correct translation for the `--force` and `--verbose` options. - This just checks that the calls do not except and that in all cases with the - force flag there is no messages. - """ - from aiida.common.exceptions import NotExistent + run_cli_command(cmd_node.node_delete, options + [str(pk), '--dry-run']) - newnode = orm.Data().store() - newnodepk = newnode.pk - options_list = [] - options_list.append(['--create-forward']) - options_list.append(['--call-calc-forward']) - options_list.append(['--call-work-forward']) - options_list.append(['--force']) - options_list.append(['--verbose']) - options_list.append(['--verbose', '--force']) - - for options in options_list: - run_options = [str(newnodepk)] - run_options.append('--dry-run') - for an_option in options: - run_options.append(an_option) - result = self.cli_runner.invoke(cmd_node.node_delete, run_options) - self.assertClickResultNoException(result) - - # To delete the created node - run_options = [str(newnodepk)] - run_options.append('--force') - result = self.cli_runner.invoke(cmd_node.node_delete, run_options) - self.assertClickResultNoException(result) + # To delete the created node + run_cli_command(cmd_node.node_delete, [str(pk), '--force']) - with self.assertRaises(NotExistent): - orm.load_node(newnodepk) + with pytest.raises(NotExistent): + orm.load_node(pk) - def test_missing_pk(self): - """Check that no exception is raised when a non-existent pk is given (just warns).""" - result = self.cli_runner.invoke(cmd_node.node_delete, ['999']) - self.assertClickResultNoException(result) + +def test_node_delete_missing_pk(run_cli_command): + """Check that no exception is raised when a non-existent pk is given (just warns).""" + run_cli_command(cmd_node.node_delete, ['999']) From 634031113bea86f46a512a4639b96046d166056d Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Tue, 24 Aug 2021 16:10:23 +0200 Subject: [PATCH 6/7] CLI: ensure all loggers have a custom formatter So far, the utility functions of the `aiida.cmdline.utils.echo` module have been rerouted through the `CMDLINE_LOGGER` which properly formats the log message with just the message itself. However, the loggers of any other modules will inherit the formatter of the parent `AIIDA_LOGGER` which will prefix the message with verbose content such as the timestamp, name of the logger and more, which is not what we want for output generated by CLI commands. The solution is to define a custom formatter, `CliFormatter`, which is added to the new module `aiida.cmdline.utils.log`, and which is configured by default for the `CMDLINE_LOGGER`. The `configure_logging` function is updated to now take a keyword argument `cli`, which when set to `True`, will configure this new formatter for all loggers. By calling this method with `cli=True` in the callback of the `VERBOSITY` option, all loggers should be correctly configured to only print the message with the log level prefixed, when invoked through the CLI. There is one peculiarity where the `load_backend_if_not_loaded` decorator utility also needs to call the `configure_logging` function. The reason is that this function loads the database backend, which indirectly calls the `configure_logging` to add the database log handler, but this does not pass `cli=True`. This means that the CLI specific configuration is undone. --- aiida/cmdline/params/options/main.py | 2 +- aiida/cmdline/utils/decorators.py | 5 +++ aiida/cmdline/utils/echo.py | 39 ++++++++----------- aiida/cmdline/utils/log.py | 57 ++++++++++++++++++++++++++++ aiida/common/log.py | 40 ++++++------------- 5 files changed, 91 insertions(+), 52 deletions(-) create mode 100644 aiida/cmdline/utils/log.py diff --git a/aiida/cmdline/params/options/main.py b/aiida/cmdline/params/options/main.py index 861b3af5f9..a84302171a 100644 --- a/aiida/cmdline/params/options/main.py +++ b/aiida/cmdline/params/options/main.py @@ -123,7 +123,7 @@ def set_log_level(ctx, __, value): if profile is not None: profile.set_option('logging.aiida_loglevel', log_level) # Make sure the value is currently loaded, even if it may be undone in the future by another call to this method - configure_logging() + configure_logging(cli=True) return log_level diff --git a/aiida/cmdline/utils/decorators.py b/aiida/cmdline/utils/decorators.py index 82bf49f170..8c2626dbc0 100644 --- a/aiida/cmdline/utils/decorators.py +++ b/aiida/cmdline/utils/decorators.py @@ -37,6 +37,7 @@ def load_backend_if_not_loaded(): If no profile has been loaded yet, the default profile will be loaded first. A spinner will be shown during both actions to indicate that the function is working and has not crashed, since loading can take a second. """ + from aiida.common.log import configure_logging from aiida.manage.configuration import get_profile, load_profile from aiida.manage.manager import get_manager @@ -46,6 +47,10 @@ def load_backend_if_not_loaded(): with spinner(): load_profile() # This will load the default profile if no profile has already been loaded manager.get_backend() # This will load the backend of the loaded profile, if not already loaded + # Loading the backend will have reconfigured the logging (see ``Manager._load_backend`` which calls the + # ``aiida.common.log.configure_logging`` function but with the default ``cli=False``), so here we have to + # call it again to ensure that the correct CLI handlers and formatters are configured. + configure_logging(with_orm=True, cli=True) def with_dbenv(): diff --git a/aiida/cmdline/utils/echo.py b/aiida/cmdline/utils/echo.py index 0e8c5ba53b..7e5e20a19c 100644 --- a/aiida/cmdline/utils/echo.py +++ b/aiida/cmdline/utils/echo.py @@ -46,7 +46,7 @@ class ExitCode(enum.IntEnum): def echo(message: str, fg: str = None, bold: bool = False, nl: bool = True, err: bool = False) -> None: """Log a message to the cmdline logger. - .. note:: The message will be logged at the ``REPORT`` level. + .. note:: The message will be logged at the ``REPORT`` level but always without the log level prefix. :param message: the message to log. :param fg: if provided this will become the foreground color. @@ -55,11 +55,11 @@ def echo(message: str, fg: str = None, bold: bool = False, nl: bool = True, err: :param err: whether to log to stderr. """ message = click.style(message, fg=fg, bold=bold) - CMDLINE_LOGGER.report(message, extra=dict(nl=nl, err=err)) + CMDLINE_LOGGER.report(message, extra=dict(nl=nl, err=err, prefix=False)) def echo_debug(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: - """Log a debug message to the cmdline logger, prefixed with 'Debug:' + """Log a debug message to the cmdline logger. :param message: the message to log. :param bold: whether to format the message in bold. @@ -67,13 +67,12 @@ def echo_debug(message: str, bold: bool = False, nl: bool = True, err: bool = Fa :param err: whether to log to stderr. :param prefix: whether the message should be prefixed with a colored version of the log level. """ - prefix = click.style('Debug: ', fg=COLORS['debug'], bold=True) if prefix else '' message = click.style(message, bold=bold) - CMDLINE_LOGGER.debug(prefix + message, extra=dict(nl=nl, err=err)) + CMDLINE_LOGGER.debug(message, extra=dict(nl=nl, err=err, prefix=prefix)) def echo_info(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: - """Log an info message to the cmdline logger, prefixed with 'Info:' + """Log an info message to the cmdline logger. :param message: the message to log. :param bold: whether to format the message in bold. @@ -81,13 +80,12 @@ def echo_info(message: str, bold: bool = False, nl: bool = True, err: bool = Fal :param err: whether to log to stderr. :param prefix: whether the message should be prefixed with a colored version of the log level. """ - prefix = click.style('Info: ', fg=COLORS['info'], bold=True) if prefix else '' message = click.style(message, bold=bold) - CMDLINE_LOGGER.info(prefix + message, extra=dict(nl=nl, err=err)) + CMDLINE_LOGGER.info(message, extra=dict(nl=nl, err=err, prefix=prefix)) def echo_report(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: - """Log an report message to the cmdline logger, prefixed with 'Report:' + """Log an report message to the cmdline logger. :param message: the message to log. :param bold: whether to format the message in bold. @@ -95,13 +93,12 @@ def echo_report(message: str, bold: bool = False, nl: bool = True, err: bool = F :param err: whether to log to stderr. :param prefix: whether the message should be prefixed with a colored version of the log level. """ - prefix = click.style('Report: ', fg=COLORS['report'], bold=True) if prefix else '' message = click.style(message, bold=bold) - CMDLINE_LOGGER.report(prefix + message, extra=dict(nl=nl, err=err)) + CMDLINE_LOGGER.report(message, extra=dict(nl=nl, err=err, prefix=prefix)) def echo_success(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: - """Log a success message to the cmdline logger, prefixed with 'Success:' + """Log a success message to the cmdline logger. :param message: the message to log. :param bold: whether to format the message in bold. @@ -109,13 +106,12 @@ def echo_success(message: str, bold: bool = False, nl: bool = True, err: bool = :param err: whether to log to stderr. :param prefix: whether the message should be prefixed with a colored version of the log level. """ - prefix = click.style('Success: ', fg=COLORS['success'], bold=True) if prefix else '' message = click.style(message, bold=bold) - CMDLINE_LOGGER.report(prefix + message, extra=dict(nl=nl, err=err)) + CMDLINE_LOGGER.report(message, extra=dict(nl=nl, err=err, prefix=prefix)) def echo_warning(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: - """Log a warning message to the cmdline logger, prefixed with 'Warning:' + """Log a warning message to the cmdline logger. :param message: the message to log. :param bold: whether to format the message in bold. @@ -123,13 +119,12 @@ def echo_warning(message: str, bold: bool = False, nl: bool = True, err: bool = :param err: whether to log to stderr. :param prefix: whether the message should be prefixed with a colored version of the log level. """ - prefix = click.style('Warning: ', fg=COLORS['warning'], bold=True) if prefix else '' message = click.style(message, bold=bold) - CMDLINE_LOGGER.warning(prefix + message, extra=dict(nl=nl, err=err)) + CMDLINE_LOGGER.warning(message, extra=dict(nl=nl, err=err, prefix=prefix)) def echo_error(message: str, bold: bool = False, nl: bool = True, err: bool = True, prefix: bool = True) -> None: - """Log an error message to the cmdline logger, prefixed with 'Error:' + """Log an error message to the cmdline logger. :param message: the message to log. :param bold: whether to format the message in bold. @@ -137,13 +132,12 @@ def echo_error(message: str, bold: bool = False, nl: bool = True, err: bool = Tr :param err: whether to log to stderr. :param prefix: whether the message should be prefixed with a colored version of the log level. """ - prefix = click.style('Error: ', fg=COLORS['error'], bold=True) if prefix else '' message = click.style(message, bold=bold) - CMDLINE_LOGGER.error(prefix + message, extra=dict(nl=nl, err=err)) + CMDLINE_LOGGER.error(message, extra=dict(nl=nl, err=err, prefix=prefix)) def echo_critical(message: str, bold: bool = False, nl: bool = True, err: bool = True, prefix: bool = True) -> None: - """Log a critical error message to the cmdline logger, prefixed with 'Critical:' and exit with ``exit_status``. + """Log a critical error message to the cmdline logger and exit with ``exit_status``. This should be used to print messages for errors that cannot be recovered from and so the script should be directly terminated with a non-zero exit status to indicate that the command failed. @@ -154,9 +148,8 @@ def echo_critical(message: str, bold: bool = False, nl: bool = True, err: bool = :param err: whether to log to stderr. :param prefix: whether the message should be prefixed with a colored version of the log level. """ - prefix = click.style('Critical: ', fg=COLORS['critical'], bold=True) if prefix else '' message = click.style(message, bold=bold) - CMDLINE_LOGGER.critical(prefix + message, extra=dict(nl=nl, err=err)) + CMDLINE_LOGGER.critical(message, extra=dict(nl=nl, err=err, prefix=prefix)) sys.exit(ExitCode.CRITICAL) diff --git a/aiida/cmdline/utils/log.py b/aiida/cmdline/utils/log.py new file mode 100644 index 0000000000..dbc715e7ad --- /dev/null +++ b/aiida/cmdline/utils/log.py @@ -0,0 +1,57 @@ +# -*- coding: utf-8 -*- +"""Utilities for logging in the command line interface context.""" +import logging + +import click + +from .echo import COLORS + + +class CliHandler(logging.Handler): + """Handler for writing to the console using click.""" + + def emit(self, record): + """Emit log record via click. + + Can make use of special attributes 'nl' (whether to add newline) and 'err' (whether to print to stderr), which + can be set via the 'extra' dictionary parameter of the logging methods. + """ + try: + nl = record.nl + except AttributeError: + nl = True + + try: + err = record.err + except AttributeError: + err = False + + try: + prefix = record.prefix + except AttributeError: + prefix = True + + record.prefix = prefix + + try: + msg = self.format(record) + click.echo(msg, err=err, nl=nl) + except Exception: # pylint: disable=broad-except + self.handleError(record) + + +class CliFormatter(logging.Formatter): + """Formatter that automatically prefixes log messages with a colored version of the log level.""" + + @staticmethod + def format(record): + """Format the record using the style required for the command line interface.""" + try: + fg = COLORS[record.levelname.lower()] + except KeyError: + fg = 'white' + + if record.prefix: + return f'{click.style(record.levelname.capitalize(), fg=fg, bold=True)}: {record.msg % record.args}' + + return f'{record.msg % record.args}' diff --git a/aiida/common/log.py b/aiida/common/log.py index 433fcd6bbf..7675f1620f 100644 --- a/aiida/common/log.py +++ b/aiida/common/log.py @@ -48,30 +48,6 @@ def report(self, msg, *args, **kwargs): AIIDA_LOGGER = logging.getLogger('aiida') -class ClickHandler(logging.Handler): - """Handler for writing to the console using click.""" - - def emit(self, record): - """Emit log record via click. - Can make use of special attributes 'nl' (whether to add newline) and 'err' (whether to print to stderr), which - can be set via the 'extra' dictionary parameter of the logging methods. - """ - import click - try: - msg = self.format(record) - try: - nl = record.nl - except AttributeError: - nl = True - try: - err = record.err - except AttributeError: - err = False - click.echo(msg, err=err, nl=nl) - except Exception: # pylint: disable=broad-except - self.handleError(record) - - # The default logging dictionary for AiiDA that can be used in conjunction # with the config.dictConfig method of python's logging module def get_logging_config(): @@ -89,14 +65,18 @@ def get_logging_config(): 'format': '%(asctime)s <%(process)d> %(name)s: [%(levelname)s] %(message)s', 'datefmt': '%m/%d/%Y %I:%M:%S %p', }, + 'cli': { + 'class': 'aiida.cmdline.utils.log.CliFormatter' + } }, 'handlers': { 'console': { 'class': 'logging.StreamHandler', 'formatter': 'halfverbose', }, - 'click': { - 'class': 'aiida.common.log.ClickHandler', + 'cli': { + 'class': 'aiida.cmdline.utils.log.CliHandler', + 'formatter': 'cli', } }, 'loggers': { @@ -106,7 +86,7 @@ def get_logging_config(): 'propagate': False, }, 'aiida.cmdline': { - 'handlers': ['click'], + 'handlers': ['cli'], 'propagate': False, }, 'plumpy': { @@ -167,7 +147,7 @@ def evaluate_logging_configuration(dictionary): return result -def configure_logging(with_orm=False, daemon=False, daemon_log_file=None): +def configure_logging(with_orm=False, daemon=False, daemon_log_file=None, cli=False): """ Setup the logging by retrieving the LOGGING dictionary from aiida and passing it to the python module logging.config.dictConfig. If the logging needs to be setup for the @@ -216,6 +196,10 @@ def configure_logging(with_orm=False, daemon=False, daemon_log_file=None): except ValueError: pass + if cli is True: + for logger in config['loggers'].values(): + logger['handlers'] = ['cli'] + # Add the `DbLogHandler` if `with_orm` is `True` if with_orm: From 35fe868fc4b4c2c5adf105dd782360415c0f2b2f Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Tue, 24 Aug 2021 15:00:49 +0200 Subject: [PATCH 7/7] Logging: remove `override_log_formatter*` utilities The `override_log_formatter` and `override_log_formatter_context` utilities of the `aiida.common.log` module were used in certain CLI commands to temporarily change the formatter of the logger. This was done because they used the logging system as the way to output messages to the terminal, but the default logger prefixed messages with extensive information such as timestamps and the name of the logger, where only the message was required. These utilities are no longer needed since this custom formatting for the CLI is now done ensured by configuring the `CliFormatter` custom formatter which takes care of only including the log level as prefix. --- aiida/cmdline/commands/cmd_archive.py | 20 +- aiida/cmdline/commands/cmd_code.py | 4 +- aiida/cmdline/commands/cmd_group.py | 4 +- aiida/cmdline/commands/cmd_node.py | 4 +- aiida/cmdline/utils/echo.py | 8 +- aiida/common/__init__.py | 1 - aiida/common/log.py | 43 +- setup.json | 2 +- tests/cmdline/commands/conftest.py | 8 +- tests/cmdline/commands/test_archive_export.py | 440 +++++++----------- tests/conftest.py | 6 +- tests/engine/test_calc_job.py | 12 +- 12 files changed, 198 insertions(+), 354 deletions(-) diff --git a/aiida/cmdline/commands/cmd_archive.py b/aiida/cmdline/commands/cmd_archive.py index 84c967c8f2..2f51249d2e 100644 --- a/aiida/cmdline/commands/cmd_archive.py +++ b/aiida/cmdline/commands/cmd_archive.py @@ -120,7 +120,6 @@ def create( You can modify some of those rules using options of this command. """ # pylint: disable=too-many-branches - from aiida.common.log import override_log_formatter_context from aiida.common.progress_reporter import set_progress_bar_tqdm, set_progress_reporter from aiida.tools.importexport import export, ExportFileFormat from aiida.tools.importexport.common.exceptions import ArchiveExportError @@ -165,14 +164,13 @@ def create( elif archive_format == 'null': export_format = 'null' - if AIIDA_LOGGER.level <= logging.INFO: + if AIIDA_LOGGER.level <= logging.REPORT: # pylint: disable=no-member set_progress_bar_tqdm(leave=(AIIDA_LOGGER.level == logging.DEBUG)) else: set_progress_reporter(None) try: - with override_log_formatter_context('%(message)s'): - export(entities, filename=output_file, file_format=export_format, **kwargs) + export(entities, filename=output_file, file_format=export_format, **kwargs) except ArchiveExportError as exception: echo.echo_critical(f'failed to write the archive file. Exception: {exception}') else: @@ -198,7 +196,6 @@ def create( ) def migrate(input_file, output_file, force, in_place, archive_format, version): """Migrate an export archive to a more recent format version.""" - from aiida.common.log import override_log_formatter_context from aiida.common.progress_reporter import set_progress_bar_tqdm, set_progress_reporter from aiida.tools.importexport import detect_archive_type, EXPORT_VERSION from aiida.tools.importexport.archive.migrators import get_migrator @@ -213,7 +210,7 @@ def migrate(input_file, output_file, force, in_place, archive_format, version): 'no output file specified. Please add --in-place flag if you would like to migrate in place.' ) - if AIIDA_LOGGER.level <= logging.INFO: + if AIIDA_LOGGER.level <= logging.REPORT: # pylint: disable=no-member set_progress_bar_tqdm(leave=(AIIDA_LOGGER.level == logging.DEBUG)) else: set_progress_reporter(None) @@ -225,8 +222,7 @@ def migrate(input_file, output_file, force, in_place, archive_format, version): migrator = migrator_cls(input_file) try: - with override_log_formatter_context('%(message)s'): - migrator.migrate(version, output_file, force=force, out_compression=archive_format) + migrator.migrate(version, output_file, force=force, out_compression=archive_format) except Exception as error: # pylint: disable=broad-except if AIIDA_LOGGER.level <= logging.DEBUG: raise @@ -310,10 +306,9 @@ def import_archive( The archive can be specified by its relative or absolute file path, or its HTTP URL. """ # pylint: disable=unused-argument - from aiida.common.log import override_log_formatter_context from aiida.common.progress_reporter import set_progress_bar_tqdm, set_progress_reporter - if AIIDA_LOGGER.level <= logging.INFO: + if AIIDA_LOGGER.level <= logging.REPORT: # pylint: disable=no-member set_progress_bar_tqdm(leave=(AIIDA_LOGGER.level == logging.DEBUG)) else: set_progress_reporter(None) @@ -332,9 +327,8 @@ def import_archive( 'comment_mode': comment_mode, } - with override_log_formatter_context('%(message)s'): - for archive, web_based in all_archives: - _import_archive(archive, web_based, import_kwargs, migration) + for archive, web_based in all_archives: + _import_archive(archive, web_based, import_kwargs, migration) def _echo_exception(msg: str, exception, warn_only: bool = False): diff --git a/aiida/cmdline/commands/cmd_code.py b/aiida/cmdline/commands/cmd_code.py index a66212922b..78665d01b6 100644 --- a/aiida/cmdline/commands/cmd_code.py +++ b/aiida/cmdline/commands/cmd_code.py @@ -189,7 +189,6 @@ def delete(codes, dry_run, force): Note that codes are part of the data provenance, and deleting a code will delete all calculations using it. """ - from aiida.common.log import override_log_formatter_context from aiida.tools import delete_nodes node_pks_to_delete = [code.pk for code in codes] @@ -200,8 +199,7 @@ def _dry_run_callback(pks): echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') return not click.confirm('Shall I continue?', abort=True) - with override_log_formatter_context('%(message)s'): - _, was_deleted = delete_nodes(node_pks_to_delete, dry_run=dry_run or _dry_run_callback) + was_deleted = delete_nodes(node_pks_to_delete, dry_run=dry_run or _dry_run_callback) if was_deleted: echo.echo_success('Finished deletion.') diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index 46d0b86339..13e31a68d4 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -95,7 +95,6 @@ def group_remove_nodes(group, nodes, clear, force): @with_dbenv() def group_delete(group, delete_nodes, dry_run, force, **traversal_rules): """Delete a group and (optionally) the nodes it contains.""" - from aiida.common.log import override_log_formatter_context from aiida.tools import delete_group_nodes from aiida import orm @@ -112,8 +111,7 @@ def _dry_run_callback(pks): echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') return not click.confirm('Do you want to continue?', abort=True) - with override_log_formatter_context('%(message)s'): - _, nodes_deleted = delete_group_nodes([group.pk], dry_run=dry_run or _dry_run_callback, **traversal_rules) + _, nodes_deleted = delete_group_nodes([group.pk], dry_run=dry_run or _dry_run_callback, **traversal_rules) if not nodes_deleted: # don't delete the group if the nodes were not deleted return diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 9be18aff3e..2a40625a44 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -290,7 +290,6 @@ def node_delete(identifier, dry_run, force, **traversal_rules): the nodes necessary to keep a consistent graph, according to the rules outlined in the documentation. You can modify some of those rules using options of this command. """ - from aiida.common.log import override_log_formatter_context from aiida.orm.utils.loaders import NodeEntityLoader from aiida.tools import delete_nodes @@ -309,8 +308,7 @@ def _dry_run_callback(pks): echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') return not click.confirm('Shall I continue?', abort=True) - with override_log_formatter_context('%(message)s'): - _, was_deleted = delete_nodes(pks, dry_run=dry_run or _dry_run_callback, **traversal_rules) + _, was_deleted = delete_nodes(pks, dry_run=dry_run or _dry_run_callback, **traversal_rules) if was_deleted: echo.echo_success('Finished deletion.') diff --git a/aiida/cmdline/utils/echo.py b/aiida/cmdline/utils/echo.py index 7e5e20a19c..2ec5834e07 100644 --- a/aiida/cmdline/utils/echo.py +++ b/aiida/cmdline/utils/echo.py @@ -100,6 +100,8 @@ def echo_report(message: str, bold: bool = False, nl: bool = True, err: bool = F def echo_success(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: """Log a success message to the cmdline logger. + .. note:: The message will be logged at the ``REPORT`` level and always with the ``Success:`` prefix. + :param message: the message to log. :param bold: whether to format the message in bold. :param nl: whether to add a newline at the end of the message. @@ -107,7 +109,11 @@ def echo_success(message: str, bold: bool = False, nl: bool = True, err: bool = :param prefix: whether the message should be prefixed with a colored version of the log level. """ message = click.style(message, bold=bold) - CMDLINE_LOGGER.report(message, extra=dict(nl=nl, err=err, prefix=prefix)) + + if prefix: + message = click.style('Success: ', bold=True, fg=COLORS['success']) + message + + CMDLINE_LOGGER.report(message, extra=dict(nl=nl, err=err, prefix=False)) def echo_warning(message: str, bold: bool = False, nl: bool = True, err: bool = False, prefix: bool = True) -> None: diff --git a/aiida/common/__init__.py b/aiida/common/__init__.py index 1143da1861..5a4963a697 100644 --- a/aiida/common/__init__.py +++ b/aiida/common/__init__.py @@ -81,7 +81,6 @@ 'ValidationError', 'create_callback', 'get_progress_reporter', - 'override_log_formatter', 'override_log_level', 'set_progress_bar_tqdm', 'set_progress_reporter', diff --git a/aiida/common/log.py b/aiida/common/log.py index 7675f1620f..527861ad0b 100644 --- a/aiida/common/log.py +++ b/aiida/common/log.py @@ -9,13 +9,11 @@ ########################################################################### """Module for all logging methods/classes that don't need the ORM.""" import collections -import copy +import contextlib import logging import types -from contextlib import contextmanager -from wrapt import decorator -__all__ = ('AIIDA_LOGGER', 'override_log_level', 'override_log_formatter') +__all__ = ('AIIDA_LOGGER', 'override_log_level') # Custom logging level, intended specifically for informative log messages reported during WorkChains. # We want the level between INFO(20) and WARNING(30) such that it will be logged for the default loglevel, however @@ -214,7 +212,7 @@ def configure_logging(with_orm=False, daemon=False, daemon_log_file=None, cli=Fa dictConfig(config) -@contextmanager +@contextlib.contextmanager def override_log_level(level=logging.CRITICAL): """Temporarily adjust the log-level of logger.""" logging.disable(level=level) @@ -222,38 +220,3 @@ def override_log_level(level=logging.CRITICAL): yield finally: logging.disable(level=logging.NOTSET) - - -@contextmanager -def override_log_formatter_context(fmt: str): - """Temporarily use a different formatter for all handlers. - - NOTE: One can _only_ set `fmt` (not `datefmt` or `style`). - """ - temp_formatter = logging.Formatter(fmt=fmt) - cached_formatters = {} - - for handler in AIIDA_LOGGER.handlers: - # Need a copy here so we keep the original one should the handler's formatter be changed during the yield - cached_formatters[handler] = copy.copy(handler.formatter) - handler.setFormatter(temp_formatter) - - yield - - for handler, formatter in cached_formatters.items(): - handler.setFormatter(formatter) - - -def override_log_formatter(fmt: str): - """Temporarily use a different formatter for all handlers. - - NOTE: One can _only_ set `fmt` (not `datefmt` or `style`). - Be aware! This may fail if the number of handlers is changed within the decorated function/method. - """ - - @decorator - def wrapper(wrapped, instance, args, kwargs): # pylint: disable=unused-argument - with override_log_formatter_context(fmt=fmt): - return wrapped(*args, **kwargs) - - return wrapper diff --git a/setup.json b/setup.json index f50e2d6e83..3050611fee 100644 --- a/setup.json +++ b/setup.json @@ -105,7 +105,7 @@ "aiida-export-migration-tests==0.9.0", "pg8000~=1.13", "pgtest~=1.3,>=1.3.1", - "pytest~=6.0", + "pytest~=6.2", "pytest-asyncio~=0.12", "pytest-timeout~=1.3", "pytest-cov~=2.7,<2.11", diff --git a/tests/cmdline/commands/conftest.py b/tests/cmdline/commands/conftest.py index 2c3b30d99d..c760b95a92 100644 --- a/tests/cmdline/commands/conftest.py +++ b/tests/cmdline/commands/conftest.py @@ -8,6 +8,8 @@ # For further information please visit http://www.aiida.net # ########################################################################### """Pytest fixtures for command line interface tests.""" +import pathlib + import click import pytest @@ -33,13 +35,17 @@ def _run_cli_command(command: click.Command, options: list = None, raises: bool """ import traceback + # Convert any ``pathlib.Path`` objects in the ``options`` to their absolute filepath string representation. + # This is necessary because the ``invoke`` command does not support these path objects. + options = [str(option) if isinstance(option, pathlib.Path) else option for option in options or []] + # We need to apply the ``VERBOSITY`` option. When invoked through the command line, this is done by the logic # of the ``VerdiCommandGroup``, but when testing commands, the command is retrieved directly from the module # which circumvents this machinery. command = VerdiCommandGroup.add_verbosity_option(command) runner = click.testing.CliRunner() - result = runner.invoke(command, options or []) + result = runner.invoke(command, options) if raises: assert result.exception is not None, result.output diff --git a/tests/cmdline/commands/test_archive_export.py b/tests/cmdline/commands/test_archive_export.py index 59262b655f..146d5e0954 100644 --- a/tests/cmdline/commands/test_archive_export.py +++ b/tests/cmdline/commands/test_archive_export.py @@ -7,293 +7,177 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### -# pylint: disable=consider-using-with """Tests for `verdi export`.""" -import errno -import os import shutil -import tempfile import tarfile -import traceback import zipfile -from click.testing import CliRunner +import pytest -from aiida.backends.testbase import AiidaTestCase from aiida.cmdline.commands import cmd_archive +from aiida.orm import Computer, Code, Group, Data from aiida.tools.importexport import EXPORT_VERSION, ReaderJsonZip from tests.utils.archives import get_archive_file +pytest.mark.usefixtures('chdir_tmp_path') -def delete_temporary_file(filepath): - """Attempt to delete a file, given an absolute path. If the deletion fails because the file does not exist - the exception will be caught and passed. Any other exceptions will raise. - - :param filepath: the absolute file path""" - - try: - os.remove(filepath) - except OSError as exception: - if exception.errno != errno.ENOENT: - raise - else: - pass - - -class TestVerdiExport(AiidaTestCase): - """Tests for `verdi export`.""" - - @classmethod - def setUpClass(cls): - super().setUpClass() - from aiida import orm - - cls.computer = orm.Computer( - label='comp', hostname='localhost', transport_type='local', scheduler_type='direct', workdir='/tmp/aiida' - ).store() - - cls.code = orm.Code(remote_computer_exec=(cls.computer, '/bin/true')).store() - cls.group = orm.Group(label='test_group').store() - cls.node = orm.Data().store() - - # some of the export tests write in the current directory, - # make sure it is writeable and we don't pollute the current one - cls.old_cwd = os.getcwd() - cls.cwd = tempfile.mkdtemp(__name__) - os.chdir(cls.cwd) - - # Utility helper - cls.fixture_archive = 'export/migrate' - cls.newest_archive = f'export_v{EXPORT_VERSION}_simple.aiida' - cls.penultimate_archive = 'export_v0.6_simple.aiida' - - @classmethod - def tearDownClass(cls): - os.chdir(cls.old_cwd) - shutil.rmtree(cls.cwd, ignore_errors=True) - - def setUp(self): - self.cli_runner = CliRunner() - - def test_create_file_already_exists(self): - """Test that using a file that already exists, which is the case when using NamedTemporaryFile, will raise.""" - with tempfile.NamedTemporaryFile() as handle: - options = [handle.name] - result = self.cli_runner.invoke(cmd_archive.create, options) - self.assertIsNotNone(result.exception) - - def test_create_force(self): - """ - Test that using a file that already exists, which is the case when using NamedTemporaryFile, will work - when the -f/--force parameter is used - """ - with tempfile.NamedTemporaryFile() as handle: - options = ['-f', handle.name] - result = self.cli_runner.invoke(cmd_archive.create, options) - self.assertIsNone(result.exception, result.output) - - options = ['--force', handle.name] - result = self.cli_runner.invoke(cmd_archive.create, options) - self.assertIsNone(result.exception, result.output) - - def test_create_zip(self): - """Test that creating an archive for a set of various ORM entities works with the zip format.""" - filename = next(tempfile._get_candidate_names()) # pylint: disable=protected-access - try: - options = [ - '-X', self.code.pk, '-Y', self.computer.pk, '-G', self.group.pk, '-N', self.node.pk, '-F', 'zip', - filename - ] - result = self.cli_runner.invoke(cmd_archive.create, options) - self.assertIsNone(result.exception, ''.join(traceback.format_exception(*result.exc_info))) - self.assertTrue(os.path.isfile(filename)) - self.assertFalse(zipfile.ZipFile(filename).testzip(), None) - finally: - delete_temporary_file(filename) - - def test_create_zip_uncompressed(self): - """Test that creating an archive for a set of various ORM entities works with the zip-uncompressed format.""" - filename = next(tempfile._get_candidate_names()) # pylint: disable=protected-access - try: - options = [ - '-X', self.code.pk, '-Y', self.computer.pk, '-G', self.group.pk, '-N', self.node.pk, '-F', - 'zip-uncompressed', filename - ] - result = self.cli_runner.invoke(cmd_archive.create, options) - self.assertIsNone(result.exception, ''.join(traceback.format_exception(*result.exc_info))) - self.assertTrue(os.path.isfile(filename)) - self.assertFalse(zipfile.ZipFile(filename).testzip(), None) - finally: - delete_temporary_file(filename) - - def test_create_tar_gz(self): - """Test that creating an archive for a set of various ORM entities works with the tar.gz format.""" - filename = next(tempfile._get_candidate_names()) # pylint: disable=protected-access - try: - options = [ - '-X', self.code.pk, '-Y', self.computer.pk, '-G', self.group.pk, '-N', self.node.pk, '-F', 'tar.gz', - filename - ] - result = self.cli_runner.invoke(cmd_archive.create, options) - self.assertIsNone(result.exception, ''.join(traceback.format_exception(*result.exc_info))) - self.assertTrue(os.path.isfile(filename)) - self.assertTrue(tarfile.is_tarfile(filename)) - finally: - delete_temporary_file(filename) - - def test_migrate_versions_old(self): - """Migrating archives with a version older than the current should work.""" - archives = [] - for version in range(1, int(EXPORT_VERSION.rsplit('.', maxsplit=1)[-1]) - 1): - archives.append(f'export_v0.{version}_simple.aiida') - - for archive in archives: - - filename_input = get_archive_file(archive, filepath=self.fixture_archive) - filename_output = next(tempfile._get_candidate_names()) # pylint: disable=protected-access - - try: - options = ['--verbosity', 'DEBUG', filename_input, filename_output] - result = self.cli_runner.invoke(cmd_archive.migrate, options) - self.assertIsNone(result.exception, result.output) - self.assertTrue(os.path.isfile(filename_output)) - self.assertEqual(zipfile.ZipFile(filename_output).testzip(), None) - finally: - delete_temporary_file(filename_output) - - def test_migrate_version_specific(self): - """Test the `-v/--version` option to migrate to a specific version instead of the latest.""" - archive = 'export_v0.1_simple.aiida' - target_version = '0.2' - - filename_input = get_archive_file(archive, filepath=self.fixture_archive) - filename_output = next(tempfile._get_candidate_names()) # pylint: disable=protected-access - - try: - options = [filename_input, filename_output, '--version', target_version] - result = self.cli_runner.invoke(cmd_archive.migrate, options) - self.assertIsNone(result.exception, result.output) - self.assertTrue(os.path.isfile(filename_output)) - self.assertEqual(zipfile.ZipFile(filename_output).testzip(), None) - - with ReaderJsonZip(filename_output) as archive_object: - self.assertEqual(archive_object.metadata.export_version, target_version) - finally: - delete_temporary_file(filename_output) - - def test_migrate_force(self): - """Test that passing the -f/--force option will overwrite the output file even if it exists.""" - filename_input = get_archive_file(self.penultimate_archive, filepath=self.fixture_archive) - - # Using the context manager will create the file and so the command should fail - with tempfile.NamedTemporaryFile() as file_output: - options = [filename_input, file_output.name] - result = self.cli_runner.invoke(cmd_archive.migrate, options) - self.assertIsNotNone(result.exception) - - for option in ['-f', '--force']: - # Using the context manager will create the file, but we pass the force flag so it should work - with tempfile.NamedTemporaryFile() as file_output: - filename_output = file_output.name - options = [option, filename_input, filename_output] - result = self.cli_runner.invoke(cmd_archive.migrate, options) - self.assertIsNone(result.exception, result.output) - self.assertTrue(os.path.isfile(filename_output)) - self.assertEqual(zipfile.ZipFile(filename_output).testzip(), None) - - def test_migrate_in_place(self): - """Test that passing the -i/--in-place option will overwrite the passed file.""" - archive = 'export_v0.1_simple.aiida' - target_version = '0.2' - filename_input = get_archive_file(archive, filepath=self.fixture_archive) - filename_tmp = next(tempfile._get_candidate_names()) # pylint: disable=protected-access - - try: - # copy file (don't want to overwrite test data) - shutil.copy(filename_input, filename_tmp) - - # specifying both output and in-place should except - options = [filename_tmp, '--in-place', '--output-file', 'test.aiida'] - result = self.cli_runner.invoke(cmd_archive.migrate, options) - self.assertIsNotNone(result.exception, result.output) - - # specifying neither output nor in-place should except - options = [filename_tmp] - result = self.cli_runner.invoke(cmd_archive.migrate, options) - self.assertIsNotNone(result.exception, result.output) - - # check that in-place migration produces a valid archive in place of the old file - options = [filename_tmp, '--in-place', '--version', target_version] - result = self.cli_runner.invoke(cmd_archive.migrate, options) - self.assertIsNone(result.exception, result.output) - self.assertTrue(os.path.isfile(filename_tmp)) - # check that files in zip file are ok - self.assertEqual(zipfile.ZipFile(filename_tmp).testzip(), None) - with ReaderJsonZip(filename_tmp) as archive_object: - self.assertEqual(archive_object.metadata.export_version, target_version) - finally: - os.remove(filename_tmp) - - def test_migrate_low_verbosity(self): - """Test that the captured output is an empty string when the ``--verbosity WARNING`` option is passed.""" - filename_input = get_archive_file(self.penultimate_archive, filepath=self.fixture_archive) - filename_output = next(tempfile._get_candidate_names()) # pylint: disable=protected-access - - delete_temporary_file(filename_output) - - for option in ['--verbosity']: - try: - options = [option, 'WARNING', filename_input, filename_output] - result = self.cli_runner.invoke(cmd_archive.migrate, options) - self.assertEqual(result.output, '') - self.assertIsNone(result.exception, result.output) - self.assertTrue(os.path.isfile(filename_output)) - self.assertEqual(zipfile.ZipFile(filename_output).testzip(), None) - finally: - delete_temporary_file(filename_output) - - def test_migrate_tar_gz(self): - """Test that -F/--archive-format option can be used to write a tar.gz instead.""" - filename_input = get_archive_file(self.penultimate_archive, filepath=self.fixture_archive) - filename_output = next(tempfile._get_candidate_names()) # pylint: disable=protected-access - - for option in ['-F', '--archive-format']: - try: - options = [option, 'tar.gz', filename_input, filename_output] - result = self.cli_runner.invoke(cmd_archive.migrate, options) - self.assertIsNone(result.exception, result.output) - self.assertTrue(os.path.isfile(filename_output)) - self.assertTrue(tarfile.is_tarfile(filename_output)) - finally: - delete_temporary_file(filename_output) - - def test_inspect(self): - """Test the functionality of `verdi export inspect`.""" - archives = [] - for version in range(1, int(EXPORT_VERSION.rsplit('.', maxsplit=1)[-1])): - archives.append((f'export_v0.{version}_simple.aiida', f'0.{version}')) - - for archive, version_number in archives: - - filename_input = get_archive_file(archive, filepath=self.fixture_archive) - - # Test the options that will print the meta data - options = ['-m', filename_input] - result = self.cli_runner.invoke(cmd_archive.inspect, options) - self.assertIsNone(result.exception, result.output) - - # Test the --version option which should print the archive format version - options = ['--version', filename_input] - result = self.cli_runner.invoke(cmd_archive.inspect, options) - self.assertIsNone(result.exception, result.output) - self.assertEqual(result.output.strip()[-len(version_number):], version_number) - - def test_inspect_empty_archive(self): - """Test the functionality of `verdi export inspect` for an empty archive.""" - filename_input = get_archive_file('empty.aiida', filepath=self.fixture_archive) - - options = [filename_input] - result = self.cli_runner.invoke(cmd_archive.inspect, options) - self.assertIsNotNone(result.exception, result.output) - self.assertIn('corrupt archive', result.output) + +def test_create_file_already_exists(run_cli_command, tmp_path): + """Test that using a file that already exists will raise.""" + assert tmp_path.exists() + options = [tmp_path] + run_cli_command(cmd_archive.create, options, raises=True) + + +def test_create_force(run_cli_command, tmp_path): + """Test that using a file that already exists will work when the ``-f/--force`` parameter is used.""" + assert tmp_path.exists() + options = ['--force', tmp_path] + run_cli_command(cmd_archive.create, options) + + +@pytest.mark.parametrize('fmt', ('zip', 'zip-uncompressed', 'tar.gz')) +@pytest.mark.usefixtures('clear_database_before_test') +def test_create_compressed(run_cli_command, tmp_path, fmt): + """Test that creating an archive for a set of various ORM entities works with the zip format.""" + computer = Computer( + label='comp', hostname='localhost', transport_type='local', scheduler_type='direct', workdir='/tmp/aiida' + ).store() + code = Code(remote_computer_exec=(computer, '/bin/true')).store() + group = Group(label='test_group').store() + node = Data().store() + filename_output = tmp_path / 'archive.aiida' + + options = ['-X', code.pk, '-Y', computer.pk, '-G', group.pk, '-N', node.pk, '-F', fmt, filename_output] + run_cli_command(cmd_archive.create, options) + assert filename_output.is_file() + + if fmt.startswith('zip'): + assert zipfile.ZipFile(filename_output).testzip() is None # pylint: disable=consider-using-with + else: + assert tarfile.is_tarfile(filename_output) + + +@pytest.mark.parametrize('version', range(1, int(EXPORT_VERSION.rsplit('.', maxsplit=1)[-1]) - 1)) +def test_migrate_versions_old(run_cli_command, tmp_path, version): + """Migrating archives with a version older than the current should work.""" + archive = f'export_v0.{version}_simple.aiida' + filename_input = get_archive_file(archive, filepath='export/migrate') + filename_output = tmp_path / 'archive.aiida' + + options = [filename_input, filename_output] + run_cli_command(cmd_archive.migrate, options) + assert filename_output.is_file() + assert zipfile.ZipFile(filename_output).testzip() is None # pylint: disable=consider-using-with + + +def test_migrate_version_specific(run_cli_command, tmp_path): + """Test the `-v/--version` option to migrate to a specific version instead of the latest.""" + archive = 'export_v0.1_simple.aiida' + target_version = '0.2' + filename_input = get_archive_file(archive, filepath='export/migrate') + filename_output = tmp_path / 'archive.aiida' + + options = [filename_input, filename_output, '--version', target_version] + run_cli_command(cmd_archive.migrate, options) + assert filename_output.is_file() + assert zipfile.ZipFile(filename_output).testzip() is None # pylint: disable=consider-using-with + + with ReaderJsonZip(filename_output) as archive_object: + assert archive_object.metadata.export_version == target_version + + +def test_migrate_file_already_exists(run_cli_command, tmp_path): + """Test that using a file that already exists will raise.""" + assert tmp_path.exists() + filename_input = get_archive_file('export_v0.6_simple.aiida', filepath='export/migrate') + options = [filename_input, tmp_path] + run_cli_command(cmd_archive.migrate, options, raises=True) + + +def test_migrate_force(run_cli_command, tmp_path): + """Test that using a file that already exists will work when the ``-f/--force`` parameter is used.""" + assert tmp_path.exists() + filename_input = get_archive_file('export_v0.6_simple.aiida', filepath='export/migrate') + options = ['--force', filename_input, tmp_path] + run_cli_command(cmd_archive.migrate, options) + + +def test_migrate_in_place(run_cli_command, tmp_path): + """Test that passing the -i/--in-place option will overwrite the passed file.""" + archive = 'export_v0.1_simple.aiida' + target_version = '0.2' + filename_input = get_archive_file(archive, filepath='export/migrate') + filename_clone = tmp_path / 'archive.aiida' + + # copy file (don't want to overwrite test data) + shutil.copy(filename_input, filename_clone) + + # specifying both output and in-place should except + options = [filename_clone, '--in-place', '--output-file', 'test.aiida'] + run_cli_command(cmd_archive.migrate, options, raises=True) + + # specifying neither output nor in-place should except + options = [filename_clone] + run_cli_command(cmd_archive.migrate, options, raises=True) + + # check that in-place migration produces a valid archive in place of the old file + options = [filename_clone, '--in-place', '--version', target_version] + run_cli_command(cmd_archive.migrate, options) + assert filename_clone.is_file() + assert zipfile.ZipFile(filename_clone).testzip() is None # pylint: disable=consider-using-with + + with ReaderJsonZip(filename_clone) as archive_object: + assert archive_object.metadata.export_version == target_version + + +@pytest.mark.usefixtures('config_with_profile') +def test_migrate_low_verbosity(run_cli_command, tmp_path): + """Test that the captured output is an empty string when the ``--verbosity WARNING`` option is passed. + + Note that we use the ``config_with_profile`` fixture to create a dummy profile, since the ``--verbosity`` option + will change the profile configuration which could potentially influence the other tests. + """ + filename_input = get_archive_file('export_v0.6_simple.aiida', filepath='export/migrate') + filename_output = tmp_path / 'archive.aiida' + + options = ['--verbosity', 'WARNING', filename_input, filename_output] + result = run_cli_command(cmd_archive.migrate, options) + assert result.output == '' + assert filename_output.is_file() + assert zipfile.ZipFile(filename_output).testzip() is None # pylint: disable=consider-using-with + + +def test_migrate_tar_gz(run_cli_command, tmp_path): + """Test that -F/--archive-format option can be used to write a tar.gz instead.""" + filename_input = get_archive_file('export_v0.6_simple.aiida', filepath='export/migrate') + filename_output = tmp_path / 'archive.aiida' + + options = ['--archive-format', 'tar.gz', filename_input, filename_output] + run_cli_command(cmd_archive.migrate, options) + assert filename_output.is_file() + assert tarfile.is_tarfile(filename_output) + + +@pytest.mark.parametrize('version', range(1, int(EXPORT_VERSION.rsplit('.', maxsplit=1)[-1]) - 1)) +def test_inspect(run_cli_command, version): + """Test the functionality of `verdi export inspect`.""" + archive = f'export_v0.{version}_simple.aiida' + filename_input = get_archive_file(archive, filepath='export/migrate') + + # Test the options that will print the meta data + options = ['-m', filename_input] + run_cli_command(cmd_archive.inspect, options) + + # Test the --version option which should print the archive format version + options = ['--version', filename_input] + result = run_cli_command(cmd_archive.inspect, options) + assert result.output.strip() == f'0.{version}' + + +def test_inspect_empty_archive(run_cli_command): + """Test the functionality of `verdi export inspect` for an empty archive.""" + filename_input = get_archive_file('empty.aiida', filepath='export/migrate') + result = run_cli_command(cmd_archive.inspect, [filename_input], raises=True) + assert 'corrupt archive' in result.output diff --git a/tests/conftest.py b/tests/conftest.py index 15df55626e..16f82f870a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -368,8 +368,8 @@ def with_daemon(): @pytest.fixture(scope='function') -def dry_run_in_tmp(request, tmpdir): - """change to the tmp directory to do the dry run, then change back to the calling directory to avoid side-effects""" - os.chdir(tmpdir) +def chdir_tmp_path(request, tmp_path): + """Change to a temporary directory before running the test and reverting to original working directory.""" + os.chdir(tmp_path) yield os.chdir(request.config.invocation_dir) diff --git a/tests/engine/test_calc_job.py b/tests/engine/test_calc_job.py index b901f88958..f6415400b2 100644 --- a/tests/engine/test_calc_job.py +++ b/tests/engine/test_calc_job.py @@ -108,8 +108,7 @@ def prepare_for_submission(self, folder): @pytest.mark.requires_rmq -@pytest.mark.usefixtures('dry_run_in_tmp') -@pytest.mark.usefixtures('clear_database_before_test') +@pytest.mark.usefixtures('clear_database_before_test', 'chdir_tmp_path') @pytest.mark.parametrize('parallel_run', [True, False]) def test_multi_codes_run_parallel(aiida_local_code_factory, file_regression, parallel_run): """test codes_run_mode set in CalcJob""" @@ -138,8 +137,7 @@ def test_multi_codes_run_parallel(aiida_local_code_factory, file_regression, par @pytest.mark.requires_rmq -@pytest.mark.usefixtures('dry_run_in_tmp') -@pytest.mark.usefixtures('clear_database_before_test') +@pytest.mark.usefixtures('clear_database_before_test', 'chdir_tmp_path') @pytest.mark.parametrize('calcjob_withmpi', [True, False]) def test_multi_codes_run_withmpi(aiida_local_code_factory, file_regression, calcjob_withmpi): """test withmpi set in CalcJob only take effect for codes which have codeinfo.withmpi not set""" @@ -373,7 +371,7 @@ def test_exception_presubmit(self): self.assertIn('exception occurred in presubmit call', str(context.exception)) - @pytest.mark.usefixtures('dry_run_in_tmp') + @pytest.mark.usefixtures('chdir_tmp_path') def test_run_local_code(self): """Run a dry-run with local code.""" inputs = deepcopy(self.inputs) @@ -389,7 +387,7 @@ def test_run_local_code(self): for filename in self.local_code.list_object_names(): self.assertTrue(filename in uploaded_files) - @pytest.mark.usefixtures('dry_run_in_tmp') + @pytest.mark.usefixtures('chdir_tmp_path') def test_rerunnable(self): """Test that setting `rerunnable` in the options results in it being set in the job template.""" inputs = deepcopy(self.inputs) @@ -406,7 +404,7 @@ def test_rerunnable(self): assert job_tmpl['rerunnable'] - @pytest.mark.usefixtures('dry_run_in_tmp') + @pytest.mark.usefixtures('chdir_tmp_path') def test_provenance_exclude_list(self): """Test the functionality of the `CalcInfo.provenance_exclude_list` attribute.""" import tempfile