)]}'
{"cyborg/common/nova_client.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c70ba7c43d90a320e1a545f9793c03646d80f1b4","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        elif response.status_code \u003d\u003d 207:"},{"line_number":42,"context_line":"            # This means the notfication was lost because the instance could"},{"line_number":43,"context_line":"            # not be associated with a host by Nova. That could be because"},{"line_number":44,"context_line":"            # Nova has not yet got around to associating the instance with"},{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_dc0aa677","line":44,"range":{"start_line":44,"start_character":27,"end_line":44,"end_character":30},"updated":"2019-12-04 20:57:10.000000000","message":"strike one of these \u0027yet\u0027s","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"1ac5b92c9bf41de0356501732c3535cd288f3216","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        elif response.status_code \u003d\u003d 207:"},{"line_number":42,"context_line":"            # This means the notfication was lost because the instance could"},{"line_number":43,"context_line":"            # not be associated with a host by Nova. That could be because"},{"line_number":44,"context_line":"            # Nova has not yet got around to associating the instance with"},{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_8cfccb0c","line":44,"range":{"start_line":44,"start_character":27,"end_line":44,"end_character":30},"in_reply_to":"3fa7e38b_dc0aa677","updated":"2019-12-08 01:58:12.000000000","message":"Done","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c70ba7c43d90a320e1a545f9793c03646d80f1b4","unresolved":false,"context_lines":[{"line_number":43,"context_line":"            # not be associated with a host by Nova. That could be because"},{"line_number":44,"context_line":"            # Nova has not yet got around to associating the instance with"},{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"},{"line_number":48,"context_line":"            # TODO(Sundar): Need to check for event code too."},{"line_number":49,"context_line":"            LOG.info(\u0027Nova returned 207.\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_3c141a9f","line":46,"updated":"2019-12-04 20:57:10.000000000","message":"You can put links like this all on one line and ask pep8 to give it a pass by putting a \" # noqa\" comment at the end of the line.","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"1ac5b92c9bf41de0356501732c3535cd288f3216","unresolved":false,"context_lines":[{"line_number":43,"context_line":"            # not be associated with a host by Nova. That could be because"},{"line_number":44,"context_line":"            # Nova has not yet got around to associating the instance with"},{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"},{"line_number":48,"context_line":"            # TODO(Sundar): Need to check for event code too."},{"line_number":49,"context_line":"            LOG.info(\u0027Nova returned 207.\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_ac018706","line":46,"in_reply_to":"3fa7e38b_3c141a9f","updated":"2019-12-08 01:58:12.000000000","message":"Done","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c70ba7c43d90a320e1a545f9793c03646d80f1b4","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"},{"line_number":48,"context_line":"            # TODO(Sundar): Need to check for event code too."},{"line_number":49,"context_line":"            LOG.info(\u0027Nova returned 207.\u0027)"},{"line_number":50,"context_line":"            return True"},{"line_number":51,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_c9d0a0cb","line":48,"range":{"start_line":48,"start_character":46,"end_line":48,"end_character":56},"updated":"2019-12-04 20:57:10.000000000","message":"IIUC it would look something like this:\n\n code \u003d response.json()[\u0027events\u0027][i][\u0027code\u0027]\n if code \u003d\u003d 422:\n     LOG...(\"Nova won, no event necessary\")\n     return True\n else:\n     LOG...(\"Couldn\u0027t fire event! Got code %d.\", code)\n     return False\n\nNote the [i]. As currently written, you\u0027re always sending exactly one event, so it\u0027ll be [0]. But (and I think we\u0027ve talked about this before) the names/interfaces of your internal methods imply that that might not always be the case. So you either need for this method to be written with a loop and granular returns to account properly for multiple events, or you should refactor/rename the other methods to make it clear that there\u0027s only ever going to be one.","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f6e66bf96dee5b15ea3ed54d0a2463c81088a0c7","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"},{"line_number":48,"context_line":"            # TODO(Sundar): Need to check for event code too."},{"line_number":49,"context_line":"            LOG.info(\u0027Nova returned 207.\u0027)"},{"line_number":50,"context_line":"            return True"},{"line_number":51,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_93a4d8b9","line":48,"range":{"start_line":48,"start_character":46,"end_line":48,"end_character":56},"in_reply_to":"3fa7e38b_0ab99114","updated":"2019-12-09 17:43:16.000000000","message":"We\u0027re fixing bug 1855752 via https://review.opendev.org/698037 to make this behave as we expected/thought.","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"97f2a7cc11244b15912ff526746f99cc48d8ee4d","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"},{"line_number":48,"context_line":"            # TODO(Sundar): Need to check for event code too."},{"line_number":49,"context_line":"            LOG.info(\u0027Nova returned 207.\u0027)"},{"line_number":50,"context_line":"            return True"},{"line_number":51,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_0ab99114","line":48,"range":{"start_line":48,"start_character":46,"end_line":48,"end_character":56},"in_reply_to":"3fa7e38b_24984318","updated":"2019-12-09 15:44:21.000000000","message":"\u003e (BTW, I still feel strongly that you should do the \"cyborg only\n \u003e uses one event\" refactor regardless.)\n\nI see you did this in PS3, ++","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fc69eaa472129faee447e893e6f1fcbaa41ae474","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"},{"line_number":48,"context_line":"            # TODO(Sundar): Need to check for event code too."},{"line_number":49,"context_line":"            LOG.info(\u0027Nova returned 207.\u0027)"},{"line_number":50,"context_line":"            return True"},{"line_number":51,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_f2ae74e2","line":48,"range":{"start_line":48,"start_character":46,"end_line":48,"end_character":56},"in_reply_to":"3fa7e38b_413f7cde","updated":"2019-12-08 18:53:40.000000000","message":"\u003e Cyborg sends only one event (for multiple ARQs of a single VM) but\n \u003e the os-server-external-events API takes an array of events as part\n \u003e of its interface [1].\n\nI get that. That\u0027s my point. The methods you have here are kind of mixed as to whether they appear to expect/emit one or multiple elements in the array. They should be consistent, for readability/maintainability. If I were doing it, everything would be single-element until you actually do the POST. _send_events(self, events) would be _send_event(self, event), and L35 would do\n\n body \u003d {\"events\": [event]}\n\nThat way it\u0027s not surprising when, later in the method, you say:\n\n # We only sent one event, so there\u0027s only one in the response\n resp_event \u003d response.json()[\u0027events\u0027][0]\n\n \u003e Also, the response is HTTP 404, and has no \u0027events\u0027 key:\n\nThis isn\u0027t the condition we were going for. HTTP 404 means the instance for which you were trying to send events doesn\u0027t exist. That *should* be a failure here, because the instance should exist when the bind occurs.\n\nThe condition we want to allow is HTTP 207 where the *event* code for the (single) event is 422.\n\nMore in PS2...","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"1ac5b92c9bf41de0356501732c3535cd288f3216","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"},{"line_number":48,"context_line":"            # TODO(Sundar): Need to check for event code too."},{"line_number":49,"context_line":"            LOG.info(\u0027Nova returned 207.\u0027)"},{"line_number":50,"context_line":"            return True"},{"line_number":51,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_413f7cde","line":48,"range":{"start_line":48,"start_character":46,"end_line":48,"end_character":56},"in_reply_to":"3fa7e38b_c9d0a0cb","updated":"2019-12-08 01:58:12.000000000","message":"Cyborg sends only one event (for multiple ARQs of a single VM) but the os-server-external-events API takes an array of events as part of its interface [1]. \n\n[1] https://docs.openstack.org/api-ref/compute/?expanded\u003drun-events-detail\n\nAlso, the response is HTTP 404, and has no \u0027events\u0027 key:\n\n {\u0027cookies\u0027: \u003c\u003cclass \u0027requests.cookies.RequestsCookieJar\u0027\u003e[]\u003e,\n  \u0027_content\u0027: \u0027\n    {\"itemNotFound\": \n      {\"message\": \"No instances found for any event\", \"code\": 404}\n    }\u0027,\n  \u0027headers\u0027: {\n     \u0027Content-Length\u0027: \u002778\u0027,\n     \u0027x-compute-request-id\u0027: \u0027req-83a667c8-3deb-4f71-8dbd-6d2f0fb7a428\u0027,\n     \u0027Vary\u0027: \u0027OpenStack-API-Version,X-OpenStack-Nova-API-Version\u0027,\n     \u0027Server\u0027: \u0027Apache/2.4.6 (CentOS) OpenSSL/1.0.2k-fips mod_wsgi/3.4 Python/2.7.5\u0027,\n     \u0027OpenStack-API-Version\u0027: \u0027compute 2.82\u0027,\n     \u0027Connection\u0027: \u0027close\u0027,\n     \u0027X-OpenStack-Nova-API-Version\u0027: \u00272.82\u0027,\n     \u0027Date\u0027: \u0027Sun, 08 Dec 2019 01:03:20 GMT\u0027,\n     \u0027Content-Type\u0027: \u0027application/json; charset\u003dUTF-8\u0027,\n     \u0027x-openstack-request-id\u0027: \u0027req-83a667c8-3deb-4f71-8dbd-6d2f0fb7a428\u0027},\n     \u0027url\u0027: u\u0027http://172.25.103.180/compute/v2.1/os-server-external-events\u0027,\n     \u0027status_code\u0027: 404,\n     \u0027_content_consumed\u0027: True,\n     \u0027encoding\u0027: \u0027UTF-8\u0027,\n     \u0027request\u0027: \u003cPreparedRequest [POST]\u003e,\n     \u0027connection\u0027: \u003ckeystoneauth1.session.TCPKeepAliveAdapter object at 0x7fc25adf5410\u003e,\n     \u0027elapsed\u0027: datetime.timedelta(0, 0, 124412),\n     \u0027raw\u0027: \u003curllib3.response.HTTPResponse object at 0x7fc25ae0c390\u003e,\n     \u0027reason\u0027: \u0027Not Found\u0027,\n     \u0027_next\u0027: None,\n     \u0027history\u0027: []\n }\n\nNova API logs also show HTTP 404:\n\n Dec 07 17:03:20 otcfpga2 devstack@n-api.service[14529]: DEBUG \nnova.api.openstack.compute.server_external_events [None req-83a667c8-3deb-4f71-8dbd-6d2f0fb7a428 service nova] Unable to find a host for instance 00344b9f-b608-4248-8c50-b39210ef955e. Dropping event accelerator-requests-bound {{(pid\u003d14538) create /opt/stack/nova/nova/api/openstack/compute/server_external_events.py:143}}\nDec 07 17:03:20 otcfpga2 devstack@n-api.service[14529]: INFO nova.api.openstack.wsgi [None req-83a667c8-3deb-4f71-8dbd-6d2f0fb7a428 service nova] HTTP exception thrown: No instances found for any event\nDec 07 17:03:20 otcfpga2 devstack@n-api.service[14529]: DEBUG nova.api.openstack.wsgi [None req-83a667c8-3deb-4f71-8dbd-6d2f0fb7a428 service nova] Returning 404 to user: No instances found for any event {{(pid\u003d14538) __call__ /opt/stack/nova/nova/api/openstack/wsgi.py:941}}\nDec 07 17:03:20 otcfpga2 devstack@n-api.service[14529]: INFO nova.api.openstack.requestlog [None req-83a667c8-3deb-4f71-8dbd-6d2f0fb7a428 service nova] 172.25.103.180 \"POST /compute/v2.1/os-server-external-events\" status: 404 len: 78 microversion: 2.82 time: 0.119942\n\nSo, I have changed the implementation to use the 404 response body.","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"4c1e6c3563554775c06242fc46ce1c023ffeec07","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"},{"line_number":48,"context_line":"            # TODO(Sundar): Need to check for event code too."},{"line_number":49,"context_line":"            LOG.info(\u0027Nova returned 207.\u0027)"},{"line_number":50,"context_line":"            return True"},{"line_number":51,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_fff2bce2","line":48,"range":{"start_line":48,"start_character":46,"end_line":48,"end_character":56},"in_reply_to":"3fa7e38b_f2ae74e2","updated":"2019-12-09 03:42:16.000000000","message":"\u003e This isn\u0027t the condition we were going for. HTTP 404 means the\n \u003e instance for which you were trying to send events doesn\u0027t exist.\n \u003e That *should* be a failure here, because the instance should exist\n \u003e when the bind occurs.\n \u003e \n \u003e The condition we want to allow is HTTP 207 where the *event* code\n \u003e for the (single) event is 422.\n\nWhen there is no host associated with an instance, Nova code enters [1], which sets the result to HTTP 207. \n\n*However*, in Line 146 [2], accepted_events is [], so an exception is raised with the message \u0027No instances found for any event\u0027, and that sends HTTP 404. That is a misleading message, but that is what we get today. It could be considered a bug: arguably, the code should have run the equivalent of lines 155-157 instead of throwing this exception.\n\n[1] https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/server_external_events.py#L137\n\n[2] https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/server_external_events.py#L146","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"488f22d5e9e788806509c72bf57e5f8b92c29377","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            # a host yet. See"},{"line_number":46,"context_line":"            # https://review.opendev.org/#/c/631244/46/nova/"},{"line_number":47,"context_line":"            #     compute/manager.py@2627"},{"line_number":48,"context_line":"            # TODO(Sundar): Need to check for event code too."},{"line_number":49,"context_line":"            LOG.info(\u0027Nova returned 207.\u0027)"},{"line_number":50,"context_line":"            return True"},{"line_number":51,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_24984318","line":48,"range":{"start_line":48,"start_character":46,"end_line":48,"end_character":56},"in_reply_to":"3fa7e38b_fff2bce2","updated":"2019-12-09 15:43:23.000000000","message":"Hm. I see, yes.\n\nI thought it was you who reported that 207/422 originally...\n\n...here [1]; but yeah, you only mentioned the debug log, not the final result.\n\nUpon closer inspection, I agree you\u0027ll get 404 whether the instance is not yet assigned to a host, or has disappeared entirely. In the latter case, presumably you would want to clean up (unbind, free the accel, etc.).\n\nThe obvious hack that comes to mind is to do an additional instance query (GET /servers/{uuid}) and take the error path if that\u0027s 404. That gets you pretty close, but it\u0027s imperfect and racy as well.\n\nLet\u0027s see if Dan has any better ideas...\n\n(BTW, I still feel strongly that you should do the \"cyborg only uses one event\" refactor regardless.)\n\n[1] https://review.opendev.org/#/c/631244/46/nova/compute/manager.py@2627","commit_id":"b234e35d460c3ec0e252dd528fcecc4f46155ed6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fc69eaa472129faee447e893e6f1fcbaa41ae474","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        url \u003d \"/os-server-external-events\""},{"line_number":36,"context_line":"        body \u003d {\"events\": events}"},{"line_number":37,"context_line":"        response \u003d self.nova_client.post(url, json\u003dbody)"},{"line_number":38,"context_line":"        if response.ok:"},{"line_number":39,"context_line":"            LOG.info(\"Sucessfully send events to Nova, events: %(events)s\","},{"line_number":40,"context_line":"                     {\"events\": events})"},{"line_number":41,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_d28a5863","line":38,"range":{"start_line":38,"start_character":11,"end_line":38,"end_character":22},"updated":"2019-12-08 18:53:40.000000000","message":"Note that response.ok hits on anything \u003c400, so your new code needs to go in this `if`, not the `else`. (FWIW the .ok is redundant; Response implements __bool__ which just answers self.ok.)","commit_id":"54041b592a45397cb01dfe685f89c1390adc8c6f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fc69eaa472129faee447e893e6f1fcbaa41ae474","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        body \u003d {\"events\": events}"},{"line_number":37,"context_line":"        response \u003d self.nova_client.post(url, json\u003dbody)"},{"line_number":38,"context_line":"        if response.ok:"},{"line_number":39,"context_line":"            LOG.info(\"Sucessfully send events to Nova, events: %(events)s\","},{"line_number":40,"context_line":"                     {\"events\": events})"},{"line_number":41,"context_line":"            return True"},{"line_number":42,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_12da3078","line":39,"range":{"start_line":39,"start_character":22,"end_line":39,"end_character":38},"updated":"2019-12-08 18:53:40.000000000","message":"Successfully sent","commit_id":"54041b592a45397cb01dfe685f89c1390adc8c6f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fc69eaa472129faee447e893e6f1fcbaa41ae474","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                             \u0027instance %s is not yet associated with a host.\u0027,"},{"line_number":54,"context_line":"                             events[0][\u0027server_uuid\u0027])"},{"line_number":55,"context_line":"                    return True"},{"line_number":56,"context_line":"            raise Exception("},{"line_number":57,"context_line":"                \"Failed to send events %s: HTTP %d: %s\" %"},{"line_number":58,"context_line":"                (events, response.status_code, response.text))"},{"line_number":59,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_f287545a","line":56,"updated":"2019-12-08 18:53:40.000000000","message":"As noted in PS1 comments, you want the overall HTTP 404 to continue to raise, as that\u0027s a real, unexpected error condition","commit_id":"54041b592a45397cb01dfe685f89c1390adc8c6f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fc69eaa472129faee447e893e6f1fcbaa41ae474","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            raise Exception("},{"line_number":57,"context_line":"                \"Failed to send events %s: HTTP %d: %s\" %"},{"line_number":58,"context_line":"                (events, response.status_code, response.text))"},{"line_number":59,"context_line":"            return False"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def notify_binding(self, instance_uuid, dev_profile_name, status):"},{"line_number":62,"context_line":"        events \u003d self._get_acc_changed_event(instance_uuid, dev_profile_name,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_92e6204a","line":59,"range":{"start_line":59,"start_character":12,"end_line":59,"end_character":24},"updated":"2019-12-08 18:53:40.000000000","message":"(unrelated) This is unreachable due to the foregoing exception.\n\nThat said, what\u0027s the actual interface of this method supposed to be on error? Return False or raise an exception? (A docstring wouldn\u0027t hurt.)","commit_id":"54041b592a45397cb01dfe685f89c1390adc8c6f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fc69eaa472129faee447e893e6f1fcbaa41ae474","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                \"Failed to send events %s: HTTP %d: %s\" %"},{"line_number":58,"context_line":"                (events, response.status_code, response.text))"},{"line_number":59,"context_line":"            return False"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def notify_binding(self, instance_uuid, dev_profile_name, status):"},{"line_number":62,"context_line":"        events \u003d self._get_acc_changed_event(instance_uuid, dev_profile_name,"},{"line_number":63,"context_line":"                                             status)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_72f12400","line":60,"updated":"2019-12-08 18:53:40.000000000","message":"summary:\n- I would refactor (which can be a different patch if you like) as noted in PS1 to make everything in this module clear that there\u0027s only one event being sent.\n- The main logic should look something like:\n\n if response:\n     if response.status_code \u003d\u003d 207:\n         # There\u0027s only one event, because we only sent one\n         code \u003d response.json()[\u0027events\u0027][0][\u0027code\u0027]\n         if code \u003d\u003d 422:\n             LOG.info(\u0027Success: We won the race...\u0027)\n             return True\n         else:\n             # \u0027status\u0027 is going to be \u0027failed\u0027, so not worth\n             # including in the message :(\n             LOG.error(\u0027Bad things: %d\u0027,  code)\n             return False\n     # \"normal\" 2xx success\n     LOG.info(\"Successfully sent...\")\n     return True\n # 4xx/5xx failure\n LOG.error(\u0027Bad things (%d): %s\u0027, response.status_code, response.text)\n return False","commit_id":"54041b592a45397cb01dfe685f89c1390adc8c6f"}]}