)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ad0a0392362ac998c799dea9d3b9500a7b5bd759","unresolved":false,"context_lines":[{"line_number":10,"context_line":"connection:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"1. Register event handling callbacks on the connection itself."},{"line_number":13,"context_line":"2. Call the driver callback to enable/disable the compute service."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"The first is essential to run before the connection is used for"},{"line_number":16,"context_line":"anything. The second needs only to run eventually once the connection"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"5a74a57a_1b375ab1","line":13,"range":{"start_line":13,"start_character":19,"end_line":13,"end_character":27},"updated":"2016-11-23 17:18:07.000000000","message":"\"_set_host_enabled callback\" to be more specific. There are lots of \u0027callback\u0027 being thrown about here so it\u0027s easy to get lost.","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"2b5d18ce30abd82ecf9c711c29a5dff7e4481c0a","unresolved":false,"context_lines":[{"line_number":10,"context_line":"connection:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"1. Register event handling callbacks on the connection itself."},{"line_number":13,"context_line":"2. Call the driver callback to enable/disable the compute service."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"The first is essential to run before the connection is used for"},{"line_number":16,"context_line":"anything. The second needs only to run eventually once the connection"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"5a74a57a_3b4fb693","line":13,"range":{"start_line":13,"start_character":19,"end_line":13,"end_character":27},"in_reply_to":"5a74a57a_1b375ab1","updated":"2016-11-23 17:54:28.000000000","message":"Done","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"}],"nova/tests/unit/virt/libvirt/test_host.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"90ed55afdc3268ec71005b9b8b72db3a2fa6b200","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        h.initialize()"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        h.get_connection()"},{"line_number":370,"context_line":"        event.wait()"},{"line_number":371,"context_line":"        # This test will timeout if it fails. Success is implicit in a"},{"line_number":372,"context_line":"        # timely return from wait(), indicating that the connection event"},{"line_number":373,"context_line":"        # handler was called."}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_f25f3c73","line":370,"updated":"2016-11-07 15:23:21.000000000","message":"We don\u0027t want to wait for a timeout to know that a test is failing, what are you trying to test here which can be done by simply passing a MagicMock and asserting the method has been called ?","commit_id":"31832c0c0cbbe14d66d1a68336c94aa2606e16be"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"7e7235a508d3fdd7bad085d4b6b0b5a596a083bd","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        h.initialize()"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        h.get_connection()"},{"line_number":370,"context_line":"        event.wait()"},{"line_number":371,"context_line":"        # This test will timeout if it fails. Success is implicit in a"},{"line_number":372,"context_line":"        # timely return from wait(), indicating that the connection event"},{"line_number":373,"context_line":"        # handler was called."}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_b855a31e","line":370,"in_reply_to":"9a629dbe_5d5e3946","updated":"2016-11-07 15:58:41.000000000","message":"Arf I wanted to say: \"something not good to do async in an unit test\"","commit_id":"31832c0c0cbbe14d66d1a68336c94aa2606e16be"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"ea3e0c09ba23a64f371dd245803957ef42278ed4","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        h.initialize()"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        h.get_connection()"},{"line_number":370,"context_line":"        event.wait()"},{"line_number":371,"context_line":"        # This test will timeout if it fails. Success is implicit in a"},{"line_number":372,"context_line":"        # timely return from wait(), indicating that the connection event"},{"line_number":373,"context_line":"        # handler was called."}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_5e7c6f20","line":370,"in_reply_to":"9a629dbe_5d5e3946","updated":"2016-11-07 16:27:12.000000000","message":"Ok, I see what you\u0027re saying about mocking utils.spawn. Running it synchronously is obviously more convenient. See the very similar test_conn_event above where I\u0027ve done exactly that. \n\nHowever, the purpose of this test is to ensure that calling initialize() actually results in work being done. Unfortunately we can\u0027t use the synchronous trick here because we\u0027re testing _conn_event_thread, which is an infinite loop. We certainly don\u0027t want to wait for that to complete ;)\n\nI can\u0027t think of a better way to do this, but I\u0027m open to suggestions.","commit_id":"31832c0c0cbbe14d66d1a68336c94aa2606e16be"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5475d6aadaac5a566a91be63bcae1a46783cf8ca","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        h.initialize()"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        h.get_connection()"},{"line_number":370,"context_line":"        event.wait()"},{"line_number":371,"context_line":"        # This test will timeout if it fails. Success is implicit in a"},{"line_number":372,"context_line":"        # timely return from wait(), indicating that the connection event"},{"line_number":373,"context_line":"        # handler was called."}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_d24b1358","line":370,"in_reply_to":"9a629dbe_5e7c6f20","updated":"2016-11-07 16:36:47.000000000","message":"Ok I do see your point know, unfortunately without updating your \u0027while True\u0027 by something else I do not see any other solution so far... I\u0027m not pro of that test but I do not have better solution so let do that :)","commit_id":"31832c0c0cbbe14d66d1a68336c94aa2606e16be"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"65718049b354a747eaa20306be913918b22198db","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        h.initialize()"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        h.get_connection()"},{"line_number":370,"context_line":"        event.wait()"},{"line_number":371,"context_line":"        # This test will timeout if it fails. Success is implicit in a"},{"line_number":372,"context_line":"        # timely return from wait(), indicating that the connection event"},{"line_number":373,"context_line":"        # handler was called."}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_5d5e3946","line":370,"in_reply_to":"9a629dbe_ddd24934","updated":"2016-11-07 15:42:36.000000000","message":"Well I would say you just have to mock utils.spawn and then ensure the method has been called... It\u0027s not something good to do async on unit tests and plus by that way we are going to remove that timeout","commit_id":"31832c0c0cbbe14d66d1a68336c94aa2606e16be"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c1493d36244118f86c826b9951198feeaf23d28d","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        h.initialize()"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        h.get_connection()"},{"line_number":370,"context_line":"        event.wait()"},{"line_number":371,"context_line":"        # This test will timeout if it fails. Success is implicit in a"},{"line_number":372,"context_line":"        # timely return from wait(), indicating that the connection event"},{"line_number":373,"context_line":"        # handler was called."}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_ddfec904","line":370,"in_reply_to":"9a629dbe_f25f3c73","updated":"2016-11-07 15:25:49.000000000","message":"I wanted to say: \u0027which can\u0027t be done...\u0027","commit_id":"31832c0c0cbbe14d66d1a68336c94aa2606e16be"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"65d67bb19104843e9ab34faf04acd9033e6cf4f4","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        h.initialize()"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        h.get_connection()"},{"line_number":370,"context_line":"        event.wait()"},{"line_number":371,"context_line":"        # This test will timeout if it fails. Success is implicit in a"},{"line_number":372,"context_line":"        # timely return from wait(), indicating that the connection event"},{"line_number":373,"context_line":"        # handler was called."}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_ddd24934","line":370,"in_reply_to":"9a629dbe_f25f3c73","updated":"2016-11-07 15:32:34.000000000","message":"That is essentially exactly what we\u0027re doing, but without using a MagicMock. If you think about it, if we passed a MagicMock in instead we\u0027d have to sit in a loop here waiting until it was called. How long to wait? It\u0027s still a Timeout. By using event.send we\u0027ll be notified immediately it\u0027s called and we don\u0027t need to write any additional code.\n\nHopefully the test will almost never be broken, in which case it succeeds almost immediately. In the event that it is broken the timeout is only 60 seconds (IIRC), and it doesn\u0027t consume many resources while it waits. We could potentially wrap this in a shorter timeout, but I\u0027m not convinced it\u0027s worth the additional complexity.","commit_id":"31832c0c0cbbe14d66d1a68336c94aa2606e16be"}],"nova/virt/libvirt/host.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c526e8096eb3d881aaad985920302da502b2b7bb","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def _conn_event_thread(self):"},{"line_number":137,"context_line":"        \"\"\"Dispatches async connection events\"\"\""},{"line_number":138,"context_line":"        # NOTE(mdbooth): This thread doesn\u0027t need to jump through the same"},{"line_number":139,"context_line":"        # hoops as _dispatch_thread because it doesn\u0027t interact directly"},{"line_number":140,"context_line":"        # with the libvirt native thread."},{"line_number":141,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_e3edbd26","line":138,"updated":"2016-11-07 12:02:32.000000000","message":"s/NOTE(mbooth)//","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"363da723e7c7feb9e5d4551675c9f006e9bb5dc3","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def _conn_event_thread(self):"},{"line_number":137,"context_line":"        \"\"\"Dispatches async connection events\"\"\""},{"line_number":138,"context_line":"        # NOTE(mdbooth): This thread doesn\u0027t need to jump through the same"},{"line_number":139,"context_line":"        # hoops as _dispatch_thread because it doesn\u0027t interact directly"},{"line_number":140,"context_line":"        # with the libvirt native thread."},{"line_number":141,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_eca045a8","line":138,"in_reply_to":"9a629dbe_e3edbd26","updated":"2016-11-07 15:01:28.000000000","message":"Meh, mdbooth is my IRC nick, which people will be most familiar with. It\u0027s what I\u0027ve used everywhere else. I think technically I\u0027m supposed to use matthew-booth-9 or something like that. I\u0027m not doing that, though :)","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c526e8096eb3d881aaad985920302da502b2b7bb","unresolved":false,"context_lines":[{"line_number":142,"context_line":"            self._dispatch_conn_event()"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    def _dispatch_conn_event(self):"},{"line_number":145,"context_line":"        # NOTE(mdbooth): Splitting out this loop looks redundant, but it"},{"line_number":146,"context_line":"        # means we can easily dispatch events synchronously from tests and"},{"line_number":147,"context_line":"        # it isn\u0027t completely awful."},{"line_number":148,"context_line":"        handler \u003d self._conn_event_handler_queue.get()"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_03f3910b","line":145,"updated":"2016-11-07 12:02:32.000000000","message":"s/NOTE(mbooth)//","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c526e8096eb3d881aaad985920302da502b2b7bb","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        try:"},{"line_number":150,"context_line":"            handler()"},{"line_number":151,"context_line":"        except Exception:"},{"line_number":152,"context_line":"            LOG.exception(_LE(\u0027Exception handing connection event\u0027))"},{"line_number":153,"context_line":"        finally:"},{"line_number":154,"context_line":"            self._conn_event_handler_queue.task_done()"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_ec592f90","line":152,"updated":"2016-11-07 12:02:32.000000000","message":"s/handing/handling?","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"363da723e7c7feb9e5d4551675c9f006e9bb5dc3","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        try:"},{"line_number":150,"context_line":"            handler()"},{"line_number":151,"context_line":"        except Exception:"},{"line_number":152,"context_line":"            LOG.exception(_LE(\u0027Exception handing connection event\u0027))"},{"line_number":153,"context_line":"        finally:"},{"line_number":154,"context_line":"            self._conn_event_handler_queue.task_done()"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_4c87710f","line":152,"in_reply_to":"9a629dbe_ec592f90","updated":"2016-11-07 15:01:28.000000000","message":"Done.","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c526e8096eb3d881aaad985920302da502b2b7bb","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        with self._wrapped_conn_lock:"},{"line_number":283,"context_line":"            if conn \u003d\u003d self._wrapped_conn:"},{"line_number":284,"context_line":"                reason \u003d str(last_close_event[\u0027reason\u0027])"},{"line_number":285,"context_line":"                msg \u003d _LE(\"Connection to libvirt lost: %s\") % reason"},{"line_number":286,"context_line":"                self._wrapped_conn \u003d None"},{"line_number":287,"context_line":"                self._queue_conn_event_handler(False, msg)"},{"line_number":288,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_4395893e","line":285,"updated":"2016-11-07 12:02:32.000000000","message":"The tag LE is used for the log system. You should not use it here, that message is going to be stored in the database.","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"363da723e7c7feb9e5d4551675c9f006e9bb5dc3","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        with self._wrapped_conn_lock:"},{"line_number":283,"context_line":"            if conn \u003d\u003d self._wrapped_conn:"},{"line_number":284,"context_line":"                reason \u003d str(last_close_event[\u0027reason\u0027])"},{"line_number":285,"context_line":"                msg \u003d _LE(\"Connection to libvirt lost: %s\") % reason"},{"line_number":286,"context_line":"                self._wrapped_conn \u003d None"},{"line_number":287,"context_line":"                self._queue_conn_event_handler(False, msg)"},{"line_number":288,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_2c0cfd93","line":285,"in_reply_to":"9a629dbe_4395893e","updated":"2016-11-07 15:01:28.000000000","message":"Good point. I thought this was ending up in an exception for some reason, but it\u0027s a service disabled reason.","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c526e8096eb3d881aaad985920302da502b2b7bb","unresolved":false,"context_lines":[{"line_number":418,"context_line":"                self._wrapped_conn \u003d None"},{"line_number":419,"context_line":"                # Connection was previously up, and went down"},{"line_number":420,"context_line":"                self._queue_conn_event_handler("},{"line_number":421,"context_line":"                    False, _LE(\u0027Connection to libvirt lost\u0027))"},{"line_number":422,"context_line":""},{"line_number":423,"context_line":"            if self._wrapped_conn is None:"},{"line_number":424,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_e38cdd37","line":421,"updated":"2016-11-07 12:02:32.000000000","message":"You should not use the tag LE here.","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"363da723e7c7feb9e5d4551675c9f006e9bb5dc3","unresolved":false,"context_lines":[{"line_number":418,"context_line":"                self._wrapped_conn \u003d None"},{"line_number":419,"context_line":"                # Connection was previously up, and went down"},{"line_number":420,"context_line":"                self._queue_conn_event_handler("},{"line_number":421,"context_line":"                    False, _LE(\u0027Connection to libvirt lost\u0027))"},{"line_number":422,"context_line":""},{"line_number":423,"context_line":"            if self._wrapped_conn is None:"},{"line_number":424,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_ecc68515","line":421,"in_reply_to":"9a629dbe_e38cdd37","updated":"2016-11-07 15:01:28.000000000","message":"Done","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c526e8096eb3d881aaad985920302da502b2b7bb","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                try:"},{"line_number":425,"context_line":"                    # This will raise if it fails to get a connection"},{"line_number":426,"context_line":"                    self._wrapped_conn \u003d self._get_new_connection()"},{"line_number":427,"context_line":"                finally:"},{"line_number":428,"context_line":"                    # If we previously had a connection and it went down,"},{"line_number":429,"context_line":"                    # then we generated a down event for that above."},{"line_number":430,"context_line":"                    # We also want to generate a down event for an initial"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_e0cbf31c","line":427,"updated":"2016-11-07 12:02:32.000000000","message":"Why did you use finally here ? the process is only when _wrapped_conn is None so when get_new_connection() is raising an exception. It would be more natural to use a simple try..except\n\n  try:\n    self._wrapped_conn \u003d self._get_new_connection()\n  except:\n    with excutils.save_and_reraise_exception():\n      if self._initial_connection:\n        self._initial_connection \u003d False\n        self._queue_conn_event_handler(event_handler(False, ...)","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"363da723e7c7feb9e5d4551675c9f006e9bb5dc3","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                try:"},{"line_number":425,"context_line":"                    # This will raise if it fails to get a connection"},{"line_number":426,"context_line":"                    self._wrapped_conn \u003d self._get_new_connection()"},{"line_number":427,"context_line":"                finally:"},{"line_number":428,"context_line":"                    # If we previously had a connection and it went down,"},{"line_number":429,"context_line":"                    # then we generated a down event for that above."},{"line_number":430,"context_line":"                    # We also want to generate a down event for an initial"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_0f2bab83","line":427,"in_reply_to":"9a629dbe_e0cbf31c","updated":"2016-11-07 15:01:28.000000000","message":"Because we want the code below to run whether it succeeds or not. Note the self._initial_connection guard. If we changed this to an except block then we would only unset _initial_connection if we failed, which isn\u0027t what we want.\n\nHowever, it\u0027s obviously trickier to read that I intended, and if it always unset _initial_connection yours would be easier to read. I\u0027ll update it.","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c526e8096eb3d881aaad985920302da502b2b7bb","unresolved":false,"context_lines":[{"line_number":475,"context_line":"        self._init_events()"},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"        LOG.debug(\"Starting connection event dispatch thread\")"},{"line_number":478,"context_line":"        utils.spawn(self._conn_event_thread)"},{"line_number":479,"context_line":""},{"line_number":480,"context_line":"        self._initialized \u003d True"},{"line_number":481,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_63ac4dee","line":478,"updated":"2016-11-07 12:02:32.000000000","message":"I would have put those 2 lines in a method to keep coherence:\n\n _init_conn_events()","commit_id":"f60ebe708806a6adb5d02c84a2347d2e55761771"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d5b7e82b5dd2d0eeb9fcc7e91af501b353013e0c","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        # NOTE(mdbooth): Splitting out this loop looks redundant, but it"},{"line_number":145,"context_line":"        # means we can easily dispatch events synchronously from tests and"},{"line_number":146,"context_line":"        # it isn\u0027t completely awful."},{"line_number":147,"context_line":"        handler \u003d self._conn_event_handler_queue.get()"},{"line_number":148,"context_line":"        try:"},{"line_number":149,"context_line":"            handler()"},{"line_number":150,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_d8f168cf","line":147,"range":{"start_line":147,"start_character":49,"end_line":147,"end_character":52},"updated":"2016-11-23 17:15:29.000000000","message":"Is it OK to get an Empty exception out of this?\n\nhttps://docs.python.org/2/library/queue.html#Queue.Empty","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6be669743f62ee5204386c2b41e84678658e1960","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        # NOTE(mdbooth): Splitting out this loop looks redundant, but it"},{"line_number":145,"context_line":"        # means we can easily dispatch events synchronously from tests and"},{"line_number":146,"context_line":"        # it isn\u0027t completely awful."},{"line_number":147,"context_line":"        handler \u003d self._conn_event_handler_queue.get()"},{"line_number":148,"context_line":"        try:"},{"line_number":149,"context_line":"            handler()"},{"line_number":150,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_31ffd702","line":147,"range":{"start_line":147,"start_character":49,"end_line":147,"end_character":52},"in_reply_to":"5a74a57a_bbc10683","updated":"2016-11-23 17:58:54.000000000","message":"Ah good point. The docs on get() were a bit confusing for the default there.","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"2b5d18ce30abd82ecf9c711c29a5dff7e4481c0a","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        # NOTE(mdbooth): Splitting out this loop looks redundant, but it"},{"line_number":145,"context_line":"        # means we can easily dispatch events synchronously from tests and"},{"line_number":146,"context_line":"        # it isn\u0027t completely awful."},{"line_number":147,"context_line":"        handler \u003d self._conn_event_handler_queue.get()"},{"line_number":148,"context_line":"        try:"},{"line_number":149,"context_line":"            handler()"},{"line_number":150,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_bbc10683","line":147,"range":{"start_line":147,"start_character":49,"end_line":147,"end_character":52},"in_reply_to":"5a74a57a_d8f168cf","updated":"2016-11-23 17:54:28.000000000","message":"We\u0027ll never get one because get() defaults to blocking.","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d5b7e82b5dd2d0eeb9fcc7e91af501b353013e0c","unresolved":false,"context_lines":[{"line_number":417,"context_line":"                self._wrapped_conn \u003d None"},{"line_number":418,"context_line":"                # Connection was previously up, and went down"},{"line_number":419,"context_line":"                self._queue_conn_event_handler("},{"line_number":420,"context_line":"                    False, _(\u0027Connection to libvirt lost\u0027))"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"            if self._wrapped_conn is None:"},{"line_number":423,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_f8aaa46f","line":420,"range":{"start_line":420,"start_character":20,"end_line":420,"end_character":59},"updated":"2016-11-23 17:15:29.000000000","message":"How do we know the args to conn_event_handler? Nothing is doc\u0027ed in the __init__.  I guess we just know it\u0027s _handle_conn_event from the LibvirtDriver code, and if we\u0027re wrong and something else is passed in that doesn\u0027t handle this arg spec we just raise an Exception and log a traceback on L151.\n\nPlus that\u0027s how it\u0027s used today in _dispatch_events.","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"2b5d18ce30abd82ecf9c711c29a5dff7e4481c0a","unresolved":false,"context_lines":[{"line_number":417,"context_line":"                self._wrapped_conn \u003d None"},{"line_number":418,"context_line":"                # Connection was previously up, and went down"},{"line_number":419,"context_line":"                self._queue_conn_event_handler("},{"line_number":420,"context_line":"                    False, _(\u0027Connection to libvirt lost\u0027))"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"            if self._wrapped_conn is None:"},{"line_number":423,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_db1a4217","line":420,"range":{"start_line":420,"start_character":20,"end_line":420,"end_character":59},"in_reply_to":"5a74a57a_f8aaa46f","updated":"2016-11-23 17:54:28.000000000","message":"Yeah, pretty much. This is a case of over-generalisation imho, because we already assume what it is. This patch just rolls with it.","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d5b7e82b5dd2d0eeb9fcc7e91af501b353013e0c","unresolved":false,"context_lines":[{"line_number":430,"context_line":"                        # We also want to generate a down event for an initial"},{"line_number":431,"context_line":"                        # failure, which won\u0027t be handled above."},{"line_number":432,"context_line":"                        if (self._initial_connection and"},{"line_number":433,"context_line":"                                self._wrapped_conn is None):"},{"line_number":434,"context_line":"                            self._queue_conn_event_handler("},{"line_number":435,"context_line":"                                False, _(\u0027Failed to connect to libvirt\u0027))"},{"line_number":436,"context_line":"                finally:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_7849348b","line":433,"range":{"start_line":433,"start_character":32,"end_line":433,"end_character":58},"updated":"2016-11-23 17:15:29.000000000","message":"Won\u0027t this always be true here?","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"2b5d18ce30abd82ecf9c711c29a5dff7e4481c0a","unresolved":false,"context_lines":[{"line_number":430,"context_line":"                        # We also want to generate a down event for an initial"},{"line_number":431,"context_line":"                        # failure, which won\u0027t be handled above."},{"line_number":432,"context_line":"                        if (self._initial_connection and"},{"line_number":433,"context_line":"                                self._wrapped_conn is None):"},{"line_number":434,"context_line":"                            self._queue_conn_event_handler("},{"line_number":435,"context_line":"                                False, _(\u0027Failed to connect to libvirt\u0027))"},{"line_number":436,"context_line":"                finally:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_fb9c5e6b","line":433,"range":{"start_line":433,"start_character":32,"end_line":433,"end_character":58},"in_reply_to":"5a74a57a_7849348b","updated":"2016-11-23 17:54:28.000000000","message":"Yes. This is a vestige of a previous version of this patch. I can clean this up.","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d5b7e82b5dd2d0eeb9fcc7e91af501b353013e0c","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                        if (self._initial_connection and"},{"line_number":433,"context_line":"                                self._wrapped_conn is None):"},{"line_number":434,"context_line":"                            self._queue_conn_event_handler("},{"line_number":435,"context_line":"                                False, _(\u0027Failed to connect to libvirt\u0027))"},{"line_number":436,"context_line":"                finally:"},{"line_number":437,"context_line":"                    self._initial_connection \u003d False"},{"line_number":438,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_f803840d","line":435,"range":{"start_line":435,"start_character":39,"end_line":435,"end_character":72},"updated":"2016-11-23 17:15:29.000000000","message":"Include the exception like we have in _dispatch_events?","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"2b5d18ce30abd82ecf9c711c29a5dff7e4481c0a","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                        if (self._initial_connection and"},{"line_number":433,"context_line":"                                self._wrapped_conn is None):"},{"line_number":434,"context_line":"                            self._queue_conn_event_handler("},{"line_number":435,"context_line":"                                False, _(\u0027Failed to connect to libvirt\u0027))"},{"line_number":436,"context_line":"                finally:"},{"line_number":437,"context_line":"                    self._initial_connection \u003d False"},{"line_number":438,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_bbff4633","line":435,"range":{"start_line":435,"start_character":39,"end_line":435,"end_character":72},"in_reply_to":"5a74a57a_f803840d","updated":"2016-11-23 17:54:28.000000000","message":"Yep. Can\u0027t remember why I didn\u0027t do this.","commit_id":"fab7fab4b0087fc1867b526a5db160a1926530bc"}]}