)]}'
{"src/c/pyeclib_c/pyeclib_c.c":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ea40fe1833bb519a4f4b35f22ed20cc6caf6fd63","unresolved":false,"context_lines":[{"line_number":91,"context_line":"{"},{"line_number":92,"context_line":"    PyObject *s \u003d PyImport_ImportModule(module);"},{"line_number":93,"context_line":"    if (s \u003d\u003d NULL) {"},{"line_number":94,"context_line":"        return NULL;"},{"line_number":95,"context_line":"    }"},{"line_number":96,"context_line":"    return (PyObject *) PyObject_GetAttrString(s, cls);"},{"line_number":97,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"fa6399be_04ae052e","line":94,"updated":"2016-10-11 23:23:59.000000000","message":"So the idea is that \n\nimport_class(\"pyeclib.ec_iface\", \"ECBooBerries\")\n\n... would fail and it\u0027s better to return a null pointer to indicate the error","commit_id":"677db6592e78be78c5ec53a3fa1500386821e0f8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ea40fe1833bb519a4f4b35f22ed20cc6caf6fd63","unresolved":false,"context_lines":[{"line_number":93,"context_line":"    if (s \u003d\u003d NULL) {"},{"line_number":94,"context_line":"        return NULL;"},{"line_number":95,"context_line":"    }"},{"line_number":96,"context_line":"    return (PyObject *) PyObject_GetAttrString(s, cls);"},{"line_number":97,"context_line":"}"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"/**"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"fa6399be_c4408d59","line":96,"updated":"2016-10-11 23:23:59.000000000","message":"is this the bit that segfaults when s is NULL?","commit_id":"677db6592e78be78c5ec53a3fa1500386821e0f8"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a625c37e51f8346d70da7426010f6f0805882562","unresolved":false,"context_lines":[{"line_number":93,"context_line":"    if (s \u003d\u003d NULL) {"},{"line_number":94,"context_line":"        return NULL;"},{"line_number":95,"context_line":"    }"},{"line_number":96,"context_line":"    return (PyObject *) PyObject_GetAttrString(s, cls);"},{"line_number":97,"context_line":"}"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"/**"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"fa6399be_3f3236f2","line":96,"in_reply_to":"fa6399be_c4408d59","updated":"2016-10-31 21:18:25.000000000","message":"Yup, was getting something like\n\n #0  0x00000000004dedb1 in PyObject_GetAttrString ()\n #1  0x00007ffff4bb7dbb in import_class (cls\u003d0x7ffff4bb99be \"ECInvalidParameter\",\n     module\u003d0x7ffff4bb9930 \"pyeclib.ec_iface\") at src/c/pyeclib_c/pyeclib_c.c:93\n #2  pyeclib_c_seterr (ret\u003dret@entry\u003d-206,\n     prefix\u003dprefix@entry\u003d0x7ffff4bb9a4b \"pyeclib_c_encode ERROR: \")\n     at src/c/pyeclib_c/pyeclib_c.c:165\n #3  0x00007ffff4bb9669 in pyeclib_c_encode (self\u003d\u003coptimized out\u003e, args\u003d\u003coptimized out\u003e)\n     at src/c/pyeclib_c/pyeclib_c.c:536\n\n(only the line numbers are all messed up, \u0027cause I was working on something else)\n\nStill not entirely sure why that happened, though. pyeclib.ec_iface should totally be importable and expose ECInvalidParameter. And `tox -e py35` works fine against master; this only came up because my tests in https://review.openstack.org/#/c/385185/1/test/test_pyeclib_api.py were originally using native strings.","commit_id":"677db6592e78be78c5ec53a3fa1500386821e0f8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ea40fe1833bb519a4f4b35f22ed20cc6caf6fd63","unresolved":false,"context_lines":[{"line_number":190,"context_line":"            strcat(err, \"Unknown error\");"},{"line_number":191,"context_line":"            break;"},{"line_number":192,"context_line":"    }"},{"line_number":193,"context_line":"    if (eo) {"},{"line_number":194,"context_line":"      strcat(err, \". Please inspect syslog for liberasurecode error report.\");"},{"line_number":195,"context_line":"      PyErr_SetString(eo, err);"},{"line_number":196,"context_line":"    }"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"fa6399be_447f1daa","line":193,"updated":"2016-10-11 23:23:59.000000000","message":"and then this is just to check if getting the \"error object\" failed?","commit_id":"677db6592e78be78c5ec53a3fa1500386821e0f8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ea40fe1833bb519a4f4b35f22ed20cc6caf6fd63","unresolved":false,"context_lines":[{"line_number":194,"context_line":"      strcat(err, \". Please inspect syslog for liberasurecode error report.\");"},{"line_number":195,"context_line":"      PyErr_SetString(eo, err);"},{"line_number":196,"context_line":"    }"},{"line_number":197,"context_line":"    // else, error occurred and we already called PyErr_SetString"},{"line_number":198,"context_line":"}"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"static int stderr_fd;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"fa6399be_644a6185","line":197,"updated":"2016-10-11 23:23:59.000000000","message":"already did what where now?","commit_id":"677db6592e78be78c5ec53a3fa1500386821e0f8"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a625c37e51f8346d70da7426010f6f0805882562","unresolved":false,"context_lines":[{"line_number":194,"context_line":"      strcat(err, \". Please inspect syslog for liberasurecode error report.\");"},{"line_number":195,"context_line":"      PyErr_SetString(eo, err);"},{"line_number":196,"context_line":"    }"},{"line_number":197,"context_line":"    // else, error occurred and we already called PyErr_SetString"},{"line_number":198,"context_line":"}"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"static int stderr_fd;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"fa6399be_1f4c926f","line":197,"in_reply_to":"fa6399be_644a6185","updated":"2016-10-31 21:18:25.000000000","message":"\"we\" isn\u0027t actually correct now; it was for a bit while I was poking around, but then I realized PyImport_ImportModule and PyObject_GetAttrString would take care of it for me.","commit_id":"677db6592e78be78c5ec53a3fa1500386821e0f8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4d3a24b3a4f05fb1b04c655f0a554f507204c0a7","unresolved":false,"context_lines":[{"line_number":90,"context_line":"static PyObject *import_class(const char *module, const char *cls)"},{"line_number":91,"context_line":"{"},{"line_number":92,"context_line":"    PyObject *s \u003d PyImport_ImportModule(module);"},{"line_number":93,"context_line":"    return (PyObject *) PyObject_GetAttrString(s, cls);"},{"line_number":94,"context_line":"}"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"/**"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"5a74a57a_6640ce36","side":"PARENT","line":93,"updated":"2016-11-29 22:35:58.000000000","message":"again, return NULL on failure\n\nhttps://docs.python.org/2.7/c-api/object.html?highlight\u003dpyobject_getattrstring#c.PyObject_GetAttrString\n\nSo we seem to be doing the right thing here.","commit_id":"a185b061a15da8c27acd413616a3845a54d5fecb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4d3a24b3a4f05fb1b04c655f0a554f507204c0a7","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    PyObject *s \u003d PyImport_ImportModule(module);"},{"line_number":93,"context_line":"    if (s \u003d\u003d NULL) {"},{"line_number":94,"context_line":"        return NULL;"},{"line_number":95,"context_line":"    }"},{"line_number":96,"context_line":"    return (PyObject *) PyObject_GetAttrString(s, cls);"},{"line_number":97,"context_line":"}"},{"line_number":98,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"5a74a57a_4624b2ae","line":95,"updated":"2016-11-29 22:35:58.000000000","message":"I\u0027m pretty sure this is correct\n\nhttps://docs.python.org/2.7/c-api/import.html#c.PyImport_ImportModule","commit_id":"736766d65f90ab1429831ac5123334db60b9934e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4d3a24b3a4f05fb1b04c655f0a554f507204c0a7","unresolved":false,"context_lines":[{"line_number":195,"context_line":"      PyErr_SetString(eo, err);"},{"line_number":196,"context_line":"    }"},{"line_number":197,"context_line":"    /* else, error occurred and PyImport_ImportModule or PyObject_GetAttrString"},{"line_number":198,"context_line":"     * already called PyErr_SetString */"},{"line_number":199,"context_line":"}"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"static int stderr_fd;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"5a74a57a_86ca4a6d","line":198,"updated":"2016-11-29 22:35:58.000000000","message":"I don\u0027t like this comment.\n\nI agree, it\u0027s not obvious to me that we should just let that the other error come out; but, I think it\u0027s just my newbie-ness.\n\nIt seems like this is the *recommend* pattern, based on my reading of the docs[1]\n\n    When a function must fail because some function it called failed, it\n    generally doesn’t set the error indicator; the function it called\n    already set it.\n\nThere\u0027s something in there about \"handling the error and clearing the exception\" but nothing about changing the error, so I don\u0027t know what else we would do...\n\nStill, IMHO it\u0027s totally inscrutable from the callers perspective:\n\n    ubuntu@saio:/vagrant/pyeclib$ nosetests test/test_pyeclib_api.py:TestPyECLibDriver.test_liberasurecode_insufficient_frags_error\n    E\n    \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n    ERROR: test_liberasurecode_insufficient_frags_error (test.test_pyeclib_api.TestPyECLibDriver)\n    ----------------------------------------------------------------------\n    Traceback (most recent call last):\n      File \"/vagrant/pyeclib/test/test_pyeclib_api.py\", line 605, in test_liberasurecode_insufficient_frags_error\n        [fragments[0]], [1, 2, 3, 4, 5, 6])\n      File \"/usr/lib/python2.7/unittest/case.py\", line 473, in assertRaises\n        callableObj(*args, **kwargs)\n      File \"/vagrant/pyeclib/pyeclib/ec_iface.py\", line 280, in reconstruct\n        available_fragment_payloads, missing_fragment_indexes)\n      File \"/vagrant/pyeclib/pyeclib/core.py\", line 109, in reconstruct\n        self.handle, _fragment_payloads, fragment_len, index)\n    AttributeError: \u0027module\u0027 object has no attribute \u0027ECInsufficientFraments\u0027\n\nOur only hope is that it\u0027s useful to the pyeclib developer once we realize that it\u0027s not the pyeclib_c.reconstruct python frame that raised the error but some mysterious CPython stack down below that who misspelled ECInsufficientFra*g*ments?\n\nBut what do you do?\n\n1. https://docs.python.org/2.7/c-api/exceptions.html?highlight\u003dpyerr_setstring#exception-handling","commit_id":"736766d65f90ab1429831ac5123334db60b9934e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2c2293fb31f1b93d6abad4b2abd3a7e00a70e429","unresolved":false,"context_lines":[{"line_number":195,"context_line":"      PyErr_SetString(eo, err);"},{"line_number":196,"context_line":"    }"},{"line_number":197,"context_line":"    /* else, error occurred and PyImport_ImportModule or PyObject_GetAttrString"},{"line_number":198,"context_line":"     * already called PyErr_SetString */"},{"line_number":199,"context_line":"}"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"static int stderr_fd;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"3f044301_02124e59","line":198,"in_reply_to":"3a71b18c_0208b690","updated":"2017-05-03 21:03:50.000000000","message":"Clay, it seems to me that \"changing the error\" is a combination of \"handle the error and clear the exception\" followed by \"raise a new error\", so I think I\u0027m getting behind Kota\u0027s idea to always call PyErr_Clear() at the start of this function.\n\nYou\u0027re right that it still isn\u0027t a terribly scrutable error, but that tends to happen when your error-handling code hits errors. And as soon as you start getting non-sensical errors out of your call to a C extension, I feel like you\u0027re going to need to drop into gdb...\n\nKota, FWIW, AttributeError *is* what\u0027s raised in such a circumstance; you can even reproduce it without any typos-in-C-code if you apply something like:\n\n diff --git a/test/test_pyeclib_api.py b/test/test_pyeclib_api.py\n index 9585035..7d3b86e 100644\n --- a/test/test_pyeclib_api.py\n +++ b/test/test_pyeclib_api.py\n @@ -31,6 +31,7 @@ import unittest\n  from distutils.version import StrictVersion\n  from itertools import combinations\n \n +import pyeclib.ec_iface\n  from pyeclib.ec_iface import ECBackendNotSupported\n  from pyeclib.ec_iface import ECDriver\n  from pyeclib.ec_iface import ECDriverError\n @@ -665,6 +666,7 @@ class TestPyECLibDriver(unittest.TestCase):\n              return None\n \n      def test_liberasurecode_insufficient_frags_error(self):\n +        del pyeclib.ec_iface.ECInsufficientFragments\n          file_size \u003d self.file_sizes[0]\n          tmp_file \u003d self.files[file_size]\n          tmp_file.seek(0)\n\n(Though really, at that point you\u0027re just begging for things to go wrong.)","commit_id":"736766d65f90ab1429831ac5123334db60b9934e"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"b8fa4522eee2258f83e8d32b36b709119e46bb84","unresolved":false,"context_lines":[{"line_number":195,"context_line":"      PyErr_SetString(eo, err);"},{"line_number":196,"context_line":"    }"},{"line_number":197,"context_line":"    /* else, error occurred and PyImport_ImportModule or PyObject_GetAttrString"},{"line_number":198,"context_line":"     * already called PyErr_SetString */"},{"line_number":199,"context_line":"}"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"static int stderr_fd;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"3a71b18c_0208b690","line":198,"in_reply_to":"5a74a57a_86ca4a6d","updated":"2016-12-02 07:50:13.000000000","message":"From my limited understanding, importing non-existing exception class or calling pyeclib_c_set_err more than once would be side effect for another bug, right?\n\nSo, perhaps, in the error statement, should we call\n\n Py_Err_Clear();  // unset the current indicator\n\nAnd then, set some standard exception for python in [1]?\n\nSystemError, RuntimeError or if no class exist, AttributeError may be good as Clay said?\n\nHow do you think of?\n\n1: https://docs.python.org/2.7/c-api/exceptions.html#standard-exceptions","commit_id":"736766d65f90ab1429831ac5123334db60b9934e"}]}