Skip to content

Conversation

@omlins
Copy link
Contributor

@omlins omlins commented Dec 10, 2018

Closes #467

@omlins omlins self-assigned this Dec 10, 2018
@omlins omlins requested review from teojgo and vkarak December 10, 2018 15:26
Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

I think we should add another layer between the _exec_load_command() and the actual implementation of the backends. The backends should then throw a SpawnedProcessError, which will contain all the information of the failing command, i.e., command arguments, stdout, stderr, exit code. We should do sth like the following:

def exec_module_command(self, *args, msg=None):
    try:
        self._exec_module_command(self, *args)
    except SpawnedProcessError as e:
        raise EnvironError(msg) from e

The advantage of this is that when we print the top-level exception, the information of the exceptions that caused the problem are also printed. And in a uniform way (you don't have to deal with where to put the :).

@vkarak
Copy link
Contributor

vkarak commented Dec 12, 2018

Following our discussion offline, the correct interface should be sth like the following:

    def _run_module_command(self, *args, msg=None):
        command = [self._command, *args]
        try:
            completed = os_ext.run_command(' '.join(command), check=True)
        except SpawnedProcessError as e:
            raise EnvironError(msg) from e

        # Check for ERROR inside the stderr as of now in _exec_module_command()
        if error:
            raise EnvironError(msg) from SpawnedProcessError(completed.command, completed.stdout, ...)

        return completed


    def _exec_module_command(self, *args, msg=None):
        completed = self._run_module_command(*args, msg)
        exec(completed.stdout)

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

My comments are mostly related to coding style issues.

@vkarak
Copy link
Contributor

vkarak commented Dec 13, 2018

@jenkins-cscs retry dom kesch

@omlins
Copy link
Contributor Author

omlins commented Dec 14, 2018

@jenkins-cscs retry dom kesch daint

@vkarak vkarak merged commit 2c17a0d into reframe-hpc:master Dec 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants