)]}'
{"neutron/agent/ovsdb/impl_idl.py":[{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"924970afb61f9e4e799a462a6d8a620f365e704f","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        try:"},{"line_number":58,"context_line":"            result \u003d self.results.get(self.timeout)"},{"line_number":59,"context_line":"        except Queue.Empty as e_empty:"},{"line_number":60,"context_line":"            LOG.error(_LE(\"Transaction result is empty\"))"},{"line_number":61,"context_line":"            return e_empty"},{"line_number":62,"context_line":"        if self.check_error:"},{"line_number":63,"context_line":"            if isinstance(result, idlutils.ExceptionResult):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_1334dcdb","line":60,"range":{"start_line":60,"start_character":27,"end_line":60,"end_character":38},"updated":"2016-05-23 05:46:03.000000000","message":"Sorry to bikeshed, but if I understand you correctly this is a recovered from error and it happens in normal operations. This is not appropriate for the ERROR level at all in the logs if that\u0027s the case.\n\nERRORs should be items that the operator can take action on. This is just useless information for an operator and should be debug level.\n\nHere is the guideline for error from the openstack guidelines.[1]\n\n\n\"Error: An error has occurred and an administrator should research the event\"\n\n\nWhat exactly do you suggest that the operator research when they see this?\n\nIf this is indicative of a real problem we need to include more info about what it might mean or at least note that it\u0027s an OVSDB transaction in the message. Right now it\u0027s basically useless for an operator because it doesn\u0027t indicate what kind of transaction or anything.\n\nIf we recover from this and it\u0027s normal, this is purely debug information and nothing higher.\n\n\n1. https://wiki.openstack.org/wiki/LoggingStandards#Guidelines","commit_id":"4aa1b9b7bb6a614b69e3632d04f9da546127ffb1"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"28d93a87268dc50f206ef1dfea97e750e40888c1","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        try:"},{"line_number":58,"context_line":"            result \u003d self.results.get(self.timeout)"},{"line_number":59,"context_line":"        except Queue.Empty as e_empty:"},{"line_number":60,"context_line":"            LOG.error(_LE(\"Transaction result is empty\"))"},{"line_number":61,"context_line":"            return e_empty"},{"line_number":62,"context_line":"        if self.check_error:"},{"line_number":63,"context_line":"            if isinstance(result, idlutils.ExceptionResult):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_3f3441fa","line":60,"range":{"start_line":60,"start_character":27,"end_line":60,"end_character":38},"in_reply_to":"bab6814e_1334dcdb","updated":"2016-05-23 08:15:55.000000000","message":"Thanks for info. Ryan already had similar comment in one of previous PS.","commit_id":"4aa1b9b7bb6a614b69e3632d04f9da546127ffb1"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"602799a401f30be44e4564aba3ce1d5c20b3339b","unresolved":false,"context_lines":[{"line_number":58,"context_line":"            result \u003d self.results.get(self.timeout)"},{"line_number":59,"context_line":"        except Queue.Empty as e_empty:"},{"line_number":60,"context_line":"            LOG.error(_LE(\"Transaction result is empty\"))"},{"line_number":61,"context_line":"            return e_empty"},{"line_number":62,"context_line":"        if self.check_error:"},{"line_number":63,"context_line":"            if isinstance(result, idlutils.ExceptionResult):"},{"line_number":64,"context_line":"                if self.log_errors:"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_c6beed63","line":61,"updated":"2016-05-23 22:02:05.000000000","message":"Just a note that returning an Exception here is not something the rest of the code would expect.\n\nRe: Kevin\u0027s comment on the LOG level, this isn\u0027t really a recovery from an error. This is a workaround for an unknown exceptional case that would otherwise block indefinitely. We don\u0027t really know whether the commit has succeeded or not if we get a timeout. So if someone saw this in a log, they should open a bug because this *should never happen*, but apparently it does at least in testing.","commit_id":"4aa1b9b7bb6a614b69e3632d04f9da546127ffb1"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"456c0de3f2ca8a87d23db05a70559eadc43e8b99","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        except Queue.Empty:"},{"line_number":60,"context_line":"            raise api.TimeoutException("},{"line_number":61,"context_line":"                _(\"Commands %(commands)s exceeded timeout %(timeout)d \""},{"line_number":62,"context_line":"                  \"seconds\") % {\u0027commands\u0027: self.commands,"},{"line_number":63,"context_line":"                                \u0027timeout\u0027: self.timeout})"},{"line_number":64,"context_line":"        if self.check_error:"},{"line_number":65,"context_line":"            if isinstance(result, idlutils.ExceptionResult):"}],"source_content_type":"text/x-python","patch_set":11,"id":"bab6814e_971a9f6d","line":62,"updated":"2016-05-26 14:31:39.000000000","message":"Since self.commands in an array and BaseCommand doesn\u0027t have a __repr__, we\u0027ll just be getting the Class name. It would be handy to get the arguments as well.","commit_id":"576426ab1e83c14861b2c814e7545b9c02a20108"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"b7569fc2d5e035e6ca6403fb19cb02c158484f61","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        except Queue.Empty:"},{"line_number":60,"context_line":"            raise api.TimeoutException("},{"line_number":61,"context_line":"                _(\"Commands %(commands)s exceeded timeout %(timeout)d \""},{"line_number":62,"context_line":"                  \"seconds\") % {\u0027commands\u0027: self.commands,"},{"line_number":63,"context_line":"                                \u0027timeout\u0027: self.timeout})"},{"line_number":64,"context_line":"        if self.check_error:"},{"line_number":65,"context_line":"            if isinstance(result, idlutils.ExceptionResult):"}],"source_content_type":"text/x-python","patch_set":11,"id":"bab6814e_8e94498b","line":62,"in_reply_to":"bab6814e_971a9f6d","updated":"2016-05-26 15:37:46.000000000","message":"Good point, I thought it\u0027s there but I mixed it with the other \"debugging\" patch.","commit_id":"576426ab1e83c14861b2c814e7545b9c02a20108"}],"neutron/agent/ovsdb/native/connection.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"2cf94d9c3918df44a90df9bf8295ee5661283075","unresolved":false,"context_lines":[{"line_number":98,"context_line":"                    helper.register_table(table_name)"},{"line_number":99,"context_line":"            self.idl \u003d idl.Idl(self.connection, helper)"},{"line_number":100,"context_line":"            idlutils.wait_for_change(self.idl, self.timeout)"},{"line_number":101,"context_line":"            self.poller \u003d poller.Poller()"},{"line_number":102,"context_line":"            self.poller.timer_wait(self.timeout)"},{"line_number":103,"context_line":"            self.thread \u003d threading.Thread(target\u003dself.safe_run)"},{"line_number":104,"context_line":"            self.thread.setDaemon(True)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_b82592c3","line":101,"updated":"2016-05-18 16:57:18.000000000","message":"timer_wait() is only good for the very next call of block(). After that it gets reset to -1. So if the intention is to never wait longer than vsctl_timeout, this should go in run().","commit_id":"60378d7675214056ce85730d391182c7728fb7ac"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"62867e161bf550995a6cc013c0d56990a904eb6f","unresolved":false,"context_lines":[{"line_number":98,"context_line":"                    helper.register_table(table_name)"},{"line_number":99,"context_line":"            self.idl \u003d idl.Idl(self.connection, helper)"},{"line_number":100,"context_line":"            idlutils.wait_for_change(self.idl, self.timeout)"},{"line_number":101,"context_line":"            self.poller \u003d poller.Poller()"},{"line_number":102,"context_line":"            self.poller.timer_wait(self.timeout)"},{"line_number":103,"context_line":"            self.thread \u003d threading.Thread(target\u003dself.safe_run)"},{"line_number":104,"context_line":"            self.thread.setDaemon(True)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_12275613","line":101,"in_reply_to":"bab6814e_a7c72e11","updated":"2016-05-18 18:28:37.000000000","message":"Done","commit_id":"60378d7675214056ce85730d391182c7728fb7ac"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"f0e58b4d467a3dcfacab6820953eecdcce952de7","unresolved":false,"context_lines":[{"line_number":98,"context_line":"                    helper.register_table(table_name)"},{"line_number":99,"context_line":"            self.idl \u003d idl.Idl(self.connection, helper)"},{"line_number":100,"context_line":"            idlutils.wait_for_change(self.idl, self.timeout)"},{"line_number":101,"context_line":"            self.poller \u003d poller.Poller()"},{"line_number":102,"context_line":"            self.poller.timer_wait(self.timeout)"},{"line_number":103,"context_line":"            self.thread \u003d threading.Thread(target\u003dself.safe_run)"},{"line_number":104,"context_line":"            self.thread.setDaemon(True)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_a7c72e11","line":101,"in_reply_to":"bab6814e_b82592c3","updated":"2016-05-18 18:08:34.000000000","message":"Thanks, good catch! I should probably also put a comment somewhere that this is just a workaround and block() shouldn\u0027t hang with existing connection.","commit_id":"60378d7675214056ce85730d391182c7728fb7ac"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"066cd71cd2399fcc73f52c26ce5c1ff9747f5af1","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                LOG.exception("},{"line_number":112,"context_line":"                    _LE(\"Service thread got exception: errno: \""},{"line_number":113,"context_line":"                        \"%(errno)d, message %(message)s\"),"},{"line_number":114,"context_line":"                    {\u0027errnor\u0027: e[0],"},{"line_number":115,"context_line":"                     \u0027message\u0027: e[1]})"},{"line_number":116,"context_line":"                self.idl.force_reconnect()"},{"line_number":117,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_88d9e024","line":114,"range":{"start_line":114,"start_character":22,"end_line":114,"end_character":28},"updated":"2016-05-20 18:10:29.000000000","message":"Whoopsie typo","commit_id":"5b96ded90edb1c36356bfe46d29430a98f8e2814"},{"author":{"_account_id":11682,"name":"Ryan Moats","email":"rmoats@us.ibm.com","username":"regXboi"},"change_message_id":"2e6d46ba0445913f6ca361e6e01ae5e9618354d3","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                    _LE(\"Service thread got exception: errno: \""},{"line_number":113,"context_line":"                        \"%(errno)d, message %(message)s\"),"},{"line_number":114,"context_line":"                    {\u0027errnor\u0027: e[0],"},{"line_number":115,"context_line":"                     \u0027message\u0027: e[1]})"},{"line_number":116,"context_line":"                self.idl.force_reconnect()"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def run(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_d24ef56d","line":115,"updated":"2016-05-20 17:08:52.000000000","message":"I\u0027m bothered by logging this at the exception level - iirc, this is at the ERROR level and so will show up in the logs even though we aren\u0027t actually in an error state. I\u0027d much rather it be at a lower level.","commit_id":"5b96ded90edb1c36356bfe46d29430a98f8e2814"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"066cd71cd2399fcc73f52c26ce5c1ff9747f5af1","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                    _LE(\"Service thread got exception: errno: \""},{"line_number":113,"context_line":"                        \"%(errno)d, message %(message)s\"),"},{"line_number":114,"context_line":"                    {\u0027errnor\u0027: e[0],"},{"line_number":115,"context_line":"                     \u0027message\u0027: e[1]})"},{"line_number":116,"context_line":"                self.idl.force_reconnect()"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def run(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_a8e304d8","line":115,"in_reply_to":"bab6814e_d24ef56d","updated":"2016-05-20 18:10:29.000000000","message":"I disagree, if we don\u0027t get fds from select in expected time, it\u0027s an error which is handled later. Also logging errors doesn\u0027t mean your system ends up in error state.","commit_id":"5b96ded90edb1c36356bfe46d29430a98f8e2814"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"f0692ca0439e68b223b9a6e535ce9d1c5f3f88ca","unresolved":false,"context_lines":[{"line_number":108,"context_line":"            try:"},{"line_number":109,"context_line":"                self.run()"},{"line_number":110,"context_line":"            except select.error as e:"},{"line_number":111,"context_line":"                LOG.exception("},{"line_number":112,"context_line":"                    _LE(\"Service thread got exception: errno: \""},{"line_number":113,"context_line":"                        \"%(errno)d, message %(message)s\"),"},{"line_number":114,"context_line":"                    {\u0027errno\u0027: e[0],"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_ec7a2b23","line":111,"range":{"start_line":111,"start_character":20,"end_line":111,"end_character":29},"updated":"2016-05-21 19:42:09.000000000","message":"Note for reviewers: Please see comment in previous PS about not logging this with ERROR level.","commit_id":"c83e98a28fe7515dfe12b99324653e16e34cf689"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"6516ac55b1f35fdd8c7b3e30d3201f50b59a9a3b","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        super(TransactionQueue, self).__init__(*args, **kwargs)"},{"line_number":31,"context_line":"        alertpipe \u003d os.pipe()"},{"line_number":32,"context_line":"        self.alertin \u003d os.fdopen(alertpipe[0], \u0027rb\u0027, 0)"},{"line_number":33,"context_line":"        self.alertout \u003d os.fdopen(alertpipe[1], \u0027wb\u0027, 0)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def get_nowait(self, *args, **kwargs):"},{"line_number":36,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"bab6814e_60dfa23a","line":33,"updated":"2016-05-25 19:19:45.000000000","message":"I don\u0027t know why suddenly py34 wants these streams to be binary.","commit_id":"c8a3907e0d48c4e8eecf0960194cbe3018ddee5a"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"129856a743a8cd4b91296f4505d1dad16886bbcf","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        super(TransactionQueue, self).__init__(*args, **kwargs)"},{"line_number":31,"context_line":"        alertpipe \u003d os.pipe()"},{"line_number":32,"context_line":"        self.alertin \u003d os.fdopen(alertpipe[0], \u0027rb\u0027, 0)"},{"line_number":33,"context_line":"        self.alertout \u003d os.fdopen(alertpipe[1], \u0027wb\u0027, 0)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def get_nowait(self, *args, **kwargs):"},{"line_number":36,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"bab6814e_c66bf4b7","line":33,"in_reply_to":"bab6814e_60dfa23a","updated":"2016-05-26 07:17:22.000000000","message":"There is a patch for that: https://review.openstack.org/#/c/314742/","commit_id":"c8a3907e0d48c4e8eecf0960194cbe3018ddee5a"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"456c0de3f2ca8a87d23db05a70559eadc43e8b99","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        while True:"},{"line_number":105,"context_line":"            self.idl.wait(self.poller)"},{"line_number":106,"context_line":"            self.poller.fd_wait(self.txns.alert_fileno, poller.POLLIN)"},{"line_number":107,"context_line":"            #TODO(jlibosva): Remove next line once losing connection to ovsdb"},{"line_number":108,"context_line":"            #                is solved."},{"line_number":109,"context_line":"            self.poller.timer_wait(self.timeout)"},{"line_number":110,"context_line":"            self.poller.block()"}],"source_content_type":"text/x-python","patch_set":11,"id":"bab6814e_978d7f7e","line":107,"updated":"2016-05-26 14:31:39.000000000","message":"It might be useful to leave this in even in the case that we fix it. Blocking indefinitely is pretty optimistic given we still have human being writing code. Though I do also like things to fail spectacularly when they are wrong, so I could go either way.","commit_id":"576426ab1e83c14861b2c814e7545b9c02a20108"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"b7569fc2d5e035e6ca6403fb19cb02c158484f61","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        while True:"},{"line_number":105,"context_line":"            self.idl.wait(self.poller)"},{"line_number":106,"context_line":"            self.poller.fd_wait(self.txns.alert_fileno, poller.POLLIN)"},{"line_number":107,"context_line":"            #TODO(jlibosva): Remove next line once losing connection to ovsdb"},{"line_number":108,"context_line":"            #                is solved."},{"line_number":109,"context_line":"            self.poller.timer_wait(self.timeout)"},{"line_number":110,"context_line":"            self.poller.block()"}],"source_content_type":"text/x-python","patch_set":11,"id":"bab6814e_ce001105","line":107,"in_reply_to":"bab6814e_978d7f7e","updated":"2016-05-26 15:37:46.000000000","message":"I didn\u0027t find any way how to check what happened underneath. OVS hides whether we timed out or polled successfully, ideally if timeout occurs, we would log that. Maybe we could get our Poller and override the block method.","commit_id":"576426ab1e83c14861b2c814e7545b9c02a20108"}],"neutron/tests/unit/agent/ovsdb/test_impl_idl.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"2a3ed80e5465428706c8ad62eb4125d81a257e81","unresolved":false,"context_lines":[{"line_number":21,"context_line":"    from ovs.db import idl  # noqa"},{"line_number":22,"context_line":"except ImportError:"},{"line_number":23,"context_line":"    raise unittest2.SkipTest("},{"line_number":24,"context_line":"        \"Skip test since ovs requirement for PY3 doesn\u0027t support yet.\")"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"from neutron.agent.ovsdb import api"},{"line_number":27,"context_line":"from neutron.agent.ovsdb import impl_idl"}],"source_content_type":"text/x-python","patch_set":13,"id":"9abb7d3a_db4cf678","line":24,"updated":"2016-05-27 09:34:23.000000000","message":"Not sure it\u0027s needed since now ovs 2.6.0dev1 is in requirements.txt for python3.","commit_id":"c13d722f3913137945c27fcc74371d3316129f30"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"c6cb7de5a0d0d7af0e7471e573ab746cad9a690e","unresolved":false,"context_lines":[{"line_number":21,"context_line":"    from ovs.db import idl  # noqa"},{"line_number":22,"context_line":"except ImportError:"},{"line_number":23,"context_line":"    raise unittest2.SkipTest("},{"line_number":24,"context_line":"        \"Skip test since ovs requirement for PY3 doesn\u0027t support yet.\")"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"from neutron.agent.ovsdb import api"},{"line_number":27,"context_line":"from neutron.agent.ovsdb import impl_idl"}],"source_content_type":"text/x-python","patch_set":13,"id":"9abb7d3a_07462123","line":24,"in_reply_to":"9abb7d3a_db4cf678","updated":"2016-05-27 10:58:45.000000000","message":"Good point, I removed the skips in https://review.openstack.org/322074","commit_id":"c13d722f3913137945c27fcc74371d3316129f30"}]}