)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"9022ca108c45daf71567bbb4bac33bafc6766728","unresolved":true,"context_lines":[{"line_number":1,"context_line":"Parent:     627b14e2 (Add Python3 xena unit tests)"},{"line_number":2,"context_line":"Author:     Rodolfo Alonso Hernandez \u003cralonsoh@redhat.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2021-04-08 13:41:21 +0000"},{"line_number":4,"context_line":"Commit:     Rodolfo Alonso Hernandez \u003cralonsoh@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"acc58172_beb3db97","line":1,"updated":"2021-04-16 20:08:14.000000000","message":"I understand what you are saying here, but it may be a little hard for someone who is not familiar with the change. Can we rephrase so it is easier for the reader? I\u0027m thinking of something like:\n\n\nAdded an active wait in the method \"Backend.lookup\". \n\nWith this change, optional RowEventHandler and timeout parameters can be used to wait and/or retry.\n\nIf a timeout and a RowEventHandler instance listening on the same IDL connection are provided and the \"lookup\" method does not find the requested register, a WaitEvent instance related to the\ntable and register requested is created. Once the wait event returns, the method tries the lookup once again. If the lookup is still unable to find a match, \"RowNotFound\" exception is raised or the default value is returned.","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1f0cf02d542b2859430055ab1f180214b3c2a5af","unresolved":true,"context_lines":[{"line_number":1,"context_line":"Parent:     627b14e2 (Add Python3 xena unit tests)"},{"line_number":2,"context_line":"Author:     Rodolfo Alonso Hernandez \u003cralonsoh@redhat.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2021-04-08 13:41:21 +0000"},{"line_number":4,"context_line":"Commit:     Rodolfo Alonso Hernandez \u003cralonsoh@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"d05ce7ca_dd1c8a63","line":1,"in_reply_to":"acc58172_beb3db97","updated":"2021-04-19 08:22:55.000000000","message":"I\u0027ll add the first description sentence you wrote.","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"37892f50ddd6687ed977e9877dd7bf653d223862","unresolved":true,"context_lines":[{"line_number":10,"context_line":"and a RowEventHandler instance, listening on the same IDL connection,"},{"line_number":11,"context_line":"are passed, if the \"lookup\" method does not find the requested"},{"line_number":12,"context_line":"register, the method creates a WaitEvent instance related to the"},{"line_number":13,"context_line":"table and register requested. Once the wait event returns, the method"},{"line_number":14,"context_line":"tries to retrieve again the register. The second time, if the register"},{"line_number":15,"context_line":"if not found, an \"RowNotFound\" exception is raised or the default"},{"line_number":16,"context_line":"value is returned."},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"358b7711_7594046b","line":14,"range":{"start_line":13,"start_character":30,"end_line":14,"end_character":37},"updated":"2021-04-21 18:14:14.000000000","message":"nit: This isn\u0027t true anymore with using the cached result from the Event, but not a big deal.","commit_id":"63509d68741669a2ed701765b88d7cbf82002ac1"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e87de4c521e368d0a1eb1a0207ed0685cdb209a9","unresolved":true,"context_lines":[{"line_number":10,"context_line":"and a RowEventHandler instance, listening on the same IDL connection,"},{"line_number":11,"context_line":"are passed, if the \"lookup\" method does not find the requested"},{"line_number":12,"context_line":"register, the method creates a WaitEvent instance related to the"},{"line_number":13,"context_line":"table and register requested. Once the wait event returns, the method"},{"line_number":14,"context_line":"tries to retrieve again the register. The second time, if the register"},{"line_number":15,"context_line":"if not found, an \"RowNotFound\" exception is raised or the default"},{"line_number":16,"context_line":"value is returned."},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"226e23b9_4152f6c2","line":14,"range":{"start_line":13,"start_character":30,"end_line":14,"end_character":37},"in_reply_to":"358b7711_7594046b","updated":"2021-04-22 15:22:20.000000000","message":"Right! If I need to respin the patch, I\u0027ll fix it.","commit_id":"63509d68741669a2ed701765b88d7cbf82002ac1"}],"ovsdbapp/backend/ovs_idl/__init__.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"c63f6330326c0b0d1cdd4db3f38c3d6526ef94fe","unresolved":true,"context_lines":[{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        wait_event \u003d LookupWaitEvent(table, record, timeout\u003dtimeout)"},{"line_number":208,"context_line":"        notify_handler.watch_event(wait_event)"},{"line_number":209,"context_line":"        wait_event.wait()"},{"line_number":210,"context_line":"        return _lookup(False)"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def _lookup(self, table, record):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5845774c_0a806eaa","line":209,"updated":"2021-04-13 13:32:49.000000000","message":"I think if wait() returns False, we\u0027d need to unwatch the event. If the event is found, it being a ONETIME event, it\u0027ll get removed. But if it isn\u0027t, I think it might stick around indefinitely.","commit_id":"0fc9ef29dee99358d8d7030273493dabd5233114"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"8b7b9feb9ace000871b772c2fe2bb21a55b1b985","unresolved":true,"context_lines":[{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        wait_event \u003d LookupWaitEvent(table, record, timeout\u003dtimeout)"},{"line_number":208,"context_line":"        notify_handler.watch_event(wait_event)"},{"line_number":209,"context_line":"        wait_event.wait()"},{"line_number":210,"context_line":"        return _lookup(False)"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def _lookup(self, table, record):"}],"source_content_type":"text/x-python","patch_set":2,"id":"370e792c_36a34b38","line":209,"in_reply_to":"5089b1a6_b03a5b0d","updated":"2021-04-15 14:04:48.000000000","message":"That was my point. I think we *will* have an endless list of watched events if we *don\u0027t* unwatch the event if wait() returns false.","commit_id":"0fc9ef29dee99358d8d7030273493dabd5233114"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"dbad5f0c3f8deb3c89acd75497c97ff7c17da22d","unresolved":true,"context_lines":[{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        wait_event \u003d LookupWaitEvent(table, record, timeout\u003dtimeout)"},{"line_number":208,"context_line":"        notify_handler.watch_event(wait_event)"},{"line_number":209,"context_line":"        wait_event.wait()"},{"line_number":210,"context_line":"        return _lookup(False)"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def _lookup(self, table, record):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5089b1a6_b03a5b0d","line":209,"in_reply_to":"5845774c_0a806eaa","updated":"2021-04-15 11:09:51.000000000","message":"Actually I think I should remove it always, regardless of the result. We don\u0027t want to have, in the IDL, an endless list of watched events","commit_id":"0fc9ef29dee99358d8d7030273493dabd5233114"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"8a4cde8f8b4fc1a51d2d9b370131900033fc466e","unresolved":true,"context_lines":[{"line_number":28,"context_line":"class LookupWaitEvent(event.WaitEvent):"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def __init__(self, table, uuid, timeout):"},{"line_number":31,"context_line":"        conditions \u003d ((\u0027_uuid\u0027, \u0027\u003d\u0027, uuid),)"},{"line_number":32,"context_line":"        events \u003d (self.ROW_CREATE,)"},{"line_number":33,"context_line":"        super().__init__(events, table, conditions, timeout\u003dtimeout)"},{"line_number":34,"context_line":"        self.event_name \u003d \u0027LookupWaitEvent_%s\u0027 % table"}],"source_content_type":"text/x-python","patch_set":4,"id":"328fc122_9424873c","line":31,"updated":"2021-04-15 19:49:33.000000000","message":"Lookup looks up by UUID or name. The condition is going to have to be a custom match_fn that does a lookup() call.","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c7eb75e5273b63e3df2fb888fc94ca41eeb5aecf","unresolved":true,"context_lines":[{"line_number":28,"context_line":"class LookupWaitEvent(event.WaitEvent):"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def __init__(self, table, uuid, timeout):"},{"line_number":31,"context_line":"        conditions \u003d ((\u0027_uuid\u0027, \u0027\u003d\u0027, uuid),)"},{"line_number":32,"context_line":"        events \u003d (self.ROW_CREATE,)"},{"line_number":33,"context_line":"        super().__init__(events, table, conditions, timeout\u003dtimeout)"},{"line_number":34,"context_line":"        self.event_name \u003d \u0027LookupWaitEvent_%s\u0027 % table"}],"source_content_type":"text/x-python","patch_set":4,"id":"2429018e_93cf1c47","line":31,"in_reply_to":"328fc122_9424873c","updated":"2021-04-16 08:25:10.000000000","message":"Right, I forgot the name. Thanks!","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"ea51fe88d368cd815bef2af4fb2751fe8899afa9","unresolved":true,"context_lines":[{"line_number":218,"context_line":"            notify_handler.watch_event(wait_event)"},{"line_number":219,"context_line":"            wait_event.wait()"},{"line_number":220,"context_line":"        finally:"},{"line_number":221,"context_line":"            notify_handler.unwatch_event(wait_event)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        return _lookup(False)"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"13a13f9a_ee404d7b","line":221,"updated":"2021-04-15 14:12:31.000000000","message":"If we do this with finally, I think we\u0027ll potentially unwatch the event twice if the event actually fires. WaitEvent is a onetime event, so will remove the event itself when it matches. I think you\u0027d have to do something like:\n\n notify_handler.watch_event(wait_event)\n if not wait_event.wait():\n     notify_handler.unwatch_event(wait_event)\n\nWe use sets/discard() in the implementation, so I don\u0027t think anything bad will happen if we do call it twice. But if we are going to rely on that behavior, we should probably document it so we know not to change that behavior in the future.","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c7eb75e5273b63e3df2fb888fc94ca41eeb5aecf","unresolved":true,"context_lines":[{"line_number":218,"context_line":"            notify_handler.watch_event(wait_event)"},{"line_number":219,"context_line":"            wait_event.wait()"},{"line_number":220,"context_line":"        finally:"},{"line_number":221,"context_line":"            notify_handler.unwatch_event(wait_event)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        return _lookup(False)"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"0a2a6a12_c91c31f2","line":221,"in_reply_to":"13a13f9a_ee404d7b","updated":"2021-04-16 08:25:10.000000000","message":"Ahhhh yes, thanks. I\u0027ll implement this as you suggested.","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"8a4cde8f8b4fc1a51d2d9b370131900033fc466e","unresolved":true,"context_lines":[{"line_number":220,"context_line":"        finally:"},{"line_number":221,"context_line":"            notify_handler.unwatch_event(wait_event)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        return _lookup(False)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    def _lookup(self, table, record):"},{"line_number":226,"context_line":"        if record \u003d\u003d \"\":"}],"source_content_type":"text/x-python","patch_set":4,"id":"23381d2a_2ce6432c","line":223,"updated":"2021-04-15 19:49:33.000000000","message":"Since the event itself actually needs to do a lookup() to know if it matches, we could avoid doing an additional lookup() at the end.\n\nI worked up a PoC of what I think the patch would need to do, but I\u0027ve not tested other than verifying it doesn\u0027t break existing tests and that doing a lookup for something bogus delays for the timeout. It also tries to detect the notify_handler if it lives at the place where it often does by convention. Since we\u0027re changing the API a bit, maybe we can document something like an optional API.notify_handler attribute? https://paste.centos.org/view/cf87e7f0\n\nIt should be noted that the event.match() which is now lookup() is going to be called for every CREATE event on whatever table we register it for, and this could be expensive-ish. lookup() should be O(1) or ~O(log n) since it is a uuid dict lookup or looking up by indexed objects. You\u0027d definitely not want lookup() with timeout set called while importing the initial database or something. :p I\u0027m not really sure whether the WaitEvent strategy or retrying with backoff is the best solution. WaitEvent should be more responsive, there\u0027s a chance for it being more expensive.","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c7eb75e5273b63e3df2fb888fc94ca41eeb5aecf","unresolved":true,"context_lines":[{"line_number":220,"context_line":"        finally:"},{"line_number":221,"context_line":"            notify_handler.unwatch_event(wait_event)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        return _lookup(False)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    def _lookup(self, table, record):"},{"line_number":226,"context_line":"        if record \u003d\u003d \"\":"}],"source_content_type":"text/x-python","patch_set":4,"id":"7fa428ad_888d8d13","line":223,"in_reply_to":"23381d2a_2ce6432c","updated":"2021-04-16 08:25:10.000000000","message":"Thanks for the code. I\u0027ll implement it and I\u0027ll document this API change.\n\nAbout the cost of this method, I\u0027m not sure about what could happen during the initialization. Actually this is something that should handled in the client, something like having a lock set until a full initialization is done.\n\nI think, as you said, a wait event should be more responsive in most of the cases. Actually, this is an exceptional case, when we know a register should be there in the DB and is not in the local cache. In any case, this functionality should be implemented in the client (Neutron, for example) with care and short timeouts (less than 2-3 seconds).","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"f3a1664304b25e1dc5c679739c2679bea3807859","unresolved":true,"context_lines":[{"line_number":216,"context_line":"        if not notify_handler:"},{"line_number":217,"context_line":"            # We assume the variable name used for the RowEventHandler is"},{"line_number":218,"context_line":"            # \"notify_handler\"."},{"line_number":219,"context_line":"            notify_handler \u003d getattr(self, \u0027notify_handler\u0027, None)"},{"line_number":220,"context_line":"        if not isinstance(notify_handler, row_event.RowEventHandler):"},{"line_number":221,"context_line":"            notify_handler \u003d None"},{"line_number":222,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"88e3c379_9a9b589a","line":219,"range":{"start_line":219,"start_character":12,"end_line":219,"end_character":66},"updated":"2021-04-16 09:19:29.000000000","message":"I\u0027m not sure about how to document it. This should came from an IDL connection but this is not a required parameter in ovs.IDL or connection.OvsdbIdl (based on the first one, Neutron clients inherit from it).\n\nI would keep it in this way: provide the handler; if not, try to get the attribute and if that doesn\u0027t work, we won\u0027t use it.","commit_id":"6065ac223b5c2943a1252694bcbe6321098d2302"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"6d0d41b88b3dbad3f2a4fae313d120ea9ebfd456","unresolved":true,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":38,"context_line":"        try:"},{"line_number":39,"context_line":"            # Normally, we would use run() to do things on match, but in this"},{"line_number":40,"context_line":"            # case, that would mean we\u0027d have to run lookup() again.."},{"line_number":41,"context_line":"            return bool(self.backend.lookup(self.table, self.record))"},{"line_number":42,"context_line":"        except idlutils.RowNotFound:"},{"line_number":43,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":6,"id":"f05b93dd_18a2056f","line":40,"range":{"start_line":39,"start_character":0,"end_line":40,"end_character":69},"updated":"2021-04-16 20:42:56.000000000","message":"This comment used to be before a self.result \u003d self.backend.lookup() call that cached the result we found so that we could then get rid of the extra _lookup call in lookup(). Did using the cached value not work and we need to remove this comment?","commit_id":"9c5b98a673109ae799c0f79fa1c79fe21b5ffe75"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1f0cf02d542b2859430055ab1f180214b3c2a5af","unresolved":true,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":38,"context_line":"        try:"},{"line_number":39,"context_line":"            # Normally, we would use run() to do things on match, but in this"},{"line_number":40,"context_line":"            # case, that would mean we\u0027d have to run lookup() again.."},{"line_number":41,"context_line":"            return bool(self.backend.lookup(self.table, self.record))"},{"line_number":42,"context_line":"        except idlutils.RowNotFound:"},{"line_number":43,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":6,"id":"ea819281_426308d5","line":40,"range":{"start_line":39,"start_character":0,"end_line":40,"end_character":69},"in_reply_to":"f05b93dd_18a2056f","updated":"2021-04-19 08:22:55.000000000","message":"Right, I didn\u0027t get the idea the first time I saw it. Let me rewrite it again.","commit_id":"9c5b98a673109ae799c0f79fa1c79fe21b5ffe75"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"9022ca108c45daf71567bbb4bac33bafc6766728","unresolved":true,"context_lines":[{"line_number":225,"context_line":"        if ret !\u003d _NO_REGISTER:"},{"line_number":226,"context_line":"            return ret"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        wait_event \u003d LookupWaitEvent(self, table, record, timeout\u003dtimeout)"},{"line_number":229,"context_line":"        notify_handler.watch_event(wait_event)"},{"line_number":230,"context_line":"        if not wait_event.wait():"},{"line_number":231,"context_line":"            notify_handler.unwatch_event(wait_event)"}],"source_content_type":"text/x-python","patch_set":6,"id":"e941e72b_7e9b1192","line":228,"range":{"start_line":228,"start_character":43,"end_line":228,"end_character":73},"updated":"2021-04-16 20:08:14.000000000","message":"NIT: be consistent as in:\n    table, record, timeout\nor\n    table\u003dtable, record\u003drecord, timeout\u003dtimeout","commit_id":"9c5b98a673109ae799c0f79fa1c79fe21b5ffe75"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1f0cf02d542b2859430055ab1f180214b3c2a5af","unresolved":false,"context_lines":[{"line_number":225,"context_line":"        if ret !\u003d _NO_REGISTER:"},{"line_number":226,"context_line":"            return ret"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        wait_event \u003d LookupWaitEvent(self, table, record, timeout\u003dtimeout)"},{"line_number":229,"context_line":"        notify_handler.watch_event(wait_event)"},{"line_number":230,"context_line":"        if not wait_event.wait():"},{"line_number":231,"context_line":"            notify_handler.unwatch_event(wait_event)"}],"source_content_type":"text/x-python","patch_set":6,"id":"40ae0200_adff0bda","line":228,"range":{"start_line":228,"start_character":43,"end_line":228,"end_character":73},"in_reply_to":"e941e72b_7e9b1192","updated":"2021-04-19 08:22:55.000000000","message":"Done","commit_id":"9c5b98a673109ae799c0f79fa1c79fe21b5ffe75"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"89e341b4e1558e1bf2c0aa60170b41d8f7867bbc","unresolved":true,"context_lines":[{"line_number":207,"context_line":"        again."},{"line_number":208,"context_line":"        \"\"\""},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        def _lookup(return_no_register):"},{"line_number":211,"context_line":"            try:"},{"line_number":212,"context_line":"                with self.ovsdb_connection.lock:"},{"line_number":213,"context_line":"                    return self._lookup(table, record)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa089052_add7d4a7","line":210,"updated":"2021-04-20 17:00:14.000000000","message":"The nested _lookup() function w/ the return_no_register stuff I think makes this more complicated than it needs to be. I tried running the tests with this and everything still passed:\n\n       try:\n            with self.ovsdb_connection.lock:\n                return self._lookup(table, record)\n        except idlutils.RowNotFound:\n            if default is not _NO_DEFAULT:\n                return default\n            if not notify_handler:\n                notify_handler \u003d getattr(self, \u0027notify_handler\u0027, None)\n            if not (timeout and notify_handler):\n                raise\n            wait_event \u003d LookupWaitEvent(self, table, record, timeout)\n            notify_handler.watch_event(wait_event)\n            if not wait_event.wait():\n                notify_handler.unwatch_event(wait_event)\n                raise\n            return wait_event.result\n\nand to me it looks a lot simpler and doesn\u0027t require the NO_REGISTER stuff, but it is possible I\u0027m missing a case. Thoughts?","commit_id":"86aaa93c965ae0da4c8af927b1e406b237eda5de"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"192301a3802c8b027040e788e0e65966575b2500","unresolved":true,"context_lines":[{"line_number":207,"context_line":"        again."},{"line_number":208,"context_line":"        \"\"\""},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        def _lookup(return_no_register):"},{"line_number":211,"context_line":"            try:"},{"line_number":212,"context_line":"                with self.ovsdb_connection.lock:"},{"line_number":213,"context_line":"                    return self._lookup(table, record)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7500f5e8_05680bbc","line":210,"in_reply_to":"fa089052_add7d4a7","updated":"2021-04-21 15:18:06.000000000","message":"I added this nested method when I was using it twice. Now we retrieve the value from the event itself, is not needed. I think your implementation is easier and more clear.","commit_id":"86aaa93c965ae0da4c8af927b1e406b237eda5de"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3c105f1f0c6e23baa03278e9bb980aadc129660b","unresolved":true,"context_lines":[{"line_number":39,"context_line":"            # case, that would mean we\u0027d have to run lookup() again."},{"line_number":40,"context_line":"            with self.backend.ovsdb_connection.lock:"},{"line_number":41,"context_line":"                self.result \u003d self.backend.lookup(self.table, self.record)"},{"line_number":42,"context_line":"            return bool(self.result)"},{"line_number":43,"context_line":"        except idlutils.RowNotFound:"},{"line_number":44,"context_line":"            return False"},{"line_number":45,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"21da498d_3e884088","line":42,"updated":"2021-04-27 12:00:20.000000000","message":"nit: This could be in the \"else\" clause but all good since self.result doesn\u0027t raise RowNotFound","commit_id":"63509d68741669a2ed701765b88d7cbf82002ac1"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3c105f1f0c6e23baa03278e9bb980aadc129660b","unresolved":true,"context_lines":[{"line_number":217,"context_line":"            wait_event \u003d LookupWaitEvent(self, table, record, timeout)"},{"line_number":218,"context_line":"            notify_handler.watch_event(wait_event)"},{"line_number":219,"context_line":"            if not wait_event.wait():"},{"line_number":220,"context_line":"                notify_handler.unwatch_event(wait_event)"},{"line_number":221,"context_line":"                raise"},{"line_number":222,"context_line":"            return wait_event.result"},{"line_number":223,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"6e04c6a9_0ea5c085","line":220,"updated":"2021-04-27 12:00:20.000000000","message":"nit: What do you think about issuing some log message that we timed out here?","commit_id":"63509d68741669a2ed701765b88d7cbf82002ac1"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"af1c91b393035e9ba36880f8ffbd104be1ee2b15","unresolved":true,"context_lines":[{"line_number":217,"context_line":"            wait_event \u003d LookupWaitEvent(self, table, record, timeout)"},{"line_number":218,"context_line":"            notify_handler.watch_event(wait_event)"},{"line_number":219,"context_line":"            if not wait_event.wait():"},{"line_number":220,"context_line":"                notify_handler.unwatch_event(wait_event)"},{"line_number":221,"context_line":"                raise"},{"line_number":222,"context_line":"            return wait_event.result"},{"line_number":223,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"b4856599_f490be7d","line":220,"in_reply_to":"6e04c6a9_0ea5c085","updated":"2021-04-27 16:42:33.000000000","message":"Right, this should be an exceptional condition that worths an info log message.","commit_id":"63509d68741669a2ed701765b88d7cbf82002ac1"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"fc9e5b3deef6f8c0ceb5dab0624cb92baee29fdf","unresolved":true,"context_lines":[{"line_number":218,"context_line":"            notify_handler.watch_event(wait_event)"},{"line_number":219,"context_line":"            if not wait_event.wait():"},{"line_number":220,"context_line":"                LOG.info(\u0027Record %s from table %s was not registered in the \u0027"},{"line_number":221,"context_line":"                         \u0027IDL DB cache after %s seconds\u0027, record, table,"},{"line_number":222,"context_line":"                         timeout)"},{"line_number":223,"context_line":"                notify_handler.unwatch_event(wait_event)"},{"line_number":224,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":12,"id":"9384a42c_45deb9e4","line":221,"range":{"start_line":221,"start_character":46,"end_line":221,"end_character":47},"updated":"2021-04-28 07:05:05.000000000","message":"d ?","commit_id":"713fc8174f618329061246b6a8ac62aa9eb1eac0"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"222393aa3e68842ade4946f3ae3d5b534fdfba0e","unresolved":false,"context_lines":[{"line_number":218,"context_line":"            notify_handler.watch_event(wait_event)"},{"line_number":219,"context_line":"            if not wait_event.wait():"},{"line_number":220,"context_line":"                LOG.info(\u0027Record %s from table %s was not registered in the \u0027"},{"line_number":221,"context_line":"                         \u0027IDL DB cache after %s seconds\u0027, record, table,"},{"line_number":222,"context_line":"                         timeout)"},{"line_number":223,"context_line":"                notify_handler.unwatch_event(wait_event)"},{"line_number":224,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":12,"id":"1bf52be9_4e10e34c","line":221,"range":{"start_line":221,"start_character":46,"end_line":221,"end_character":47},"in_reply_to":"9384a42c_45deb9e4","updated":"2021-04-28 08:51:28.000000000","message":"Done","commit_id":"713fc8174f618329061246b6a8ac62aa9eb1eac0"}],"ovsdbapp/event.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"89e341b4e1558e1bf2c0aa60170b41d8f7867bbc","unresolved":true,"context_lines":[{"line_number":153,"context_line":"        while True:"},{"line_number":154,"context_line":"            try:"},{"line_number":155,"context_line":"                match, event, row, updates \u003d self.notifications.get()"},{"line_number":156,"context_line":"                self.data \u003d (match, event, row, updates)"},{"line_number":157,"context_line":"                if (match, event, row, updates) \u003d\u003d STOP_EVENT:"},{"line_number":158,"context_line":"                    self.notifications.task_done()"},{"line_number":159,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":10,"id":"1fe8200d_8dda4b60","line":156,"updated":"2021-04-20 17:00:14.000000000","message":"Is leaving this in intentional, or leftover debugging?","commit_id":"86aaa93c965ae0da4c8af927b1e406b237eda5de"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"192301a3802c8b027040e788e0e65966575b2500","unresolved":true,"context_lines":[{"line_number":153,"context_line":"        while True:"},{"line_number":154,"context_line":"            try:"},{"line_number":155,"context_line":"                match, event, row, updates \u003d self.notifications.get()"},{"line_number":156,"context_line":"                self.data \u003d (match, event, row, updates)"},{"line_number":157,"context_line":"                if (match, event, row, updates) \u003d\u003d STOP_EVENT:"},{"line_number":158,"context_line":"                    self.notifications.task_done()"},{"line_number":159,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":10,"id":"cff8d938_5a5ff485","line":156,"in_reply_to":"1fe8200d_8dda4b60","updated":"2021-04-21 15:18:06.000000000","message":"oh yes, I forgot that","commit_id":"86aaa93c965ae0da4c8af927b1e406b237eda5de"}],"ovsdbapp/tests/functional/backend/ovs_idl/test_backend.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"b14f074fdb73655ea7e016d111aed9b1f60d5814","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"93f7c848_5703bd27","line":54,"updated":"2021-04-11 15:38:16.000000000","message":"Shouldn\u0027t we call t_create.join() here?","commit_id":"0fc9ef29dee99358d8d7030273493dabd5233114"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"dbad5f0c3f8deb3c89acd75497c97ff7c17da22d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f19d34b5_2727c981","line":54,"in_reply_to":"93f7c848_5703bd27","updated":"2021-04-15 11:09:51.000000000","message":"Yes, we should finish the threads inside the caller method.","commit_id":"0fc9ef29dee99358d8d7030273493dabd5233114"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"8a4cde8f8b4fc1a51d2d9b370131900033fc466e","unresolved":true,"context_lines":[{"line_number":42,"context_line":"    def _test_lookup_exception(self, timeout, notify_handler):"},{"line_number":43,"context_line":"        t_create \u003d threading.Thread(target\u003dself._create_ls, args\u003d())"},{"line_number":44,"context_line":"        t_create.start()"},{"line_number":45,"context_line":"        self.assertRaises(idlutils.RowNotFound, self.api.lookup,"},{"line_number":46,"context_line":"                          \u0027Logical_Switch\u0027, self.lsp_name, timeout\u003dtimeout,"},{"line_number":47,"context_line":"                          notify_handler\u003dnotify_handler)"},{"line_number":48,"context_line":"        t_create.join()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3c539996_92390bf9","line":45,"updated":"2021-04-15 19:49:33.000000000","message":"Since we are starting the thread, isn\u0027t it possible that we will create the switch before getting to the assertRaises?","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1f0cf02d542b2859430055ab1f180214b3c2a5af","unresolved":true,"context_lines":[{"line_number":42,"context_line":"    def _test_lookup_exception(self, timeout, notify_handler):"},{"line_number":43,"context_line":"        t_create \u003d threading.Thread(target\u003dself._create_ls, args\u003d())"},{"line_number":44,"context_line":"        t_create.start()"},{"line_number":45,"context_line":"        self.assertRaises(idlutils.RowNotFound, self.api.lookup,"},{"line_number":46,"context_line":"                          \u0027Logical_Switch\u0027, self.lsp_name, timeout\u003dtimeout,"},{"line_number":47,"context_line":"                          notify_handler\u003dnotify_handler)"},{"line_number":48,"context_line":"        t_create.join()"}],"source_content_type":"text/x-python","patch_set":4,"id":"da1c378e_6f7cdf4b","line":45,"in_reply_to":"3c539996_92390bf9","updated":"2021-04-19 08:22:55.000000000","message":"In last PS I added a 1 second sleep before the LS creation.\n\nBut, most important, those are eventlet threads. That means we can start them but nothing will be executed until we give the GIL. In other words, the main thread will continue running until we execute the self.api.lookup() -\u003e wait(), that will yield the GIL to the next eventlet thread in the list, in this case t_create. This is because green threads are not pre-emptive.","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1a656288b58d40458d909c189aa9ad1cd61d1374","unresolved":true,"context_lines":[{"line_number":42,"context_line":"    def _test_lookup_exception(self, timeout, notify_handler):"},{"line_number":43,"context_line":"        t_create \u003d threading.Thread(target\u003dself._create_ls, args\u003d())"},{"line_number":44,"context_line":"        t_create.start()"},{"line_number":45,"context_line":"        self.assertRaises(idlutils.RowNotFound, self.api.lookup,"},{"line_number":46,"context_line":"                          \u0027Logical_Switch\u0027, self.lsp_name, timeout\u003dtimeout,"},{"line_number":47,"context_line":"                          notify_handler\u003dnotify_handler)"},{"line_number":48,"context_line":"        t_create.join()"}],"source_content_type":"text/x-python","patch_set":4,"id":"00aa3d70_6e5be1b0","line":45,"in_reply_to":"da1c378e_6f7cdf4b","updated":"2021-04-19 11:54:22.000000000","message":"Ok, we are not using eventlet in ovsdbapp testing, so we do use threading instead. That means the time.sleep is needed.\n\nAnyway this is not working... I\u0027ve pushed last PS with some debug lines. I see how the LS creation thread works but I don\u0027t see the RowEventHandler.notify_loop thread running. This is why we don\u0027t read the LS creation event during the event wait. I don\u0027t understand this... I need some help here!","commit_id":"2ef0408e72a6991509070809dcf8016589ad9be3"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"89e341b4e1558e1bf2c0aa60170b41d8f7867bbc","unresolved":true,"context_lines":[{"line_number":30,"context_line":"        self.a \u003d None"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _create_ls(self):"},{"line_number":33,"context_line":"        time.sleep(1)  # Wait a bit to allow a first unsuccessful lookup()."},{"line_number":34,"context_line":"        self.api.db_create(\u0027Logical_Switch\u0027, name\u003dself.lsp_name).execute()"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def test_lookup_with_timeout_and_notify_handler(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"4fd4b72d_625ae2d4","line":33,"updated":"2021-04-20 17:00:14.000000000","message":"Functional tests with sleeps make me uncomfortable, but I don\u0027t have a better suggestion. I\u0027m running the tests with -- --until-fail for a while just verifying stability (which in this case I expect to be stable, just being cautious).","commit_id":"86aaa93c965ae0da4c8af927b1e406b237eda5de"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"192301a3802c8b027040e788e0e65966575b2500","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        self.a \u003d None"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _create_ls(self):"},{"line_number":33,"context_line":"        time.sleep(1)  # Wait a bit to allow a first unsuccessful lookup()."},{"line_number":34,"context_line":"        self.api.db_create(\u0027Logical_Switch\u0027, name\u003dself.lsp_name).execute()"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def test_lookup_with_timeout_and_notify_handler(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"d5915839_8d9cfbd2","line":33,"in_reply_to":"41c03067_202c675f","updated":"2021-04-21 15:18:06.000000000","message":"Because we are not using eventlet, this should not be an issue. I don\u0027t like it either but I don\u0027t find an\u003d batter alternative idea for this test.","commit_id":"86aaa93c965ae0da4c8af927b1e406b237eda5de"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"80f2c428fdd8027a53419c5b7ae7b6082bc81d99","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        self.a \u003d None"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _create_ls(self):"},{"line_number":33,"context_line":"        time.sleep(1)  # Wait a bit to allow a first unsuccessful lookup()."},{"line_number":34,"context_line":"        self.api.db_create(\u0027Logical_Switch\u0027, name\u003dself.lsp_name).execute()"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def test_lookup_with_timeout_and_notify_handler(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"41c03067_202c675f","line":33,"in_reply_to":"4fd4b72d_625ae2d4","updated":"2021-04-20 20:18:16.000000000","message":"Ran the tests for 3.5 hours with no failures, so this looks good to me.","commit_id":"86aaa93c965ae0da4c8af927b1e406b237eda5de"}],"ovsdbapp/venv.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"c63f6330326c0b0d1cdd4db3f38c3d6526ef94fe","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    PATH_VAR_TEMPLATE \u003d \"{0}/ovsdb:{0}/vswitchd:{0}/utilities\""},{"line_number":31,"context_line":"    OVS_PATHS \u003d ("},{"line_number":32,"context_line":"        os.path.join(os.path.sep, \u0027usr\u0027, \u0027local\u0027, \u0027share\u0027, \u0027openvswitch\u0027),"},{"line_number":33,"context_line":"        os.path.join(os.path.sep, \u0027usr\u0027, \u0027share\u0027, \u0027openvswitch\u0027), )"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def __init__(self, venv, ovsdir\u003dNone, dummy\u003dDUMMY_OVERRIDE_ALL,"},{"line_number":36,"context_line":"                 remove\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":2,"id":"74410998_dfe5a1b7","line":33,"updated":"2021-04-13 13:32:49.000000000","message":"unrelated change","commit_id":"0fc9ef29dee99358d8d7030273493dabd5233114"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"dbad5f0c3f8deb3c89acd75497c97ff7c17da22d","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    PATH_VAR_TEMPLATE \u003d \"{0}/ovsdb:{0}/vswitchd:{0}/utilities\""},{"line_number":31,"context_line":"    OVS_PATHS \u003d ("},{"line_number":32,"context_line":"        os.path.join(os.path.sep, \u0027usr\u0027, \u0027local\u0027, \u0027share\u0027, \u0027openvswitch\u0027),"},{"line_number":33,"context_line":"        os.path.join(os.path.sep, \u0027usr\u0027, \u0027share\u0027, \u0027openvswitch\u0027), )"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def __init__(self, venv, ovsdir\u003dNone, dummy\u003dDUMMY_OVERRIDE_ALL,"},{"line_number":36,"context_line":"                 remove\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":2,"id":"d2344065_c3c20f36","line":33,"in_reply_to":"74410998_dfe5a1b7","updated":"2021-04-15 11:09:51.000000000","message":"sorry, that was for testing in my env, a leftover","commit_id":"0fc9ef29dee99358d8d7030273493dabd5233114"}]}