)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"edcf52d9974a32814696b559c1761fb8a13e2f5e","unresolved":false,"context_lines":[{"line_number":20,"context_line":"up again only after restart."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"While the intention of I55417a5b91282c69432bb2ab64441c5cea474d31 was"},{"line_number":23,"context_line":"to avoid cathing all the possible exceptions, but it looks like we must"},{"line_number":24,"context_line":"do that to avoid creating a zombie."},{"line_number":25,"context_line":"The other part of that change was to ensure that during upgrade, we do"},{"line_number":26,"context_line":"not spam the log server about MessagingTimeouts while the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9a8ffd7b_42579726","line":23,"updated":"2015-11-30 16:33:39.000000000","message":"s/cathing/catching","commit_id":"49b0d1741c674714fabf24d8409810064b953202"}],"nova/servicegroup/drivers/db.py":[{"author":{"_account_id":18940,"name":"Gyorgy Szombathelyi","email":"gyurco@freemail.hu","username":"gyurco78"},"change_message_id":"df1f7f28c251fd9443e70a723caba55b352a05df","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        else:"},{"line_number":92,"context_line":"            # need to catch messaging errors (timeout or general RemoteError)"},{"line_number":93,"context_line":"            exc_cls \u003d (messaging.MessagingTimeout, messaging.RemoteError)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        try:"},{"line_number":96,"context_line":"            service.service_ref.report_count +\u003d 1"},{"line_number":97,"context_line":"            service.service_ref.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_d491c19b","line":94,"updated":"2015-11-23 12:23:32.000000000","message":"And what if the dberror occurs with the conductor?","commit_id":"ab84c344bac37449b6f73f0074e86adb71d1b793"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"fc44edb245cde4bb4d360172578fdbb6e46274d7","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        else:"},{"line_number":92,"context_line":"            # need to catch messaging errors (timeout or general RemoteError)"},{"line_number":93,"context_line":"            exc_cls \u003d (messaging.MessagingTimeout, messaging.RemoteError)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        try:"},{"line_number":96,"context_line":"            service.service_ref.report_count +\u003d 1"},{"line_number":97,"context_line":"            service.service_ref.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_e6a0cda7","line":94,"in_reply_to":"ba8a016a_d491c19b","updated":"2015-11-23 16:20:44.000000000","message":"so it will be a DBError on the conductor side and on compute side, where _report_state() is executed it will be wrapped with RemoteError","commit_id":"ab84c344bac37449b6f73f0074e86adb71d1b793"},{"author":{"_account_id":18940,"name":"Gyorgy Szombathelyi","email":"gyurco@freemail.hu","username":"gyurco78"},"change_message_id":"91290c4525d9c0e15c5b4418ed95cce64cc5a662","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        except exc_cls:"},{"line_number":108,"context_line":"            if not getattr(service, \u0027model_disconnected\u0027, False):"},{"line_number":109,"context_line":"                service.model_disconnected \u003d True"},{"line_number":110,"context_line":"                LOG.warn(_LW(\u0027Lost connection to nova-conductor \u0027"},{"line_number":111,"context_line":"                             \u0027for reporting service status.\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_fdd5c91a","line":110,"updated":"2015-11-23 13:05:55.000000000","message":"Please readd the except Exception: in some form (e.g. log the error). The mentioned regression in the commit comment just removed it.","commit_id":"ab84c344bac37449b6f73f0074e86adb71d1b793"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"fc44edb245cde4bb4d360172578fdbb6e46274d7","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        except exc_cls:"},{"line_number":108,"context_line":"            if not getattr(service, \u0027model_disconnected\u0027, False):"},{"line_number":109,"context_line":"                service.model_disconnected \u003d True"},{"line_number":110,"context_line":"                LOG.warn(_LW(\u0027Lost connection to nova-conductor \u0027"},{"line_number":111,"context_line":"                             \u0027for reporting service status.\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_c60ee98b","line":110,"in_reply_to":"ba8a016a_fdd5c91a","updated":"2015-11-23 16:20:44.000000000","message":"That\u0027s actually what I\u0027ve been thinking of. On the one hand - it would be great to except only specific exceptions here, on the other hand - if we miss something, report_state thread will stop abruptly...","commit_id":"ab84c344bac37449b6f73f0074e86adb71d1b793"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9e91207f5dd8895999ba2b569e8adeaf148c0b66","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                LOG.info("},{"line_number":103,"context_line":"                    _LI(\u0027Recovered connection to nova-conductor \u0027"},{"line_number":104,"context_line":"                        \u0027for reporting service status.\u0027))"},{"line_number":105,"context_line":"        # we\u0027d like to avoid cathing all the possible exceptions here, but"},{"line_number":106,"context_line":"        # if we don\u0027t do that, there is a possibility that reporting state"},{"line_number":107,"context_line":"        # thread will abruptly stop and the service will remain silent until"},{"line_number":108,"context_line":"        # it\u0027s restarted. We log only unexpected exceptions, though."}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_f6a38a9c","line":105,"updated":"2015-11-23 22:02:58.000000000","message":"Proper format for this is NOTE($user):\n\nAlso please put this inside the indented region under the except line.","commit_id":"4e8c5dd71cd90606afa1ed775c1f755e17384f45"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"0d35fda2e10c01722bd096c2f16cf60fb7b85963","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                LOG.info("},{"line_number":103,"context_line":"                    _LI(\u0027Recovered connection to nova-conductor \u0027"},{"line_number":104,"context_line":"                        \u0027for reporting service status.\u0027))"},{"line_number":105,"context_line":"        # we\u0027d like to avoid cathing all the possible exceptions here, but"},{"line_number":106,"context_line":"        # if we don\u0027t do that, there is a possibility that reporting state"},{"line_number":107,"context_line":"        # thread will abruptly stop and the service will remain silent until"},{"line_number":108,"context_line":"        # it\u0027s restarted. We log only unexpected exceptions, though."}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_0f7b8129","line":105,"in_reply_to":"ba8a016a_f6a38a9c","updated":"2015-11-24 12:18:12.000000000","message":"Done","commit_id":"4e8c5dd71cd90606afa1ed775c1f755e17384f45"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9e91207f5dd8895999ba2b569e8adeaf148c0b66","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        # thread will abruptly stop and the service will remain silent until"},{"line_number":108,"context_line":"        # it\u0027s restarted. We log only unexpected exceptions, though."},{"line_number":109,"context_line":"        except Exception as exc:"},{"line_number":110,"context_line":"            if not isinstance(exc, exc_cls):"},{"line_number":111,"context_line":"                LOG.exception("},{"line_number":112,"context_line":"                    _LE(\u0027Unexpected error while reporting service status\u0027))"},{"line_number":113,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_3699126f","line":110,"updated":"2015-11-23 22:02:58.000000000","message":"I think you could catch (and ignore) these separately and just catch/log Exception if nothing else matches.","commit_id":"4e8c5dd71cd90606afa1ed775c1f755e17384f45"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"0d35fda2e10c01722bd096c2f16cf60fb7b85963","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        # thread will abruptly stop and the service will remain silent until"},{"line_number":108,"context_line":"        # it\u0027s restarted. We log only unexpected exceptions, though."},{"line_number":109,"context_line":"        except Exception as exc:"},{"line_number":110,"context_line":"            if not isinstance(exc, exc_cls):"},{"line_number":111,"context_line":"                LOG.exception("},{"line_number":112,"context_line":"                    _LE(\u0027Unexpected error while reporting service status\u0027))"},{"line_number":113,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_ef7ddd1c","line":110,"in_reply_to":"ba8a016a_3699126f","updated":"2015-11-24 12:18:12.000000000","message":"Done","commit_id":"4e8c5dd71cd90606afa1ed775c1f755e17384f45"},{"author":{"_account_id":18940,"name":"Gyorgy Szombathelyi","email":"gyurco@freemail.hu","username":"gyurco78"},"change_message_id":"3a9bc13592646b4e327cc9ea6fad9bbb54145f1d","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            if not getattr(service, \u0027model_disconnected\u0027, False):"},{"line_number":115,"context_line":"                service.model_disconnected \u003d True"},{"line_number":116,"context_line":"                LOG.warn(_LW(\u0027Lost connection to nova-conductor \u0027"},{"line_number":117,"context_line":"                             \u0027for reporting service status.\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_10973591","line":117,"updated":"2015-11-23 21:32:04.000000000","message":"It works, but the log messages will be misleading:\n 1. DBError in conductor/scheduler/consolauth will results as an \"Unexpected error while reporting...\"\n 2. Any error in conductor/scheduler/consolauth will log \"Lost connection to nova-conductor\"\n 3. DBError in compute with local conductor (conductor-less) will report \"Lost connection to nova-conductor\"\n\n I would write 3 exception handler (sorry for not providing the patch itself):\n\nexcept messaging.MessagingTimeout, messaging.RemoteError:\n  LOG(lost connection to nova-conductor)\n  disconnect\nexcept db_exception.DBError:\n  LOG(lost connection to DB)\n  disconnect\nexcept Exception:\n  LOG(Unexpected error)\n  disconnect\n\nor with one exception:\nexcept Exception as exc:\n  if isintance(exc,(messaging.MessagingTimeout, messaging.RemoteError)):\n    LOG (lost connection to nova-conductor)\n  elif isinstance(exc, db_exception.DBError:\n    LOG (db error)\n  else:\n    LOG (unexpected error)\n\n  disconnect\n\n\n\nAnd no need to check CONF.conductor.use_local","commit_id":"4e8c5dd71cd90606afa1ed775c1f755e17384f45"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"0d35fda2e10c01722bd096c2f16cf60fb7b85963","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            if not getattr(service, \u0027model_disconnected\u0027, False):"},{"line_number":115,"context_line":"                service.model_disconnected \u003d True"},{"line_number":116,"context_line":"                LOG.warn(_LW(\u0027Lost connection to nova-conductor \u0027"},{"line_number":117,"context_line":"                             \u0027for reporting service status.\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_af73554d","line":117,"in_reply_to":"ba8a016a_10973591","updated":"2015-11-24 12:18:12.000000000","message":"Done","commit_id":"4e8c5dd71cd90606afa1ed775c1f755e17384f45"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"bd423228e1182e346d59da53ca66e801f917dea1","unresolved":false,"context_lines":[{"line_number":98,"context_line":"            if getattr(service, \u0027model_disconnected\u0027, False):"},{"line_number":99,"context_line":"                service.model_disconnected \u003d False"},{"line_number":100,"context_line":"                LOG.info("},{"line_number":101,"context_line":"                    _LI(\u0027Recovered service state reporting after failure.\u0027))"},{"line_number":102,"context_line":"        except db_exception.DBError:"},{"line_number":103,"context_line":"            self._handle_reporting_error("},{"line_number":104,"context_line":"                service,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_85a45fa6","line":101,"updated":"2015-11-25 16:35:35.000000000","message":"Maybe: \"Recovered from being unable to report status\"?","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"8c2f1547a0cd7f817e3f4ca8ef00704a20c15dc0","unresolved":false,"context_lines":[{"line_number":98,"context_line":"            if getattr(service, \u0027model_disconnected\u0027, False):"},{"line_number":99,"context_line":"                service.model_disconnected \u003d False"},{"line_number":100,"context_line":"                LOG.info("},{"line_number":101,"context_line":"                    _LI(\u0027Recovered service state reporting after failure.\u0027))"},{"line_number":102,"context_line":"        except db_exception.DBError:"},{"line_number":103,"context_line":"            self._handle_reporting_error("},{"line_number":104,"context_line":"                service,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_5bff34aa","line":101,"in_reply_to":"9a8ffd7b_85a45fa6","updated":"2015-11-25 18:13:04.000000000","message":"Done","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"bd423228e1182e346d59da53ca66e801f917dea1","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                service.model_disconnected \u003d False"},{"line_number":100,"context_line":"                LOG.info("},{"line_number":101,"context_line":"                    _LI(\u0027Recovered service state reporting after failure.\u0027))"},{"line_number":102,"context_line":"        except db_exception.DBError:"},{"line_number":103,"context_line":"            self._handle_reporting_error("},{"line_number":104,"context_line":"                service,"},{"line_number":105,"context_line":"                _LW(\u0027DB error while reporting service status.\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_65c05b2b","line":102,"updated":"2015-11-25 16:35:35.000000000","message":"I think we should treat DBError and MessingTimout the same, assume its a \"blip\" during an upgrade, so just log a warning when we first see that happen.","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"8c2f1547a0cd7f817e3f4ca8ef00704a20c15dc0","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                service.model_disconnected \u003d False"},{"line_number":100,"context_line":"                LOG.info("},{"line_number":101,"context_line":"                    _LI(\u0027Recovered service state reporting after failure.\u0027))"},{"line_number":102,"context_line":"        except db_exception.DBError:"},{"line_number":103,"context_line":"            self._handle_reporting_error("},{"line_number":104,"context_line":"                service,"},{"line_number":105,"context_line":"                _LW(\u0027DB error while reporting service status.\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_56e38b33","line":102,"in_reply_to":"9a8ffd7b_65c05b2b","updated":"2015-11-25 18:13:04.000000000","message":"I\u0027d actually go for that. As long as it\u0027s an \u0027expected\u0027 error in DB or RPC, I don\u0027t really care which one that is.\n\nDone.","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"bd423228e1182e346d59da53ca66e801f917dea1","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            self._handle_reporting_error("},{"line_number":104,"context_line":"                service,"},{"line_number":105,"context_line":"                _LW(\u0027DB error while reporting service status.\u0027))"},{"line_number":106,"context_line":"        except (messaging.MessagingTimeout, messaging.RemoteError):"},{"line_number":107,"context_line":"            self._handle_reporting_error("},{"line_number":108,"context_line":"                service,"},{"line_number":109,"context_line":"                _LW(\u0027Lost connection to nova-conductor \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_c5a58704","line":106,"updated":"2015-11-25 16:35:35.000000000","message":"I think RemoteError probably should go into the general exception clause here. I assume thats when we see an actual error.\n\nI think the idea is just to log a warning when you first hit an \"expected\" issue during upgrade.","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"8c2f1547a0cd7f817e3f4ca8ef00704a20c15dc0","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            self._handle_reporting_error("},{"line_number":104,"context_line":"                service,"},{"line_number":105,"context_line":"                _LW(\u0027DB error while reporting service status.\u0027))"},{"line_number":106,"context_line":"        except (messaging.MessagingTimeout, messaging.RemoteError):"},{"line_number":107,"context_line":"            self._handle_reporting_error("},{"line_number":108,"context_line":"                service,"},{"line_number":109,"context_line":"                _LW(\u0027Lost connection to nova-conductor \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_366da798","line":106,"in_reply_to":"9a8ffd7b_c5a58704","updated":"2015-11-25 18:13:04.000000000","message":"Agreed. Done.","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"efbfb374c061d88361fa78a9868e83d293af2cf1","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        except (messaging.MessagingTimeout, messaging.RemoteError):"},{"line_number":107,"context_line":"            self._handle_reporting_error("},{"line_number":108,"context_line":"                service,"},{"line_number":109,"context_line":"                _LW(\u0027Lost connection to nova-conductor \u0027"},{"line_number":110,"context_line":"                    \u0027for reporting service status.\u0027))"},{"line_number":111,"context_line":"        except Exception:"},{"line_number":112,"context_line":"            # NOTE(rpodolyaka): we\u0027d like to avoid cathing of all possible"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_85dc5fe5","line":109,"updated":"2015-11-25 16:36:57.000000000","message":"Maybe change this to: \"Currently unable to report status to database\", or something like that. That should cover conductor and non-conductor cases.","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"8c2f1547a0cd7f817e3f4ca8ef00704a20c15dc0","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        except (messaging.MessagingTimeout, messaging.RemoteError):"},{"line_number":107,"context_line":"            self._handle_reporting_error("},{"line_number":108,"context_line":"                service,"},{"line_number":109,"context_line":"                _LW(\u0027Lost connection to nova-conductor \u0027"},{"line_number":110,"context_line":"                    \u0027for reporting service status.\u0027))"},{"line_number":111,"context_line":"        except Exception:"},{"line_number":112,"context_line":"            # NOTE(rpodolyaka): we\u0027d like to avoid cathing of all possible"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_718a59c1","line":109,"in_reply_to":"9a8ffd7b_85dc5fe5","updated":"2015-11-25 18:13:04.000000000","message":"Heh, np. It\u0027s stored / checked in DB, fwiw.\n\nDone","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"5fd4c24a128b903322c97c1b8014bf206cce049c","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                _LW(\u0027Lost connection to nova-conductor \u0027"},{"line_number":110,"context_line":"                    \u0027for reporting service status.\u0027))"},{"line_number":111,"context_line":"        except Exception:"},{"line_number":112,"context_line":"            # NOTE(rpodolyaka): we\u0027d like to avoid cathing of all possible"},{"line_number":113,"context_line":"            # exceptions here, but otherwise it would become possible for"},{"line_number":114,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":115,"context_line":"            # the service unusable until it\u0027s restarted"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_2586d365","line":112,"updated":"2015-11-25 16:34:45.000000000","message":"nit: catching","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"8c2f1547a0cd7f817e3f4ca8ef00704a20c15dc0","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                _LW(\u0027Lost connection to nova-conductor \u0027"},{"line_number":110,"context_line":"                    \u0027for reporting service status.\u0027))"},{"line_number":111,"context_line":"        except Exception:"},{"line_number":112,"context_line":"            # NOTE(rpodolyaka): we\u0027d like to avoid cathing of all possible"},{"line_number":113,"context_line":"            # exceptions here, but otherwise it would become possible for"},{"line_number":114,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":115,"context_line":"            # the service unusable until it\u0027s restarted"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_3159315b","line":112,"in_reply_to":"9a8ffd7b_2586d365","updated":"2015-11-25 18:13:04.000000000","message":"oops, thanks!\n\nDone","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"bd423228e1182e346d59da53ca66e801f917dea1","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            # exceptions here, but otherwise it would become possible for"},{"line_number":114,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":115,"context_line":"            # the service unusable until it\u0027s restarted"},{"line_number":116,"context_line":"            LOG.exception("},{"line_number":117,"context_line":"                _LE(\u0027Unexpected error while reporting service status\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_651cdbc1","line":116,"updated":"2015-11-25 16:35:35.000000000","message":"This is good, we log the details on ever unexpected error we see.","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"49aa4d69d69fa9cf9574588e4c89ef06f22a32e7","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"CONF \u003d cfg.CONF"},{"line_number":29,"context_line":"CONF.import_opt(\u0027service_down_time\u0027, \u0027nova.service\u0027)"},{"line_number":30,"context_line":"CONF.import_opt(\u0027use_local\u0027, \u0027nova.conductor.api\u0027, group\u003d\u0027conductor\u0027)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_6624a65f","line":30,"updated":"2015-11-26 15:32:33.000000000","message":"femtonit: you should remove that line too","commit_id":"36a90091def3bca6327c7a1b3d8067e219a2d7b6"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d4bd93b001d0a341fcc21f82b116a499bb43fa25","unresolved":false,"context_lines":[{"line_number":94,"context_line":"                service.model_disconnected \u003d False"},{"line_number":95,"context_line":"                LOG.info("},{"line_number":96,"context_line":"                    _LI(\u0027Recovered from being unable to report status.\u0027))"},{"line_number":97,"context_line":"        except (db_exception.DBError, messaging.MessagingException):"},{"line_number":98,"context_line":"            # NOTE(rpodolyaka): DB and RPC errors are expected in a sense that"},{"line_number":99,"context_line":"            # they can happen on DB / MQ failover, so we just log a warning"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_534efd3b","line":97,"updated":"2015-11-26 16:35:56.000000000","message":"Hmm, why is this not MessagingTimeout? That would keep it inline with the old behaviour I guess.","commit_id":"36a90091def3bca6327c7a1b3d8067e219a2d7b6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"cebd23632911de59d4deea28d9321722f087cd84","unresolved":false,"context_lines":[{"line_number":94,"context_line":"                service.model_disconnected \u003d False"},{"line_number":95,"context_line":"                LOG.info("},{"line_number":96,"context_line":"                    _LI(\u0027Recovered from being unable to report status.\u0027))"},{"line_number":97,"context_line":"        except (db_exception.DBError, messaging.MessagingException):"},{"line_number":98,"context_line":"            # NOTE(rpodolyaka): DB and RPC errors are expected in a sense that"},{"line_number":99,"context_line":"            # they can happen on DB / MQ failover, so we just log a warning"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_ee8068c2","line":97,"in_reply_to":"9a8ffd7b_534efd3b","updated":"2015-11-26 16:57:15.000000000","message":"Yeah, I\u0027d prefer to only catch MessagingTimeout","commit_id":"36a90091def3bca6327c7a1b3d8067e219a2d7b6"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fd8e633574b5cce3341a5548fa4a607e1b1c8ac7","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            # NOTE(rpodolyaka): we\u0027d like to avoid catching of all possible"},{"line_number":106,"context_line":"            # exceptions here, but otherwise it would become possible for"},{"line_number":107,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":108,"context_line":"            # the service unusable until it\u0027s restarted"},{"line_number":109,"context_line":"            LOG.exception("},{"line_number":110,"context_line":"                _LE(\u0027Unexpected error while reporting service status\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_9380e5f8","line":108,"updated":"2015-11-26 16:38:11.000000000","message":"you know, we should still set service.model_disconnected \u003d True here, even though we don\u0027t check it, so we get the \"I have recovered\" log message.","commit_id":"36a90091def3bca6327c7a1b3d8067e219a2d7b6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"49aa4d69d69fa9cf9574588e4c89ef06f22a32e7","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":108,"context_line":"            # the service unusable until it\u0027s restarted"},{"line_number":109,"context_line":"            LOG.exception("},{"line_number":110,"context_line":"                _LE(\u0027Unexpected error while reporting service status\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_668026f8","line":110,"updated":"2015-11-26 15:32:33.000000000","message":"not sure I\u0027m a big fan of that... The current behaviour is that if the MQ goes down, the service is seen as down, right?\n\nSince lots of our API long-running actions are async\u0027d by doing a RPC cast to the compute manager, it would mean that the API could continue to send some messages while the MQ is trampling them.","commit_id":"36a90091def3bca6327c7a1b3d8067e219a2d7b6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"cebd23632911de59d4deea28d9321722f087cd84","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":108,"context_line":"            # the service unusable until it\u0027s restarted"},{"line_number":109,"context_line":"            LOG.exception("},{"line_number":110,"context_line":"                _LE(\u0027Unexpected error while reporting service status\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_8e0afc77","line":110,"in_reply_to":"9a8ffd7b_50611304","updated":"2015-11-26 16:57:15.000000000","message":"Nevermind my above comment, I made a misinterpretation.\n\nThat said, I feel we should be very cautious about what should be automatically recovering if something else than just a MessagingTimeout is raised.\n\nAs an operator, I\u0027d by far prefer to have the compute status being reported as down rather than keeping pushing new VMs to my host if something bad happened.","commit_id":"36a90091def3bca6327c7a1b3d8067e219a2d7b6"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1c4fe2ac3ec2a50196f17c0389479c2f498d7b09","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":108,"context_line":"            # the service unusable until it\u0027s restarted"},{"line_number":109,"context_line":"            LOG.exception("},{"line_number":110,"context_line":"                _LE(\u0027Unexpected error while reporting service status\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_70af77e2","line":110,"in_reply_to":"9a8ffd7b_668026f8","updated":"2015-11-26 16:14:06.000000000","message":"So I don\u0027t think that changes that...\n\nI think service.model_disconnected \u003d True simply suppresses multiple logs for the same error.\n\nWhen its an unexpected error, I am OK with us spamming the logs, I think.","commit_id":"36a90091def3bca6327c7a1b3d8067e219a2d7b6"},{"author":{"_account_id":18940,"name":"Gyorgy Szombathelyi","email":"gyurco@freemail.hu","username":"gyurco78"},"change_message_id":"65796bc730729321ae9c30aa5233353956b8afd7","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":108,"context_line":"            # the service unusable until it\u0027s restarted"},{"line_number":109,"context_line":"            LOG.exception("},{"line_number":110,"context_line":"                _LE(\u0027Unexpected error while reporting service status\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_50611304","line":110,"in_reply_to":"9a8ffd7b_668026f8","updated":"2015-11-26 16:30:52.000000000","message":"The patch will not change the current behavior in this case, just fixes a critical bug.","commit_id":"36a90091def3bca6327c7a1b3d8067e219a2d7b6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8340b90d9ae5cf6072f2c5d167ebb65cadae686c","unresolved":false,"context_lines":[{"line_number":90,"context_line":"            exc_cls \u003d db_exception.DBError  # oslo.db exception base class"},{"line_number":91,"context_line":"        else:"},{"line_number":92,"context_line":"            # need to catch messaging timeouts"},{"line_number":93,"context_line":"            exc_cls \u003d messaging.MessagingTimeout"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        try:"},{"line_number":96,"context_line":"            service.service_ref.report_count +\u003d 1"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_81563828","side":"PARENT","line":93,"updated":"2015-11-27 10:24:17.000000000","message":"so I\u0027m okay with removing that check provided that we handle the MessagingTimeout as the main error cause and that we still handle the other exceptions.","commit_id":"7bc2171869e2d7a670dc3c93494cf6ca05cac2fc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8340b90d9ae5cf6072f2c5d167ebb65cadae686c","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"CONF \u003d cfg.CONF"},{"line_number":28,"context_line":"CONF.import_opt(\u0027service_down_time\u0027, \u0027nova.service\u0027)"},{"line_number":29,"context_line":"CONF.import_opt(\u0027use_local\u0027, \u0027nova.conductor.api\u0027, group\u003d\u0027conductor\u0027)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_4165b01b","line":29,"updated":"2015-11-27 10:24:17.000000000","message":"please remove that since we no longer use that.","commit_id":"9a9731cdbb8bae6d13b8757e0cf6888444c82f7e"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"663a463414e60a10c0562d5fbdbdeeb266d06eb1","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"CONF \u003d cfg.CONF"},{"line_number":28,"context_line":"CONF.import_opt(\u0027service_down_time\u0027, \u0027nova.service\u0027)"},{"line_number":29,"context_line":"CONF.import_opt(\u0027use_local\u0027, \u0027nova.conductor.api\u0027, group\u003d\u0027conductor\u0027)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_ab3772fd","line":29,"in_reply_to":"9a8ffd7b_4165b01b","updated":"2015-11-28 13:08:03.000000000","message":"Oops, thanks! Done","commit_id":"9a9731cdbb8bae6d13b8757e0cf6888444c82f7e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8340b90d9ae5cf6072f2c5d167ebb65cadae686c","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            # NOTE(rpodolyaka): we\u0027d like to avoid catching of all possible"},{"line_number":105,"context_line":"            # exceptions here, but otherwise it would become possible for"},{"line_number":106,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":107,"context_line":"            # the service unusable until it\u0027s restarted."},{"line_number":108,"context_line":"            LOG.exception("},{"line_number":109,"context_line":"                _LE(\u0027Unexpected error while reporting service status\u0027))"},{"line_number":110,"context_line":"            # trigger the recovery log message, if this error goes away"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_7c32d7f3","line":107,"updated":"2015-11-27 10:24:17.000000000","message":"IMHO, the comment is a bit incorrect. That catch-all is due to the fact that we spawn a periodic greenlet in the join() method and that if there is an exception, that greenlet silently dies.\n\nInstead, we should comment that this is an implementation hack because of the greenlet and leave a big TODO for finding another better way to do periodic updates (like for example using the compute manager periodic tasks) but that\u0027s a follow-up change IMHO.","commit_id":"9a9731cdbb8bae6d13b8757e0cf6888444c82f7e"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"663a463414e60a10c0562d5fbdbdeeb266d06eb1","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            # NOTE(rpodolyaka): we\u0027d like to avoid catching of all possible"},{"line_number":105,"context_line":"            # exceptions here, but otherwise it would become possible for"},{"line_number":106,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":107,"context_line":"            # the service unusable until it\u0027s restarted."},{"line_number":108,"context_line":"            LOG.exception("},{"line_number":109,"context_line":"                _LE(\u0027Unexpected error while reporting service status\u0027))"},{"line_number":110,"context_line":"            # trigger the recovery log message, if this error goes away"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_6b3d0adc","line":107,"in_reply_to":"9a8ffd7b_7c32d7f3","updated":"2015-11-28 13:08:03.000000000","message":"That\u0027s an interesting point how we don\u0027t have the very same problem with periodic tasks. I believe, they are all run in the context of 1 green thread, so there must be a similar try/except for each task - it\u0027s just probably implemented one level up the stack, so that we don\u0027t have to worry about that in the task code. \n\nI\u0027ll take a look, if we can make this a periodic task.","commit_id":"9a9731cdbb8bae6d13b8757e0cf6888444c82f7e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"edcf52d9974a32814696b559c1761fb8a13e2f5e","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                LOG.warn(_LW(\u0027Lost connection to nova-conductor \u0027"},{"line_number":101,"context_line":"                             \u0027for reporting service status.\u0027))"},{"line_number":102,"context_line":"        except Exception:"},{"line_number":103,"context_line":"            # NOTE(rpodolyaka): we\u0027d like to avoid catching of all possible"},{"line_number":104,"context_line":"            # exceptions here, but otherwise it would become possible for"},{"line_number":105,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":106,"context_line":"            # the service unusable until it\u0027s restarted."}],"source_content_type":"text/x-python","patch_set":8,"id":"9a8ffd7b_5d19b678","line":103,"updated":"2015-11-30 16:33:39.000000000","message":"s/catching of all/catching all/","commit_id":"49b0d1741c674714fabf24d8409810064b953202"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4421828af5c1749d26c12a47ae705ffc7235c594","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            # NOTE(rpodolyaka): we\u0027d like to avoid catching of all possible"},{"line_number":104,"context_line":"            # exceptions here, but otherwise it would become possible for"},{"line_number":105,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":106,"context_line":"            # the service unusable until it\u0027s restarted."},{"line_number":107,"context_line":"            LOG.exception("},{"line_number":108,"context_line":"                _LE(\u0027Unexpected error while reporting service status\u0027))"},{"line_number":109,"context_line":"            # trigger the recovery log message, if this error goes away"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a8ffd7b_4e17d844","line":106,"updated":"2015-11-30 10:45:14.000000000","message":"I don\u0027t want to nitpick but I\u0027d love to get other feedback for that since it appears to me as just an implementation hack like I said in PS7.","commit_id":"49b0d1741c674714fabf24d8409810064b953202"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"edcf52d9974a32814696b559c1761fb8a13e2f5e","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            # NOTE(rpodolyaka): we\u0027d like to avoid catching of all possible"},{"line_number":104,"context_line":"            # exceptions here, but otherwise it would become possible for"},{"line_number":105,"context_line":"            # the state reporting thread to stop abruptly, and thus leave"},{"line_number":106,"context_line":"            # the service unusable until it\u0027s restarted."},{"line_number":107,"context_line":"            LOG.exception("},{"line_number":108,"context_line":"                _LE(\u0027Unexpected error while reporting service status\u0027))"},{"line_number":109,"context_line":"            # trigger the recovery log message, if this error goes away"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a8ffd7b_dd7fe664","line":106,"in_reply_to":"9a8ffd7b_4e17d844","updated":"2015-11-30 16:33:39.000000000","message":"I don\u0027t view this as a hack. We deliberately catch any and all non-Timeout exception in order to prevent any chance of the service reporting greenthread becoming a zombie.","commit_id":"49b0d1741c674714fabf24d8409810064b953202"}],"nova/tests/unit/servicegroup/test_db_servicegroup.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"5fd4c24a128b903322c97c1b8014bf206cce049c","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        service \u003d mock.MagicMock(model_disconnected\u003dFalse,"},{"line_number":96,"context_line":"                                 service_ref\u003dservice_ref)"},{"line_number":97,"context_line":"        fn \u003d self.servicegroup_api._driver._report_state"},{"line_number":98,"context_line":"        fn(service)  # fail if exception not caught"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def test_report_state_remote_error_handling(self):"},{"line_number":101,"context_line":"        # test error handling using remote conductor"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_454af7c6","line":98,"updated":"2015-11-25 16:34:45.000000000","message":"Only a nit because it wasn\u0027t there before, but as you touched it:\n\nIt would be good to test here that model_disconnected is set for the relevant circumstances.","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"449de903ae5a43e4c7646671e748ad5e610fd1b6","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        service \u003d mock.MagicMock(model_disconnected\u003dFalse,"},{"line_number":96,"context_line":"                                 service_ref\u003dservice_ref)"},{"line_number":97,"context_line":"        fn \u003d self.servicegroup_api._driver._report_state"},{"line_number":98,"context_line":"        fn(service)  # fail if exception not caught"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def test_report_state_remote_error_handling(self):"},{"line_number":101,"context_line":"        # test error handling using remote conductor"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_459437e1","line":98,"in_reply_to":"9a8ffd7b_454af7c6","updated":"2015-11-25 16:38:14.000000000","message":"Good point, we should test its not set for unexpected errors.","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"8c2f1547a0cd7f817e3f4ca8ef00704a20c15dc0","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        service \u003d mock.MagicMock(model_disconnected\u003dFalse,"},{"line_number":96,"context_line":"                                 service_ref\u003dservice_ref)"},{"line_number":97,"context_line":"        fn \u003d self.servicegroup_api._driver._report_state"},{"line_number":98,"context_line":"        fn(service)  # fail if exception not caught"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def test_report_state_remote_error_handling(self):"},{"line_number":101,"context_line":"        # test error handling using remote conductor"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_ecbaf0e0","line":98,"in_reply_to":"9a8ffd7b_459437e1","updated":"2015-11-25 18:13:04.000000000","message":"Agreed.\n\nDone.","commit_id":"abe1409549a8ebc9062fcd61d1a8744b306a2d87"}]}