MMS BYO notebook does not detect OSError correctly

[tvoipio]

The multi-model serving BYO notebook apparently wants to retry if subprocess.CalledProcessError or OSError are raised, but only subprocess.CalledProcessError is detected correctly.

https://github.com/aws/amazon-sagemaker-examples/blob/cb28f5f09c536c8ce5db96c8cf06cd4913d762e9/advanced_functionality/multi_model_bring_your_own/container/dockerd-entrypoint.py#L12

The expression CalledProcessError or OSError evaluates simply to CalledProcessError. In order to test whether an object is an instance of one of listed classes, the class list should be passed as a tuple.

The issue can be fixed by replacing the referred line with return isinstance(exception, (CalledProcessError, OSError))

[tvoipio]

Complete working example:

def _retry_if_error(exception):
    return isinstance(exception, CalledProcessError or OSError)

def _retry_if_error_fixed(exception):
    return isinstance(exception, (CalledProcessError, OSError))

def raise_error(err_cls: type):
    raise err_cls("arg1", "arg2")

if __name__ == "__main__":

    try:
        raise_error(CalledProcessError)
    except Exception as e:
        print(
            "CalledProcessError, should be True, is: "
            f"{_retry_if_error(e)} (original fn)"
        )

    try:
        raise_error(CalledProcessError)
    except Exception as e:
        print(
            "CalledProcessError, should be True, is: "
            f"{_retry_if_error_fixed(e)} (fixed fn)"
        )

    try:
        raise_error(OSError)
    except Exception as e:
        print(
            "OSError, should be True, is: "
            f"{_retry_if_error(e)} (original fn)"
        )

    try:
        raise_error(OSError)
    except Exception as e:
        print(
            "OSError, should be True, is: "
            f"{_retry_if_error_fixed(e)} (fixed fn)"
        )

    try:
        raise_error(Exception)
    except Exception as e:
        print(
            "Exception, should be False, is: "
            f"{_retry_if_error(e)} (original fn)"
        )

    try:
        raise_error(Exception)
    except Exception as e:
        print(
            "Exception, should be False, is: "
            f"{_retry_if_error_fixed(e)} (fixed fn)"
        )

The output (at least with CPython 3.8.10) is

CalledProcessError, should be True, is: True (original fn)
CalledProcessError, should be True, is: True (fixed fn)
OSError, should be True, is: False (original fn)
OSError, should be True, is: True (fixed fn)
Exception, should be False, is: False (original fn)
Exception, should be False, is: False (fixed fn)

See the 3rd output line: OSError is not detected correctly.

It might be that OSErrors do not even need to be treated as being retriable, but in this case, the code is also misleading if it is indeed sufficient to test for CalledProcessError.