Skip to content

Commit

Permalink
verdi process list: fix logging bug in check_worker_load
Browse files Browse the repository at this point in the history 
The `check_worker_load` utility, called in `verdi process list`, caused
an exception in the logging because it logged a message including a
literal percentage sign. However, percentage signs carry special meaning
as they are placeholders for argument substitution and literal
percentage signs need to be escaped by doubling the character.
  • Loading branch information
@sphuber
sphuber committed Sep 24, 2021
1 parent b7e37ce commit 1071ae6
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 87 deletions.
21 changes: 11 additions & 10 deletions aiida/cmdline/utils/common.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,12 +491,13 @@ def get_num_workers():


def check_worker_load(active_slots):
"""
Check if the percentage usage of the daemon worker slots exceeds a threshold.
If it does, print a warning.
"""Log a message with information on the current daemon worker load.
If there are daemon workers active, it logs the current load. If that exceeds 90%, a warning is included with the
suggestion to run ``verdi daemon incr``.
The purpose of this check is to warn the user if they are close to running out of worker slots
which could lead to their processes becoming stuck indefinitely.
The purpose of this check is to warn the user if they are close to running out of worker slots which could lead to
their processes becoming stuck indefinitely.
:param active_slots: the number of currently active worker slots
"""
Expand All @@ -511,16 +512,16 @@ def check_worker_load(active_slots):
try:
active_workers = get_num_workers()
except CircusCallError:
echo.echo_critical('Could not contact Circus to get the number of active workers')
echo.echo_critical('Could not contact Circus to get the number of active workers.')

if active_workers is not None:
available_slots = active_workers * slots_per_worker
percent_load = 1.0 if not available_slots else (active_slots / available_slots)
if percent_load > warning_threshold:
echo.echo('') # New line
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")
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`.')
else:
echo.echo_report(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_report('No active daemon workers')
echo.echo_report('No active daemon workers.')
172 changes: 95 additions & 77 deletions tests/cmdline/utils/test_common.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,106 +7,124 @@
# For further information on the license, see the LICENSE.txt file #
# For further information please visit http://www.aiida.net #
###########################################################################
"""Tests for common command line utilities."""
"""Tests for the :mod:`aiida.cmdline.utils.common` module."""
import pytest

from aiida.cmdline.utils import common
from aiida.common import LinkType
from aiida.engine import Process, calcfunction
from aiida.orm import CalcFunctionNode, CalculationNode, WorkflowNode


@pytest.mark.usefixtures('clear_database_before_test')
def test_get_node_summary(aiida_local_code_factory):
"""Test the ``get_node_summary`` utility."""
code = aiida_local_code_factory(entry_point='core.arithmetic.add', executable='/bin/bash')
node = CalculationNode()
node.computer = code.computer
node.add_incoming(code, link_type=LinkType.INPUT_CALC, link_label='code')
node.store()

from aiida import orm
from aiida.backends.testbase import AiidaTestCase
from aiida.common.links import LinkType
summary = common.get_node_summary(node)
assert node.uuid in summary
assert node.computer.label in summary


class TestCommonUtilities(AiidaTestCase):
"""Tests for common command line utilities."""
@pytest.mark.usefixtures('clear_database_before_test')
def test_get_node_info_multiple_call_links():
"""Test the ``get_node_info`` utility.
def test_get_node_summary(self):
"""Test the `get_node_summary` utility."""
from aiida.cmdline.utils.common import get_node_summary
Regression test for #2868:
Verify that all `CALL` links are included in the formatted string even if link labels are identical.
"""
workflow = WorkflowNode().store()
node_one = CalculationNode()
node_two = CalculationNode()

computer_label = self.computer.label # pylint: disable=no-member
node_one.add_incoming(workflow, link_type=LinkType.CALL_CALC, link_label='CALL_IDENTICAL')
node_two.add_incoming(workflow, link_type=LinkType.CALL_CALC, link_label='CALL_IDENTICAL')
node_one.store()
node_two.store()

code = orm.Code(
input_plugin_name='core.arithmetic.add',
remote_computer_exec=[self.computer, '/remote/abs/path'],
)
code.store()
node_info = common.get_node_info(workflow)
assert 'CALL_IDENTICAL' in node_info
assert str(node_one.pk) in node_info
assert str(node_two.pk) in node_info

node = orm.CalculationNode()
node.computer = self.computer
node.add_incoming(code, link_type=LinkType.INPUT_CALC, link_label='code')
node.store()

summary = get_node_summary(node)
self.assertIn(node.uuid, summary)
self.assertIn(computer_label, summary)
@pytest.mark.usefixtures('clear_database_before_test')
def test_get_process_function_report():
"""Test the ``get_process_function_report`` utility."""
warning = 'You have been warned'
node = CalcFunctionNode()
node.store()

def test_get_node_info_multiple_call_links(self):
"""Test the `get_node_info` utility.
node.logger.warning(warning)
assert warning in common.get_process_function_report(node)

