Various minor improvements

Also adds a test for the `--monitor` option.

aiida/cmdline/commands/cmd_calcjob.py

@@ -123,56 +123,45 @@ def calcjob_inputcat(calcjob, path):
 
 @verdi_calcjob.command('remotecat')
 @arguments.CALCULATION('calcjob', type=CalculationParamType(sub_classes=('aiida.node:process.calculation.calcjob',)))
-@click.argument('path', type=click.STRING, required=False)
+@click.argument('path', type=str, required=False)
 @click.option('--monitor', is_flag=True, default=False, help='Monitor the file using `tail -f` instead.')
 @decorators.with_dbenv()
 def calcjob_remotecat(calcjob, path, monitor):
-    """
-    Show the contents of one of the calcjob files in the remote working directory
-
-    You can specify the relative PATH in the working folder of the CalcJob.
+    """Show the contents of a file in the remote working directory.
 
-    If PATH is not specified, the default output file path will be used which is defined by the calcjob plugin class.
+    The file to show can be specified using the PATH argument. If PATH is not specified, the default output file path
+    as defined by the `CalcJob` plugin class will be used instead.
     """
     from shutil import copyfileobj
     import sys
     import tempfile
 
     from aiida.common.exceptions import NotExistent
 
-    if monitor:
-        _, path = get_remote_and_path(calcjob, path)
+    remote_folder, path = get_remote_and_path(calcjob, path)
 
+    if monitor:
         try:
             transport = calcjob.get_transport()
         except NotExistent as exception:
-            echo.echo_critical(repr(exception))
+            echo.echo_critical(str(exception))
 
         remote_workdir = calcjob.get_remote_workdir()
 
         if not remote_workdir:
-            echo.echo_critical('no remote work directory for this calcjob, maybe the daemon did not submit it yet')
+            echo.echo_critical('no remote work directory for this calcjob, maybe it has not yet started running?')
         cmds = f"-c 'tail -f {path}'"
         command = transport.gotocomputer_command(remote_workdir, cmds)
         os.system(command)
         return
 
-    remote_folder, path = get_remote_and_path(calcjob, path)
-
-    try:
-        with tempfile.NamedTemporaryFile(delete=False) as tmpf:
-            tmpf.close()
-            remote_folder.getfile(path, tmpf.name)
-            with open(tmpf.name, 'rb') as fhandle:
-                copyfileobj(fhandle, sys.stdout.buffer)
-    except IOError as err:
-        echo.echo_critical(f'{err.errno}: {str(err)}')
-
-    try:
-        os.remove(tmpf.name)
-    except OSError:
-        # If you cannot delete, ignore (maybe I didn't manage to create it in the first place
-        pass
+    with tempfile.NamedTemporaryFile() as tmp_path:
+        try:
+            remote_folder.getfile(path, tmp_path.name)
+            with open(tmp_path.name, 'rb') as handle:
+                copyfileobj(handle, sys.stdout.buffer)
+        except IOError as exception:
+            echo.echo_critical(str(exception))
 
 
 @verdi_calcjob.command('outputcat')
@@ -336,38 +325,53 @@ def calcjob_cleanworkdir(calcjobs, past_days, older_than, computers, force, exit
 
 
 def get_remote_and_path(calcjob, path=None):
-    """
-    Get the RemoteFolder and process the path argument
+    """Return the remote folder output node and process the path argument.
 
     :param calcjob: The ``CalcJobNode`` whose remote_folder to be returned.
-    :param path: The relative path of file. The default from the spec will be used if it is not defined.
-
+    :param path: The relative path of file. If not defined, it is attempted to determine the default output file from
+        the node options or otherwise from the associated process class. If neither are defined, a ``ValueError`` is
+        raised.
     :returns: A tuple of the ``RemoteData`` and the path of the output file to be used.
+    :raises ValueError: If path is not defined and no default output file is defined on the node nor its associated
+        process class.
     """
-
     remote_folder_linkname = 'remote_folder'  # The `remote_folder` is the standard output of a calculation.
 
     try:
         remote_folder = getattr(calcjob.outputs, remote_folder_linkname)
     except AttributeError:
-        echo.echo_critical(f'No "{remote_folder_linkname}" found. Have the calcjob files been submitted?')
+        echo.echo_critical(
+            f'`CalcJobNode<{calcjob.pk}>` has no `{remote_folder_linkname}` output. '
+            'It probably has not started running yet.'
+        )
 
-    # Get path from the given CalcJobNode if not defined by user
-    if path is None:
-        path = calcjob.get_option('output_filename')
+    if path is not None:
+        return remote_folder, path
 
-    # Get path from current process class spec of CalcJobNode if still not defined
-    if path is None:
-        fname = calcjob.process_class.spec_options.get('output_filename')
-        if fname and fname.has_default():
-            path = fname.default
+    # Try to get the default output filename from the node
+    path = calcjob.get_option('output_filename')
 
-    if path is None:
-        # Still no path available?
-        echo.echo_critical(
-            '"{}" and its process class "{}" do not define a default output file '
-            '(option "output_filename" not found).\n'
-            'Please specify a path explicitly.'.format(calcjob.__class__.__name__, calcjob.process_class.__name__)
-        )
+    if path is not None:
+        return remote_folder, path
 
-    return remote_folder, path
+    try:
+        process_class = calcjob.process_class
+    except ValueError as exception:
+        raise ValueError(
+            f'The process class of `CalcJobNode<{calcjob.pk}>` cannot be loaded and so the default output filename '
+            'cannot be determined.\nPlease specify a path explicitly.'
+        ) from exception
+
+    # Try to get the default output filename from the node's associated process class spec
+    port = process_class.spec_options.get('output_filename')
+    if port and port.has_default():
+        path = port.default
+
+    if path is not None:
+        return remote_folder, path
+
+    raise ValueError(
+        f'`CalcJobNode<{calcjob.pk}>` does not define a default output file (option "output_filename" not found) '
+        f'nor does its associated process class `{calcjob.process_class.__class__.__name__}`\n'
+        'Please specify a path explicitly.'
+    )

