Skip to content

Commit

Permalink
feat: inherit CommandError (also) from subprocess.CalledProcessError
Browse files Browse the repository at this point in the history
In the DataLad world `CommandError` was traditionally a `RuntimeError`.
Whether or not that makes sense in the general case, one can have
different opinions about.

The rest of the world is using `subprocess.CalledProcessError`. This
exception type is very similar to `CommandError` in the properties
that it can capture. It is only missing the CWD info, and the
context message that comes with `RuntimeError` (see class docs).

This change connects the two worlds, by inheriting from both
`RuntimeError` and `CalledProcessError`, and using the properties
of both, where possible.

The behavior of `CommandError` remains unchanged, but it will now also
work with code that expects a `subprocess` exception.
  • Loading branch information
mih committed Oct 1, 2024
1 parent 3099d87 commit f167ef7
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 5 deletions.
14 changes: 9 additions & 5 deletions datasalad/runners/exception.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
import os

import signal
import subprocess
import sys


class CommandError(RuntimeError):
class CommandError(subprocess.CalledProcessError, RuntimeError):
"""Raised when a subprocess execution fails (non-zero exit)
Key class attributes are aligned with the ``CalledProcessError`` exception
Expand Down Expand Up @@ -49,11 +50,14 @@ def __init__(
cwd: str | os.PathLike | None = None,
) -> None:
RuntimeError.__init__(self, msg)
self.cmd = cmd
subprocess.CalledProcessError.__init__(
self,
returncode=returncode,
cmd=cmd,
output=stdout,
stderr=stderr,
)
self.msg = msg
self.returncode = returncode
self.stdout = stdout
self.stderr = stderr
self.cwd = cwd

def __str__(self) -> str:
Expand Down
5 changes: 5 additions & 0 deletions datasalad/runners/tests/test_exception.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import subprocess
import sys

import pytest
Expand All @@ -12,6 +13,10 @@ def test_CommandError_minial():
# we need a command
CommandError()

def test_CommandError_multiple_inheritence():
ce = CommandError('dummy')
assert isinstance(ce, RuntimeError)
assert isinstance(ce, subprocess.CalledProcessError)

def test_CommandError_str_repr() -> None:
unicode_out = 'ΔЙקم๗あösdohabcdefdf0j23d2däüߧ!אؠॵΔЙקم๗あöäüߧ!אؠॵℱ⏰︻𐂃𐃍'
Expand Down

0 comments on commit f167ef7

Please sign in to comment.