From: M. Eric Irrgang Date: Wed, 18 Nov 2020 16:58:03 +0000 (+0000) Subject: Replace gmxapi `erroroutput` with `stderr` and `stdout` X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?a=commitdiff_plain;h=090ae1c9e0d9224e2adfa809bacaceb98125b9ed;p=alexxy%2Fgromacs.git Replace gmxapi `erroroutput` with `stderr` and `stdout` --- diff --git a/docs/gmxapi/userguide/usage.rst b/docs/gmxapi/userguide/usage.rst index cc9afd0cc5..7cd08a764b 100644 --- a/docs/gmxapi/userguide/usage.rst +++ b/docs/gmxapi/userguide/usage.rst @@ -145,7 +145,7 @@ For example, you might create a :command:`gmx solvate` operation as:: } To check the status or error output of a command line operation, refer to the -``returncode`` and ``erroroutput`` outputs. +``returncode`` and ``stderr`` outputs. To access the results from the output file arguments, use the command line flags as keys in the ``file`` dictionary output. diff --git a/python_packaging/sample_restraint/tests/conftest.py b/python_packaging/sample_restraint/tests/conftest.py index 37ef076782..e439b3366e 100644 --- a/python_packaging/sample_restraint/tests/conftest.py +++ b/python_packaging/sample_restraint/tests/conftest.py @@ -96,7 +96,7 @@ def spc_water_box(gmxcli, remove_tempdir): assert os.path.exists(topfile) if solvate.output.returncode.result() != 0: - logging.debug(solvate.output.erroroutput.result()) + logging.debug(solvate.output.stderr.result()) raise RuntimeError('solvate failed in spc_water_box testing fixture.') # Choose an exactly representable dt of 2^-9 ps (approximately 0.002) @@ -132,7 +132,7 @@ def spc_water_box(gmxcli, remove_tempdir): output_files={'-o': tprfile}) tprfilename = grompp.output.file['-o'].result() if grompp.output.returncode.result() != 0: - logging.debug(grompp.output.erroroutput.result()) + logging.debug(grompp.output.stderr.result()) raise RuntimeError('grompp failed in spc_water_box testing fixture.') # TODO: more inspection of grompp errors... diff --git a/python_packaging/src/gmxapi/commandline.py b/python_packaging/src/gmxapi/commandline.py index 0166dcebe1..0426985492 100644 --- a/python_packaging/src/gmxapi/commandline.py +++ b/python_packaging/src/gmxapi/commandline.py @@ -60,7 +60,8 @@ logger.info('Importing {}'.format(__name__)) # * provides `output` publishing proxy to the inner function and # * produce a result with attributes for # * file: mapping of output flags to output filenames -# * erroroutput: text results in case of error +# * stdout: process STDOUT +# * stderr: porcess STDERR # * returncode: integer return code of wrapped command # # Note that the existence of the 'file' output map is expressed here, but @@ -70,7 +71,9 @@ logger.info('Importing {}'.format(__name__)) # # TODO: Operation returns the output object when called with the shorter signature. # -@gmx.function_wrapper(output={'erroroutput': str, 'returncode': int}) +@gmx.function_wrapper(output={'stdout': str, + 'stderr': str, + 'returncode': int}) def cli(command: NDArray, shell: bool, output: OutputCollectionDescription, stdin: str = ''): """Execute a command line program in a subprocess. @@ -117,23 +120,21 @@ def cli(command: NDArray, shell: bool, output: OutputCollectionDescription, stdi >>> my_filename = "somefilename" >>> result = cli(('exe', '--origin', 1.0, 2.0, 3.0, '-f', my_filename), shell=False) >>> assert hasattr(result, 'file') - >>> assert hasattr(result, 'erroroutput') + >>> assert hasattr(result, 'stdout') + >>> assert hasattr(result, 'stderr') >>> assert hasattr(result, 'returncode') Returns: - A data structure with attributes for each of the results `file`, `erroroutput`, and `returncode` + A data structure with attributes for each of the results `file`, `stdout`, `stderr`, and `returncode` Result object attributes: * `file`: the mapping of CLI flags to filename strings resulting from the `output` kwarg - * `erroroutput`: A string of error output (if any) if the process failed. + * `stdout`: A string mapping from process STDOUT. + * `stderr`: A string mapping from process STDERR; it will be the + error output (if any) if the process failed. * `returncode`: return code of the subprocess. """ - # Note: we could make provisions for stdio filehandles in a future version. E.g. - # * STDOUT is available if a consuming operation is bound to `output.stdout`. - # * STDERR is available if a consuming operation is bound to `output.stderr`. - # * Otherwise, STDOUT and/or STDERR is(are) closed when command is called. - # In the operation implementation, we expect the `shell` parameter to be intercepted by the # wrapper and set to False. if shell: @@ -154,7 +155,8 @@ def cli(command: NDArray, shell: bool, output: OutputCollectionDescription, stdi # TODO: (FR9) Can OS input/output filehandles be a responsibility of # the code providing 'resources'? - erroroutput = '' + stdout = '' + stderr = '' logger.debug('executing subprocess') try: completed_process = subprocess.run(command, @@ -162,20 +164,38 @@ def cli(command: NDArray, shell: bool, output: OutputCollectionDescription, stdi input=stdin, check=True, stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, + stderr=subprocess.PIPE, encoding='utf-8', universal_newlines=True ) returncode = completed_process.returncode # TODO: Resource management code should manage a safe data object for `output`. - for line in completed_process.stdout.split('\n'): - logger.debug(line) + logger.debug('STDOUT:') + if completed_process.stderr is not None: + for line in completed_process.stdout.split('\n'): + logger.debug(line) + else: + logger.debug('STDOUT is empty') + logger.debug('STDERR:') + if completed_process.stderr is not None: + for line in completed_process.stderr.split('\n'): + logger.debug(line) + else: + logger.debug('STDERR is empty') + + stdout = completed_process.stdout + stderr = completed_process.stderr + except subprocess.CalledProcessError as e: - logger.info("commandline operation had non-zero return status when calling {}".format(e.cmd)) - erroroutput = e.output + logger.info("commandline operation had non-zero return status" + "when calling {}".format(e.cmd)) + stdout = e.stdout + stderr = e.stderr returncode = e.returncode + # Publish outputs. - output.erroroutput = erroroutput + output.stdout = stdout + output.stderr = stderr output.returncode = returncode @@ -253,7 +273,9 @@ def commandline_operation(executable=None, The output node of the resulting operation handle contains * ``file``: the mapping of CLI flags to filename strings resulting from the ``output_files`` kwarg - * ``erroroutput``: A string of error output (if any) if the process failed. + * ``stdout``: A string mapping from process STDOUT. + * ``stderr``: A string mapping from process STDERR; it will be the + error output (if any) if the process failed. * ``returncode``: return code of the subprocess. """ @@ -284,15 +306,23 @@ def commandline_operation(executable=None, # # TODO: (FR4+) Characterize the `file` dictionary key type: # explicitly sequences rather than maybe-string/maybe-sequence-of-strings - @gmx.function_wrapper(output={'erroroutput': str, 'returncode': int, 'file': dict}) - def merged_ops(erroroutput: str = None, returncode: int = None, file: dict = None, + @gmx.function_wrapper(output={'stdout': str, + 'stderr': str, + 'returncode': int, + 'file': dict}) + def merged_ops(stdout: str = None, + stderr: str = None, + returncode: int = None, + file: dict = None, output: OutputCollectionDescription = None): - assert erroroutput is not None + assert stdout is not None + assert stderr is not None assert returncode is not None assert file is not None assert output is not None output.returncode = returncode - output.erroroutput = erroroutput + output.stdout = stdout + output.stderr = stderr if returncode == 0: output.file = file else: @@ -328,7 +358,8 @@ def commandline_operation(executable=None, # TODO: ``label`` kwarg # TODO: input fingerprinting cli_result = cli(**cli_args) - merged_result = merged_ops(erroroutput=cli_result.output.erroroutput, + merged_result = merged_ops(stdout=cli_result.output.stdout, + stderr=cli_result.output.stderr, returncode=cli_result.output.returncode, file=output_files, **kwargs) diff --git a/python_packaging/src/test/conftest.py b/python_packaging/src/test/conftest.py index 15ca729861..b80c21d439 100644 --- a/python_packaging/src/test/conftest.py +++ b/python_packaging/src/test/conftest.py @@ -94,7 +94,7 @@ def spc_water_box(gmxcli, remove_tempdir): assert os.path.exists(topfile) if solvate.output.returncode.result() != 0: - logging.debug(solvate.output.erroroutput.result()) + logging.debug(solvate.output.stderr.result()) raise RuntimeError('solvate failed in spc_water_box testing fixture.') # Choose an exactly representable dt of 2^-9 ps (approximately 0.002) @@ -130,8 +130,7 @@ def spc_water_box(gmxcli, remove_tempdir): output_files={'-o': tprfile}) tprfilename = grompp.output.file['-o'].result() if grompp.output.returncode.result() != 0: - # TODO: update for #3549 - logging.debug(grompp.output.erroroutput.result()) + logging.debug(grompp.output.stderr.result()) raise RuntimeError('grompp failed in spc_water_box testing fixture.') # TODO: more inspection of grompp errors... diff --git a/python_packaging/src/test/test_commandline.py b/python_packaging/src/test/test_commandline.py index 94956b7704..b5dd1838fe 100644 --- a/python_packaging/src/test/test_commandline.py +++ b/python_packaging/src/test/test_commandline.py @@ -60,15 +60,12 @@ class SimpleCliTestCase(unittest.TestCase): command = shutil.which('true') operation = commandline.cli(command=[command], shell=False) - # Note: 'stdout' and 'stderr' not mapped. # Note: getitem not implemented. - # assert not 'stdout' in operation.output - # assert not 'stderr' in operation.output - assert not hasattr(operation.output, 'stdout') - assert not hasattr(operation.output, 'stderr') - - # Check for the attributes that we _do_ expect. - assert hasattr(operation.output, 'erroroutput') + # assert 'stdout' in operation.output + # assert 'stderr' in operation.output + assert hasattr(operation.output, 'stdout') + assert hasattr(operation.output, 'stderr') + assert not hasattr(operation.output, 'erroroutput') assert hasattr(operation.output, 'returncode') operation.run() @@ -118,14 +115,13 @@ class CommandLineOperationSimpleTestCase(unittest.TestCase): def test_true(self): operation = commandline.commandline_operation(executable='true') - # Note: 'stdout' and 'stderr' not mapped. # Note: getitem not implemented. - # assert not 'stdout' in operation.output - # assert not 'stderr' in operation.output - assert not hasattr(operation.output, 'stdout') - assert not hasattr(operation.output, 'stderr') + # assert 'stdout' in operation.output + # assert 'stderr' in operation.output + assert not hasattr(operation.output, 'erroroutput') assert hasattr(operation.output, 'file') - assert hasattr(operation.output, 'erroroutput') + assert hasattr(operation.output, 'stdout') + assert hasattr(operation.output, 'stderr') assert hasattr(operation.output, 'returncode') assert operation.output.returncode.result() == 0