)]}'
{"ovsdbapp/event.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"703e6f95c8f07bf03d872e327149be8b24d67a17","unresolved":true,"context_lines":[{"line_number":75,"context_line":"    def precheck(self):"},{"line_number":76,"context_line":"        \"\"\"Check that the event should be watched"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        :returns: boolean, True if should watch else false"},{"line_number":79,"context_line":"        \"\"\""},{"line_number":80,"context_line":"        return True"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"17ef9642_ca078550","line":78,"range":{"start_line":78,"start_character":27,"end_line":78,"end_character":58},"updated":"2021-01-13 15:54:23.000000000","message":"Seems like it always returns True :) I understand you want to tell if this is subclassed it can be overridden but if you do \"print(event.precheck.__doc__)\" then it prints misleading doc for the given object","commit_id":"03e6d43bb1203429d66df398df9803d55d11099d"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"d2126f82a3457fd439cbfc1ee2ace861ba0da515","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _watch_event(self, event):"},{"line_number":139,"context_line":"        # must be called with lock held"},{"line_number":140,"context_line":"        if event.precheck():"},{"line_number":141,"context_line":"            self.__watched_events.add(event)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def watch_event(self, event):"},{"line_number":144,"context_line":"        with self.__lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_4c07111e","line":141,"range":{"start_line":140,"start_character":0,"end_line":141,"end_character":44},"updated":"2020-07-27 09:13:51.000000000","message":"Would it be possible to add the event only if the precondition is not met in the first place?","commit_id":"03e6d43bb1203429d66df398df9803d55d11099d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"6b97839bc488c54c1604a659652b0feb523a16d6","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _watch_event(self, event):"},{"line_number":139,"context_line":"        # must be called with lock held"},{"line_number":140,"context_line":"        if event.precheck():"},{"line_number":141,"context_line":"            self.__watched_events.add(event)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def watch_event(self, event):"},{"line_number":144,"context_line":"        with self.__lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_53ae80e7","line":141,"range":{"start_line":140,"start_character":0,"end_line":141,"end_character":44},"in_reply_to":"9f560f44_1297f092","updated":"2020-07-27 20:30:55.000000000","message":"Right, (except for the conditional monitoring!) basically some syntactic sugar just to make it easy. The brute force way would just be doing things like:\n\n    event \u003d WaitForSwitch(api, sw, timeout\u003d5)\n    api.event_handler.watch_event(event)\n    event.wait()\n    api.lsp_add(sw, p).execute(check_error\u003dTrue)\n\ncreating the event is cheap, running the event should just be a log n lookup that almost never fail, and if it does then it waits the shortest time possible since we can monitor the event stream.\n\nTransaction objects already have a pre_commit(), but this would essentially be a pre_run() for the Command. It might be good to have the option to disable the checks for things like doing a migration where we know nothing else is writing to the db and we are doing 100s of thousands of operations at once though. There are other optimizations like if you have a transaction that adds multiple ports to a switch, you don\u0027t really want to do the same check multiple times either...so we\u0027d probably want to take that into account. But before we start optimizing, I figured I\u0027d just make it possible to do with this patch. :p","commit_id":"03e6d43bb1203429d66df398df9803d55d11099d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"6b97839bc488c54c1604a659652b0feb523a16d6","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _watch_event(self, event):"},{"line_number":139,"context_line":"        # must be called with lock held"},{"line_number":140,"context_line":"        if event.precheck():"},{"line_number":141,"context_line":"            self.__watched_events.add(event)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def watch_event(self, event):"},{"line_number":144,"context_line":"        with self.__lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_173eb528","line":141,"range":{"start_line":140,"start_character":0,"end_line":141,"end_character":44},"in_reply_to":"9f560f44_4c07111e","updated":"2020-07-27 20:30:55.000000000","message":"That\u0027s what this should do. If event.precheck doesn\u0027t evaluate to True, then that means the event has overriden precheck, meaning that there are preconditions added and also that they have not been met. Maybe I\u0027m misunderstanding the question though.","commit_id":"03e6d43bb1203429d66df398df9803d55d11099d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"22fb74ff10e6285adb619807efadd81f8037f903","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _watch_event(self, event):"},{"line_number":139,"context_line":"        # must be called with lock held"},{"line_number":140,"context_line":"        if event.precheck():"},{"line_number":141,"context_line":"            self.__watched_events.add(event)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def watch_event(self, event):"},{"line_number":144,"context_line":"        with self.__lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_6fe127d1","line":141,"range":{"start_line":140,"start_character":0,"end_line":141,"end_character":44},"in_reply_to":"9f560f44_4c07111e","updated":"2020-07-27 09:51:00.000000000","message":"You mean have precheck() return false by default? Ideally, the precheck() would be the place for avoiding adding the event (line 140) and hopefully that would be returning false in most cases.","commit_id":"03e6d43bb1203429d66df398df9803d55d11099d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"fc98b80d0179a57b34ff480656b8dc3bc9416d69","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _watch_event(self, event):"},{"line_number":139,"context_line":"        # must be called with lock held"},{"line_number":140,"context_line":"        if event.precheck():"},{"line_number":141,"context_line":"            self.__watched_events.add(event)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def watch_event(self, event):"},{"line_number":144,"context_line":"        with self.__lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_d208d829","line":141,"range":{"start_line":140,"start_character":0,"end_line":141,"end_character":44},"in_reply_to":"9f560f44_6fe127d1","updated":"2020-07-27 10:12:50.000000000","message":"Let me ask if I understand correctly: if we don\u0027t have any event, that won\u0027t check anything.\n\nWho will add those checks? This is supposed to be requested by the user, not in any command here in the backend, right?","commit_id":"03e6d43bb1203429d66df398df9803d55d11099d"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"a2962169d74698509d2b0c3099dac4567af33ffe","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _watch_event(self, event):"},{"line_number":139,"context_line":"        # must be called with lock held"},{"line_number":140,"context_line":"        if event.precheck():"},{"line_number":141,"context_line":"            self.__watched_events.add(event)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def watch_event(self, event):"},{"line_number":144,"context_line":"        with self.__lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_b25ba42c","line":141,"range":{"start_line":140,"start_character":0,"end_line":141,"end_character":44},"in_reply_to":"9f560f44_6fe127d1","updated":"2020-07-27 10:16:35.000000000","message":"Not sure about the implementation details. Conditional monitoring is known to be expensive so unless I understand wrongly the purpose of Terry\u0027s approach, it looks like we\u0027d be setting tons of these events (e.g. adding a NAT rule would make sure that the LSP exist, adding a route would make sure that the LR exist,...) and this could have a big penalty. I might be wrong though...\n\nWhat I was trying to suggest with my comment is to locally determine whether the onetime event is needed and avoid adding it to the server if not.","commit_id":"03e6d43bb1203429d66df398df9803d55d11099d"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"af83167335bf3f696fd99355d5dc17f4b6358e8c","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _watch_event(self, event):"},{"line_number":139,"context_line":"        # must be called with lock held"},{"line_number":140,"context_line":"        if event.precheck():"},{"line_number":141,"context_line":"            self.__watched_events.add(event)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def watch_event(self, event):"},{"line_number":144,"context_line":"        with self.__lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_1297f092","line":141,"range":{"start_line":140,"start_character":0,"end_line":141,"end_character":44},"in_reply_to":"9f560f44_b25ba42c","updated":"2020-07-27 10:21:46.000000000","message":"@Rodolfo, IIRC Terry\u0027s approach would be doing something like:\n\n@ovsdb_precondition(\"Logical_Switch\", (\u0027name\u0027, \u0027\u003d\u003d\u0027, \u0027switch\u0027))\nclass LspAddCommand(Command):\n    def __init__(self, switch, port_name, ...)\n        ....\n\nThe same would go for things like NatAddCommand, LrpAddCommand, PortGroupAddCommand, ... creating a lot of conditional monitoring (and hence load) on the server. I could\u0027ve misunderstood it and maybe it\u0027s not like that.","commit_id":"03e6d43bb1203429d66df398df9803d55d11099d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"6b97839bc488c54c1604a659652b0feb523a16d6","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _watch_event(self, event):"},{"line_number":139,"context_line":"        # must be called with lock held"},{"line_number":140,"context_line":"        if event.precheck():"},{"line_number":141,"context_line":"            self.__watched_events.add(event)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def watch_event(self, event):"},{"line_number":144,"context_line":"        with self.__lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_d75b9d84","line":141,"range":{"start_line":140,"start_character":0,"end_line":141,"end_character":44},"in_reply_to":"9f560f44_b25ba42c","updated":"2020-07-27 20:30:55.000000000","message":"Definitely no conditional monitoring. Just listening for the event in our normal event stream.\n\nEssentially, in a multi-server (or even multi-connection) environment, there is absolutely no guarantee that we have received updates that were triggered by other servers. We\u0027ve just been skating by because generally ovsdb-server responds faster than when someone creates two API requests that happen to hit other servers.\n\nEverything in this patch is entirely local. The only overhead would be doing a lookup (similar to if we passed may_exist\u003dTrue) and with indexing this should be very cheap.\n\nIt is one of those edge cases where you might only hit it very rarely unless a system is really overloaded, but for the code to be correct you really do need to handle the case.","commit_id":"03e6d43bb1203429d66df398df9803d55d11099d"}]}