Regression test for #2868:
Verify that all `CALL` links are included in the formatted string even if link labels are identical.
"""
from aiida.cmdline.utils.common import get_node_info

workflow = orm.WorkflowNode().store()
node_one = orm.CalculationNode()
node_two = orm.CalculationNode()
def test_print_process_info():
"""Test the ``print_process_info`` method."""

node_one.add_incoming(workflow, link_type=LinkType.CALL_CALC, link_label='CALL_IDENTICAL')
node_two.add_incoming(workflow, link_type=LinkType.CALL_CALC, link_label='CALL_IDENTICAL')
node_one.store()
node_two.store()
class TestProcessWithoutDocstring(Process):
# pylint: disable=missing-docstring

node_info = get_node_info(workflow)
self.assertTrue('CALL_IDENTICAL' in node_info)
self.assertTrue(str(node_one.pk) in node_info)
self.assertTrue(str(node_two.pk) in node_info)
@classmethod
def define(cls, spec):
super().define(spec)
spec.input('some_input')

def test_get_process_function_report(self):
"""Test the `get_process_function_report` utility."""
from aiida.cmdline.utils.common import get_process_function_report
class TestProcessWithDocstring(Process):
"""Some docstring."""

warning = 'You have been warned'
@classmethod
def define(cls, spec):
super().define(spec)
spec.input('some_input')

node = orm.CalcFunctionNode()
node.store()
@calcfunction
def test_without_docstring():
pass

# Add a log message through the logger
node.logger.warning(warning)
@calcfunction
def test_with_docstring():
"""Some docstring."""

self.assertIn(warning, get_process_function_report(node))
# We are just checking that the command does not except
common.print_process_info(TestProcessWithoutDocstring)
common.print_process_info(TestProcessWithDocstring)
common.print_process_info(test_without_docstring)
common.print_process_info(test_with_docstring)

def test_print_process_info(self):
"""Test the `print_process_info` method."""
from aiida.cmdline.utils.common import print_process_info
from aiida.common.utils import Capturing
from aiida.engine import Process, calcfunction

class TestProcessWithoutDocstring(Process):
# pylint: disable=missing-docstring
@pytest.mark.parametrize(
'active_workers, active_slots, expected', (
(None, None, 'No active daemon workers.'),
(1, 0, 'Report: Using 0% of the available daemon worker slots.'),
(1, 200, 'Warning: 100% of the available daemon worker slots have been used!'),
)
)
def test_check_worker_load(monkeypatch, capsys, active_workers, active_slots, expected):
"""Test the ``check_worker_load`` function.
@classmethod
def define(cls, spec):
super().define(spec)
spec.input('some_input')
We monkeypatch the ``get_num_workers`` method which is called by ``check_worker_load`` to return the number of
active workers that we parametrize.
"""
monkeypatch.setattr(common, 'get_num_workers', lambda: active_workers)
common.check_worker_load(active_slots)
assert expected in capsys.readouterr().out

class TestProcessWithDocstring(Process):
"""Some docstring."""

@classmethod
def define(cls, spec):
super().define(spec)
spec.input('some_input')
def test_check_worker_load_fail(monkeypatch, capsys):
"""Test the ``check_worker_load`` function when ``get_num_workers`` will except with ``CircusCallError``."""

@calcfunction
def test_without_docstring():
pass
def get_num_workers():
from aiida.common.exceptions import CircusCallError
raise CircusCallError

@calcfunction
def test_with_docstring():
"""Some docstring."""
monkeypatch.setattr(common, 'get_num_workers', get_num_workers)

# We are just checking that the command does not except
with Capturing():
print_process_info(TestProcessWithoutDocstring)
print_process_info(TestProcessWithDocstring)
print_process_info(test_without_docstring)
print_process_info(test_with_docstring)
with pytest.raises(SystemExit):
common.check_worker_load(None)

assert 'Could not contact Circus to get the number of active workers.' in capsys.readouterr().err

0 comments on commit 1071ae6

Please sign in to comment.