)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f249d5e0fc93ae60733742aed34a2f10454302ba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"03171137_36fcfa28","updated":"2024-10-09 23:06:23.000000000","message":"I do like this approach, it means we get to use the destroy instance in \u0027close\u0027 and the destructor. So doubly the chance to clean up properly.\n\nI think there is a case however where we may nore free up the handler properly.","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e63952ca022ca60c4fe4691d6de7dc19c70f4e92","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f06826a6_182b023d","updated":"2024-10-08 20:59:54.000000000","message":"recheck","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0a3dfc8e837f6e41bdef5d5e80c63f19935c37f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f9e51321_8bf841f6","updated":"2024-10-08 22:55:43.000000000","message":"recheck\n\nhttps://review.opendev.org/c/openstack/pyeclib/+/931846 fixed the py35 job.","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"96e245a4417e5b03676530d4f89eede22c0ad4a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"ed55c985_36a35fee","updated":"2024-10-09 23:46:47.000000000","message":"I\u0027ll wait on zuul, but yeah that looks much better!","commit_id":"ee237e8040c1eadc5c4626e1dcec5815bc635fa0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0efeb532524f47644430a453e0a50d558729c91f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"46936742_913dc13e","updated":"2024-10-10 19:01:01.000000000","message":"OK, so this is getting close -- do we want to squash it into the parent? Or keep them separate? After digging into it more, I\u0027m not sure we really want to be calling `pyeclib_c_seterr` in the destructor at all...","commit_id":"ee237e8040c1eadc5c4626e1dcec5815bc635fa0"}],"pyeclib/core.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f249d5e0fc93ae60733742aed34a2f10454302ba","unresolved":true,"context_lines":[{"line_number":66,"context_line":"    def close(self):"},{"line_number":67,"context_line":"        if self.handle:"},{"line_number":68,"context_line":"            pyeclib_c.destroy(self.handle)"},{"line_number":69,"context_line":"        self.handle \u003d None"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def encode(self, data_bytes):"},{"line_number":72,"context_line":"        return pyeclib_c.encode(self.handle, data_bytes)"}],"source_content_type":"text/x-python","patch_set":3,"id":"97a0d983_11c385f9","line":69,"updated":"2024-10-09 23:06:23.000000000","message":"So setting this to `None` should mean the rest of the object is cleaned up by the GC? Which calls the destructor method too?","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"95c70e2dc5cb6903660f76e52a14ad1bf2eae387","unresolved":true,"context_lines":[{"line_number":66,"context_line":"    def close(self):"},{"line_number":67,"context_line":"        if self.handle:"},{"line_number":68,"context_line":"            pyeclib_c.destroy(self.handle)"},{"line_number":69,"context_line":"        self.handle \u003d None"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def encode(self, data_bytes):"},{"line_number":72,"context_line":"        return pyeclib_c.encode(self.handle, data_bytes)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1b03e682_6eca0322","line":69,"in_reply_to":"97a0d983_11c385f9","updated":"2024-10-09 23:22:58.000000000","message":"Unless someone else holds a reference, too. 😕","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"}],"src/c/pyeclib_c/pyeclib_c.c":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f249d5e0fc93ae60733742aed34a2f10454302ba","unresolved":true,"context_lines":[{"line_number":335,"context_line":"    if (set_err) {"},{"line_number":336,"context_line":"      pyeclib_c_seterr(ret, \"pyeclib_c_destroy\");"},{"line_number":337,"context_line":"    }"},{"line_number":338,"context_line":"    return NULL;"},{"line_number":339,"context_line":"  }"},{"line_number":340,"context_line":"  return pyeclib_handle;"},{"line_number":341,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"5e96f943_b431e810","line":338,"updated":"2024-10-09 23:06:23.000000000","message":"I think we still want to return the pyeclib_handle here, because we\u0027ve only failed to destroy the eclib descriptor. We still want to pass the rest of the handle through  `check_and_free_buffer` so the rest of it can be freed (when following the destructor line)","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0efeb532524f47644430a453e0a50d558729c91f","unresolved":false,"context_lines":[{"line_number":335,"context_line":"    if (set_err) {"},{"line_number":336,"context_line":"      pyeclib_c_seterr(ret, \"pyeclib_c_destroy\");"},{"line_number":337,"context_line":"    }"},{"line_number":338,"context_line":"    return NULL;"},{"line_number":339,"context_line":"  }"},{"line_number":340,"context_line":"  return pyeclib_handle;"},{"line_number":341,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"3a500137_3a6d5c6e","line":338,"in_reply_to":"168a1adb_82e3417a","updated":"2024-10-10 19:01:01.000000000","message":"Done","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"95c70e2dc5cb6903660f76e52a14ad1bf2eae387","unresolved":true,"context_lines":[{"line_number":335,"context_line":"    if (set_err) {"},{"line_number":336,"context_line":"      pyeclib_c_seterr(ret, \"pyeclib_c_destroy\");"},{"line_number":337,"context_line":"    }"},{"line_number":338,"context_line":"    return NULL;"},{"line_number":339,"context_line":"  }"},{"line_number":340,"context_line":"  return pyeclib_handle;"},{"line_number":341,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"168a1adb_82e3417a","line":338,"in_reply_to":"5e96f943_b431e810","updated":"2024-10-09 23:22:58.000000000","message":"Hmm... if we do that, I think we need to *not* call `pyeclib_c_seterr`... which feels off. Or -- maybe I could have the return switched off `set_err`, too...","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c6674eb2d2e33d9d3184ddc83fc94d7f093b282b","unresolved":true,"context_lines":[{"line_number":346,"context_line":"static PyObject *"},{"line_number":347,"context_line":"pyeclib_c_destroy(PyObject *self, PyObject *obj)"},{"line_number":348,"context_line":"{"},{"line_number":349,"context_line":"  if (_destroy(obj, 1) \u003c\u003d 0) {"},{"line_number":350,"context_line":"    return NULL;"},{"line_number":351,"context_line":"  }"},{"line_number":352,"context_line":"  Py_RETURN_NONE;"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"d119f939_6397322f","line":349,"updated":"2024-10-09 23:50:23.000000000","message":"Could change to just `if (!_destroy(obj, 1)) {` or `if (_destroy(obj, 1) \u003d\u003d NULL) {` -- I\u0027d been debating about trying to return `-pyeclib_handle` if calling `liberasurecode_instance_destroy` failed, but compiler didn\u0027t care for that...","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f249d5e0fc93ae60733742aed34a2f10454302ba","unresolved":true,"context_lines":[{"line_number":348,"context_line":"{"},{"line_number":349,"context_line":"  if (_destroy(obj, 1) \u003c\u003d 0) {"},{"line_number":350,"context_line":"    return NULL;"},{"line_number":351,"context_line":"  }"},{"line_number":352,"context_line":"  Py_RETURN_NONE;"},{"line_number":353,"context_line":"}"},{"line_number":354,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"51d0bad8_142b0033","line":351,"updated":"2024-10-09 23:06:23.000000000","message":"If we still have a handle present, should we try and `check_and_free_buffer` and free it. Or just let the GC try that later?","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0efeb532524f47644430a453e0a50d558729c91f","unresolved":true,"context_lines":[{"line_number":348,"context_line":"{"},{"line_number":349,"context_line":"  if (_destroy(obj, 1) \u003c\u003d 0) {"},{"line_number":350,"context_line":"    return NULL;"},{"line_number":351,"context_line":"  }"},{"line_number":352,"context_line":"  Py_RETURN_NONE;"},{"line_number":353,"context_line":"}"},{"line_number":354,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"cfd5d282_a5430e39","line":351,"in_reply_to":"4ee00e44_c6a69fa7","updated":"2024-10-10 19:01:01.000000000","message":"Part of me really wants to try to simplify this all such that we don\u0027t need the capsule -- we ought to be able to just have `init` return the descriptor, without all the args attached... That\u0027ll surely be later, though.","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"95c70e2dc5cb6903660f76e52a14ad1bf2eae387","unresolved":true,"context_lines":[{"line_number":348,"context_line":"{"},{"line_number":349,"context_line":"  if (_destroy(obj, 1) \u003c\u003d 0) {"},{"line_number":350,"context_line":"    return NULL;"},{"line_number":351,"context_line":"  }"},{"line_number":352,"context_line":"  Py_RETURN_NONE;"},{"line_number":353,"context_line":"}"},{"line_number":354,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"e18d6bb2_0cd8031e","line":351,"in_reply_to":"51d0bad8_142b0033","updated":"2024-10-09 23:22:58.000000000","message":"No, we want to hold off until the capsule is truly GC\u0027ed -- otherwise (IIRC) we get segfaults if someone tries to use after `close`. This way, they just get `ECBackendInstanceNotAvailable`.","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"96e245a4417e5b03676530d4f89eede22c0ad4a3","unresolved":true,"context_lines":[{"line_number":348,"context_line":"{"},{"line_number":349,"context_line":"  if (_destroy(obj, 1) \u003c\u003d 0) {"},{"line_number":350,"context_line":"    return NULL;"},{"line_number":351,"context_line":"  }"},{"line_number":352,"context_line":"  Py_RETURN_NONE;"},{"line_number":353,"context_line":"}"},{"line_number":354,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"4ee00e44_c6a69fa7","line":351,"in_reply_to":"e18d6bb2_0cd8031e","updated":"2024-10-09 23:46:47.000000000","message":"yup, got it. Read up on Py capsules. So the destructor is call back for when the capsule is destroyed. So yeah only need to `check_and_free_buffer` there!","commit_id":"c6b75d236698851bd10f3f3c82f76f8354d262e2"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"96e245a4417e5b03676530d4f89eede22c0ad4a3","unresolved":true,"context_lines":[{"line_number":305,"context_line":""},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"static pyeclib_t *"},{"line_number":308,"context_line":"_destroy(PyObject *obj, int in_destructor)"},{"line_number":309,"context_line":"{"},{"line_number":310,"context_line":"  pyeclib_t *pyeclib_handle \u003d NULL;  /* pyeclib object to destroy */"},{"line_number":311,"context_line":"  int ret;"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"92fb2c03_6003ddbd","line":308,"updated":"2024-10-09 23:46:47.000000000","message":"Yeah cool, that\u0027s basically what the switch was doing. skipping the erroring or cleaning up.","commit_id":"ee237e8040c1eadc5c4626e1dcec5815bc635fa0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"96e245a4417e5b03676530d4f89eede22c0ad4a3","unresolved":true,"context_lines":[{"line_number":334,"context_line":"  if (ret !\u003d 0) {"},{"line_number":335,"context_line":"    if (in_destructor) {"},{"line_number":336,"context_line":"      /* destructor still wants to check_and_free */"},{"line_number":337,"context_line":"      return pyeclib_handle;"},{"line_number":338,"context_line":"    }"},{"line_number":339,"context_line":"    pyeclib_c_seterr(ret, \"pyeclib_c_destroy\");"},{"line_number":340,"context_line":"    return NULL;"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"e21e724f_faddb5ee","line":337,"updated":"2024-10-09 23:46:47.000000000","message":"Great! So now we clean up the rest of the struct properly.","commit_id":"ee237e8040c1eadc5c4626e1dcec5815bc635fa0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0efeb532524f47644430a453e0a50d558729c91f","unresolved":false,"context_lines":[{"line_number":334,"context_line":"  if (ret !\u003d 0) {"},{"line_number":335,"context_line":"    if (in_destructor) {"},{"line_number":336,"context_line":"      /* destructor still wants to check_and_free */"},{"line_number":337,"context_line":"      return pyeclib_handle;"},{"line_number":338,"context_line":"    }"},{"line_number":339,"context_line":"    pyeclib_c_seterr(ret, \"pyeclib_c_destroy\");"},{"line_number":340,"context_line":"    return NULL;"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"81e5aa28_fb8d6c95","line":337,"in_reply_to":"e21e724f_faddb5ee","updated":"2024-10-10 19:01:01.000000000","message":"Acknowledged","commit_id":"ee237e8040c1eadc5c4626e1dcec5815bc635fa0"}],"test/test_pyeclib_c.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c6674eb2d2e33d9d3184ddc83fc94d7f093b282b","unresolved":true,"context_lines":[{"line_number":479,"context_line":"        pyeclib_c.encode(handle, whole_file_bytes)"},{"line_number":480,"context_line":"        pyeclib_c.destroy(handle)"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"        # Can\u0027t destroy again"},{"line_number":483,"context_line":"        with self.assertRaises(ECBackendInstanceNotAvailable) as caught:"},{"line_number":484,"context_line":"            pyeclib_c.destroy(handle)"},{"line_number":485,"context_line":"        self.assertEqual("},{"line_number":486,"context_line":"            str(caught.exception),"},{"line_number":487,"context_line":"            \u0027pyeclib_c_destroy ERROR: Backend instance not found. Please \u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"7e3a865f_9fffca31","line":484,"range":{"start_line":482,"start_character":0,"end_line":484,"end_character":37},"updated":"2024-10-09 23:50:23.000000000","message":"Hmm... arguably, instance-not-available could be the one error case where we could swallow it...","commit_id":"ee237e8040c1eadc5c4626e1dcec5815bc635fa0"}]}