docs/source/reference/command_line.rst

@@ -53,7 +53,7 @@ Below is a list with all available subcommands.
       inputls       Show the list of the generated calcjob input files.
       outputcat     Show the contents of one of the calcjob retrieved outputs.
       outputls      Show the list of the retrieved calcjob output files.
-      remotecat     Show the contents of one of the calcjob files in the remote working...
+      remotecat     Show the contents of a file in the remote working directory.
       res           Print data from the result output Dict node of a calcjob.
 
 

tests/cmdline/commands/test_calcjob.py

@@ -10,7 +10,6 @@
 # pylint: disable=protected-access,too-many-locals,invalid-name,too-many-public-methods
 """Tests for `verdi calcjob`."""
 import io
-import pathlib
 
 from click.testing import CliRunner
 import pytest
@@ -34,9 +33,9 @@ class TestVerdiCalculation:
     """Tests for `verdi calcjob`."""
 
     @pytest.fixture(autouse=True)
-    def init_profile(self, aiida_profile_clean, aiida_localhost):  # pylint: disable=unused-argument
+    def init_profile(self, aiida_profile_clean, aiida_localhost, tmp_path):  # pylint: disable=unused-argument
         """Initialize the profile."""
-        # pylint: disable=attribute-defined-outside-init
+        # pylint: disable=attribute-defined-outside-init,too-many-statements
 
         self.computer = aiida_localhost
         self.code = orm.InstalledCode(computer=self.computer, filepath_executable='/bin/true').store()
@@ -48,17 +47,20 @@ def init_profile(self, aiida_profile_clean, aiida_localhost):  # pylint: disable
         process_type = get_entry_point_string_from_class(process_class.__module__, process_class.__name__)
 
         # Create 5 CalcJobNodes (one for each CalculationState)
-        for calculation_state in CalcJobState:
+        for index, calculation_state in enumerate(CalcJobState):
+
+            dirpath = (tmp_path / str(index))
+            dirpath.mkdir()
 
             calc = orm.CalcJobNode(computer=self.computer, process_type=process_type)
             calc.set_option('resources', {'num_machines': 1, 'num_mpiprocs_per_machine': 1})
             calc.set_option('output_filename', 'fileA.txt')
-            calc.set_remote_workdir('/tmp/aiida/work')
-            remote = RemoteData(remote_path='/tmp/aiida/work')
+            calc.set_remote_workdir(str(dirpath))
+            remote = RemoteData(remote_path=str(dirpath))
             remote.computer = calc.computer
             remote.base.links.add_incoming(calc, LinkType.CREATE, link_label='remote_folder')
-            (pathlib.Path('/tmp/aiida/work') / 'fileA.txt').write_text('test stringA')
-            (pathlib.Path('/tmp/aiida/work') / 'fileB.txt').write_text('test stringB')
+            (dirpath / 'fileA.txt').write_text('test stringA')
+            (dirpath / 'fileB.txt').write_text('test stringB')
             calc.store()
             remote.store()
 
@@ -85,14 +87,16 @@ def init_profile(self, aiida_profile_clean, aiida_localhost):  # pylint: disable
                 self.group.add_nodes([calc])
 
         # Create a single failed CalcJobNode
+        dirpath = (tmp_path / 'failed')
+        dirpath.mkdir()
         self.EXIT_STATUS = 100
         calc = orm.CalcJobNode(computer=self.computer)
         calc.set_option('resources', {'num_machines': 1, 'num_mpiprocs_per_machine': 1})
         calc.store()
         calc.set_exit_status(self.EXIT_STATUS)
         calc.set_process_state(ProcessState.FINISHED)
-        calc.set_remote_workdir('/tmp/aiida/work')
-        remote = RemoteData(remote_path='/tmp/aiida/work')
+        calc.set_remote_workdir(str(tmp_path))
+        remote = RemoteData(remote_path=str(tmp_path))
         remote.computer = calc.computer
         remote.base.links.add_incoming(calc, LinkType.CREATE, link_label='remote_folder')
         remote.store()
@@ -330,9 +334,11 @@ def test_calcjob_inoutputcat_old(self):
         assert len(get_result_lines(result)) == 1
         assert get_result_lines(result)[0] == '5'
 
-    def test_calcjob_remotecat(self):
+    def test_calcjob_remotecat(self, monkeypatch):
         """Test the remotecat command that prints the remote file for a given calcjob"""
         # Specifying no filtering options and no explicit calcjobs should exit with non-zero status
+        import os
+
         options = []
         result = self.cli_runner.invoke(command.calcjob_remotecat, options)
         assert result.exception is not None, result.output
@@ -353,3 +359,10 @@ def test_calcjob_remotecat(self):
         options = [str(self.result_job.uuid), 'fileA.txt']
         result = self.cli_runner.invoke(command.calcjob_remotecat, options)
         assert result.stdout == 'test stringA'
+
+        # To test the ``--monitor`` option, mock the ``os.system`` command and simply print the command it receives.
+        with monkeypatch.context() as ctx:
+            ctx.setattr(os, 'system', lambda x: print(x))  # pylint: disable=unnecessary-lambda
+            options = ['--monitor', str(self.result_job.uuid), 'fileA.txt']
+            result = self.cli_runner.invoke(command.calcjob_remotecat, options)
+            assert "bash -l  -c 'tail -f fileA.txt'" in result.stdout