Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

verdi process show: order called by ctime and use process label #4407

Merged
merged 1 commit into from
Sep 29, 2020
Merged

verdi process show: order called by ctime and use process label #4407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions aiida/cmdline/utils/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,17 +147,21 @@ def get_node_info(node, include_summary=True):
nodes_input = node.get_incoming(link_type=(LinkType.INPUT_CALC, LinkType.INPUT_WORK))
nodes_output = node.get_outgoing(link_type=(LinkType.CREATE, LinkType.RETURN))

if nodes_caller:
result += '\n' + format_nested_links(nodes_caller.nested(), headers=['Called by', 'PK', 'Type'])

if nodes_input:
result += '\n' + format_nested_links(nodes_input.nested(), headers=['Inputs', 'PK', 'Type'])

if nodes_output:
result += '\n' + format_nested_links(nodes_output.nested(), headers=['Outputs', 'PK', 'Type'])

if nodes_caller:
result += '\n' + format_flat_links(
sorted(nodes_caller.all(), key=lambda x: x.node.ctime), headers=['Caller', 'PK', 'Type']
)
Comment on lines +156 to +159
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(oops, I think I forgot to add this to the review, I'll just leave it here)

This "if-path" seems not to be covered by the tests; I think the easiest way to include it would be by either adding a cli-invocation on one of the calcjob nodes + respective checks, or by adding a call link between the workchains and just adding a check after the already existing cli-invoke (I woult think the second one is the most "economical one", but I'm not sure if you had a reason for creating those two workchains and leaving one of those disconected).

Also, tangential question: a process will either have one caller or none at all, right? Why do we write this here like there could be many callers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I woult think the second one is the most "economical one", but I'm not sure if you had a reason for creating those two workchains and leaving one of those disconected

No particular reason to leave it unconnected just that I only needed one of those two to do the test. I will add the second one as the caller of the first one and add the test.

Also, tangential question: a process will either have one caller or none at all, right? Why do we write this here like there could be many callers?

Absolutely correct. There is no specific reason other than that it is just easier to write and understand compared to the nodes_called.


if nodes_called:
result += '\n' + format_flat_links(nodes_called.all(), headers=['Called', 'PK', 'Type'])
result += '\n' + format_flat_links(
sorted(nodes_called.all(), key=lambda x: x.node.ctime), headers=['Called', 'PK', 'Type']
)

log_messages = orm.Log.objects.get_logs_for(node)

Expand All @@ -181,7 +185,7 @@ def format_flat_links(links, headers):
table = []

for link_triple in links:
table.append([link_triple.link_label, link_triple.node.pk, link_triple.node.__class__.__name__])
table.append([link_triple.link_label, link_triple.node.pk, link_triple.node.get_attribute('process_label', '')])

result = '\n{}'.format(tabulate(table, headers=headers))

Expand Down
58 changes: 28 additions & 30 deletions tests/cmdline/commands/test_process.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from aiida.common.links import LinkType
from aiida.common.log import LOG_LEVEL_REPORT
from aiida.manage.manager import get_manager
from aiida.orm import WorkflowNode, WorkFunctionNode, WorkChainNode
from aiida.orm import CalcJobNode, WorkflowNode, WorkFunctionNode, WorkChainNode

from tests.utils import processes as test_processes

Expand Down Expand Up @@ -274,8 +274,26 @@ def test_list(self):

def test_process_show(self):
"""Test verdi process show"""
# We must choose a Node we can store
node = WorkflowNode().store()
workchain_one = WorkChainNode()
workchain_two = WorkChainNode()
workchains = [workchain_one, workchain_two]

workchain_two.set_attribute('process_label', 'workchain_one_caller')
workchain_two.store()
workchain_one.add_incoming(workchain_two, link_type=LinkType.CALL_WORK, link_label='called')
workchain_one.store()

calcjob_one = CalcJobNode()
calcjob_two = CalcJobNode()

calcjob_one.set_attribute('process_label', 'process_label_one')
calcjob_two.set_attribute('process_label', 'process_label_two')

calcjob_one.add_incoming(workchain_one, link_type=LinkType.CALL_CALC, link_label='one')
calcjob_two.add_incoming(workchain_one, link_type=LinkType.CALL_CALC, link_label='two')

calcjob_one.store()
calcjob_two.store()

# Running without identifiers should not except and not print anything
options = []
Expand All @@ -284,13 +302,17 @@ def test_process_show(self):
self.assertEqual(len(get_result_lines(result)), 0)

# Giving a single identifier should print a non empty string message
options = [str(node.pk)]
options = [str(workchain_one.pk)]
result = self.cli_runner.invoke(cmd_process.process_show, options)
lines = get_result_lines(result)
self.assertClickResultNoException(result)
self.assertTrue(len(get_result_lines(result)) > 0)
self.assertTrue(len(lines) > 0)
self.assertIn('workchain_one_caller', result.output)
self.assertIn('process_label_one', lines[-2])
self.assertIn('process_label_two', lines[-1])

# Giving multiple identifiers should print a non empty string message
options = [str(calc.pk) for calc in [node]]
options = [str(node.pk) for node in workchains]
result = self.cli_runner.invoke(cmd_process.process_show, options)
self.assertIsNone(result.exception, result.output)
self.assertTrue(len(get_result_lines(result)) > 0)
Expand Down Expand Up @@ -360,30 +382,6 @@ def test_report(self):
self.assertEqual(len(get_result_lines(result)), 1)
self.assertEqual(get_result_lines(result)[0], 'No log messages recorded for this entry')

def test_work_show(self):
"""Test verdi process show"""
workchain_one = WorkChainNode().store()
workchain_two = WorkChainNode().store()
workchains = [workchain_one, workchain_two]

# Running without identifiers should not except and not print anything
options = []
result = self.cli_runner.invoke(cmd_process.process_show, options)
self.assertIsNone(result.exception, result.output)
self.assertEqual(len(get_result_lines(result)), 0)

# Giving a single identifier should print a non empty string message
options = [str(workchain_one.pk)]
result = self.cli_runner.invoke(cmd_process.process_show, options)
self.assertClickResultNoException(result)
self.assertTrue(len(get_result_lines(result)) > 0)

# Giving multiple identifiers should print a non empty string message
options = [str(workchain.pk) for workchain in workchains]
result = self.cli_runner.invoke(cmd_process.process_show, options)
self.assertIsNone(result.exception, result.output)
self.assertTrue(len(get_result_lines(result)) > 0)


class TestVerdiProcessListWarning(AiidaTestCase):
"""Tests for the `verdi process list` active slots warning."""
Expand Down