)]}'
{"networking_ovn/common/maintenance.py":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8d2680ad457a2133a08ca06899c0fd7007b5fa22","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"DB_CONSISTENCY_CHECK_INTERVAL \u003d 300  # 5 minutes"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"class MaintenanceWorker(worker.BaseWorker):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_f71ef40a","line":30,"range":{"start_line":30,"start_character":0,"end_line":30,"end_character":35},"updated":"2017-12-22 13:11:04.000000000","message":"I\u0027m not a big fan of adding config options and this is probably good AS IS. However, there\u0027s should be a way that we can monitor how frequent the inconsistencies are vs the deployment size. It\u0027s nice to start off with 5 minutes but I\u0027d like to test this \u0027at scale\u0027 to see how long a maintenance task would take in a \u0027large\u0027 cloud because it may suffice with doing it once a day...","commit_id":"2bf318dbfbc3c4288fda9dd2338ab0e6dc5fc9d5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8883c8fc0cbcb89fa957488961e7fe7810d3d185","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"DB_CONSISTENCY_CHECK_INTERVAL \u003d 300  # 5 minutes"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"class MaintenanceWorker(worker.BaseWorker):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_e8c29585","line":30,"range":{"start_line":30,"start_character":0,"end_line":30,"end_character":35},"in_reply_to":"bf8cb3f7_1d893922","updated":"2017-12-22 16:08:50.000000000","message":"Me and daniel talked over IRC and decided to leave it as-is until we have more data. Adding a configuration option is simple but, removing one is not (deprecation time and so on). So we will wait a bit.","commit_id":"2bf318dbfbc3c4288fda9dd2338ab0e6dc5fc9d5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f68c164efe4d9c83a5770061ebc56b4a039763db","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"DB_CONSISTENCY_CHECK_INTERVAL \u003d 300  # 5 minutes"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"class MaintenanceWorker(worker.BaseWorker):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_1d893922","line":30,"range":{"start_line":30,"start_character":0,"end_line":30,"end_character":35},"in_reply_to":"bf8cb3f7_f71ef40a","updated":"2017-12-22 15:23:07.000000000","message":"Hi there,\n\nYeah I don\u0027t have much data about it to actually determine what would be a ideal interval here. As you said, I can create a configuration option and allow people to easily modify it.\n\nBut as for the default value right now, I\u0027m not sure what the ideal would be. I figured that 5 minutes would be fine because it\u0027s basically a database query (join) each 5 minutes, which should be fine.","commit_id":"2bf318dbfbc3c4288fda9dd2338ab0e6dc5fc9d5"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8d2680ad457a2133a08ca06899c0fd7007b5fa22","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        return not self._idl.is_lock_contended"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def _fix_create_update_network(self, row):"},{"line_number":97,"context_line":"        # Get the lasted version of the networking in Neutron DB"},{"line_number":98,"context_line":"        admin_context \u003d n_context.get_admin_context()"},{"line_number":99,"context_line":"        n_db_obj \u003d self._ovn_client._plugin.get_network("},{"line_number":100,"context_line":"            admin_context, row.resource_uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_971be819","line":97,"range":{"start_line":97,"start_character":18,"end_line":97,"end_character":24},"updated":"2017-12-22 13:11:04.000000000","message":"nit: latest","commit_id":"2bf318dbfbc3c4288fda9dd2338ab0e6dc5fc9d5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f68c164efe4d9c83a5770061ebc56b4a039763db","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        return not self._idl.is_lock_contended"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def _fix_create_update_network(self, row):"},{"line_number":97,"context_line":"        # Get the lasted version of the networking in Neutron DB"},{"line_number":98,"context_line":"        admin_context \u003d n_context.get_admin_context()"},{"line_number":99,"context_line":"        n_db_obj \u003d self._ovn_client._plugin.get_network("},{"line_number":100,"context_line":"            admin_context, row.resource_uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_771f446c","line":97,"range":{"start_line":97,"start_character":18,"end_line":97,"end_character":24},"in_reply_to":"bf8cb3f7_971be819","updated":"2017-12-22 15:23:07.000000000","message":"done","commit_id":"2bf318dbfbc3c4288fda9dd2338ab0e6dc5fc9d5"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"38d825cde25a1945874e4ee8dff47722707bb271","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        self._ovn_client \u003d ovn_client"},{"line_number":86,"context_line":"        # FIXME(lucasagomes): We should not be accessing private"},{"line_number":87,"context_line":"        # attributes like that, perhaps we should extend the OVNClient"},{"line_number":88,"context_line":"        # class and create an interface for the locks ?"},{"line_number":89,"context_line":"        self._idl \u003d self._ovn_client._nb_idl.idl"},{"line_number":90,"context_line":"        self._idl.set_lock(\u0027ovn_db_inconsistencies_periodics\u0027)"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_3af51584","line":88,"updated":"2018-01-04 11:48:39.000000000","message":"+1. Agree its good to have a clean interface","commit_id":"7db223b38e4f2b04c24470dc06e77448d437a47f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"7a190d88e9de710218e07f81d4d3ccd907669c18","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        self._ovn_client \u003d ovn_client"},{"line_number":86,"context_line":"        # FIXME(lucasagomes): We should not be accessing private"},{"line_number":87,"context_line":"        # attributes like that, perhaps we should extend the OVNClient"},{"line_number":88,"context_line":"        # class and create an interface for the locks ?"},{"line_number":89,"context_line":"        self._idl \u003d self._ovn_client._nb_idl.idl"},{"line_number":90,"context_line":"        self._idl.set_lock(\u0027ovn_db_inconsistencies_periodics\u0027)"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_03f53c83","line":88,"in_reply_to":"9f91af0f_3af51584","updated":"2018-01-04 11:58:15.000000000","message":"Yeah, on top of it it would be nice to extend the OVS python IDL to support multiple locks per IDL. Currently, it\u0027s only possible to manage 1 lock per IDL in the python modules (it\u0027s supporeted in OVSDB but just not exposed via the python IDL class)","commit_id":"7db223b38e4f2b04c24470dc06e77448d437a47f"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"38d825cde25a1945874e4ee8dff47722707bb271","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    def _fix_create_update_network(self, row):"},{"line_number":97,"context_line":"        # Get the latest version of the networking in Neutron DB"},{"line_number":98,"context_line":"        admin_context \u003d n_context.get_admin_context()"},{"line_number":99,"context_line":"        n_db_obj \u003d self._ovn_client._plugin.get_network("},{"line_number":100,"context_line":"            admin_context, row.resource_uuid)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        if row.revision_number \u003d\u003d ovn_const.INITIAL_REV_NUM:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_ba65a547","line":99,"updated":"2018-01-04 11:48:39.000000000","message":"Same here for _plugin. May be a function in ovn client to get the plugin object.","commit_id":"7db223b38e4f2b04c24470dc06e77448d437a47f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"fc9039e6955e82e03ee015bf6b026542ce1bcdb1","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    def _fix_create_update_network(self, row):"},{"line_number":97,"context_line":"        # Get the latest version of the networking in Neutron DB"},{"line_number":98,"context_line":"        admin_context \u003d n_context.get_admin_context()"},{"line_number":99,"context_line":"        n_db_obj \u003d self._ovn_client._plugin.get_network("},{"line_number":100,"context_line":"            admin_context, row.resource_uuid)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        if row.revision_number \u003d\u003d ovn_const.INITIAL_REV_NUM:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_03c77c47","line":99,"in_reply_to":"9f91af0f_ba65a547","updated":"2018-01-04 11:56:24.000000000","message":"++ agreed,\n\nWe do have methods to create/update/delete resources in OVNClient, but usually the get_*() methods only live in the impl_idl_ovn.py module. Would be nice to refactor it a bit and expose it via OVNClient.","commit_id":"7db223b38e4f2b04c24470dc06e77448d437a47f"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cc3da3b3059592499c7e8f8f7309937541113560","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        self._ovn_client \u003d ovn_client"},{"line_number":86,"context_line":"        # FIXME(lucasagomes): We should not be accessing private"},{"line_number":87,"context_line":"        # attributes like that, perhaps we should extend the OVNClient"},{"line_number":88,"context_line":"        # class and create an interface for the locks ?"},{"line_number":89,"context_line":"        self._idl \u003d self._ovn_client._nb_idl.idl"},{"line_number":90,"context_line":"        self._idl.set_lock(\u0027ovn_db_inconsistencies_periodics\u0027)"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_eb2258a4","line":88,"updated":"2018-01-05 10:33:51.000000000","message":"+1 on the future fix-me approach :)","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"14996938fcef2040f3e0dfc95bcaba2a3e1dcf8d","unresolved":false,"context_lines":[{"line_number":100,"context_line":"            admin_context, row.resource_uuid)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        if row.revision_number \u003d\u003d ovn_const.INITIAL_REV_NUM:"},{"line_number":103,"context_line":"            self._ovn_client.create_network(n_db_obj)"},{"line_number":104,"context_line":"        else:"},{"line_number":105,"context_line":"            self._ovn_client.update_network(n_db_obj)"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_bc1d7c10","line":103,"range":{"start_line":103,"start_character":29,"end_line":103,"end_character":43},"updated":"2018-01-05 05:16:13.000000000","message":"doesn\u0027t this need to be idempotent? (may_exist\u003dTrue?)\neg. the previous attempt might have failed between nb txn and bump_revision.","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cc3da3b3059592499c7e8f8f7309937541113560","unresolved":false,"context_lines":[{"line_number":100,"context_line":"            admin_context, row.resource_uuid)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        if row.revision_number \u003d\u003d ovn_const.INITIAL_REV_NUM:"},{"line_number":103,"context_line":"            self._ovn_client.create_network(n_db_obj)"},{"line_number":104,"context_line":"        else:"},{"line_number":105,"context_line":"            self._ovn_client.update_network(n_db_obj)"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_1018097f","line":103,"range":{"start_line":103,"start_character":29,"end_line":103,"end_character":43},"in_reply_to":"9f91af0f_bc1d7c10","updated":"2018-01-05 10:33:51.000000000","message":"+1 :)","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"25549a5adff03875a83a0ca19d6d30d1d488339b","unresolved":false,"context_lines":[{"line_number":100,"context_line":"            admin_context, row.resource_uuid)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        if row.revision_number \u003d\u003d ovn_const.INITIAL_REV_NUM:"},{"line_number":103,"context_line":"            self._ovn_client.create_network(n_db_obj)"},{"line_number":104,"context_line":"        else:"},{"line_number":105,"context_line":"            self._ovn_client.update_network(n_db_obj)"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_8b8dcc7c","line":103,"range":{"start_line":103,"start_character":29,"end_line":103,"end_character":43},"in_reply_to":"9f91af0f_bc1d7c10","updated":"2018-01-05 09:57:31.000000000","message":"Oh that\u0027s a good point, yeah I thought may_exist would be the default (it\u0027s not [0]).\n\nPerhaps I could have something more explicit and actually check if the network is already present in OVNDB.\n\nI will correct it! Thanks for pointing it out\n\n[0] https://github.com/openstack/ovsdbapp/blob/d7e3313b2c6b9b7ee03dd6606dedc9dc4b3cb7dc/ovsdbapp/schema/ovn_northbound/impl_idl.py#L29","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cc3da3b3059592499c7e8f8f7309937541113560","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        else:"},{"line_number":105,"context_line":"            self._ovn_client.update_network(n_db_obj)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @periodics.periodic(spacing\u003dDB_CONSISTENCY_CHECK_INTERVAL,"},{"line_number":108,"context_line":"                        run_immediately\u003dTrue)"},{"line_number":109,"context_line":"    def check_for_inconsistencies(self):"},{"line_number":110,"context_line":"        # Only the worker holding a valid lock within OVSDB will run"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_cbc75445","line":107,"range":{"start_line":107,"start_character":32,"end_line":107,"end_character":61},"updated":"2018-01-05 10:33:51.000000000","message":"I know config knobs suck a bit, but, may be it\u0027s worth adding this one. (on a later follow up may be fine)","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"05072c35c7f90a0a87d99a9df798fb94c216d540","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        else:"},{"line_number":105,"context_line":"            self._ovn_client.update_network(n_db_obj)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @periodics.periodic(spacing\u003dDB_CONSISTENCY_CHECK_INTERVAL,"},{"line_number":108,"context_line":"                        run_immediately\u003dTrue)"},{"line_number":109,"context_line":"    def check_for_inconsistencies(self):"},{"line_number":110,"context_line":"        # Only the worker holding a valid lock within OVSDB will run"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_268b971f","line":107,"range":{"start_line":107,"start_character":32,"end_line":107,"end_character":61},"in_reply_to":"9f91af0f_cbc75445","updated":"2018-01-05 11:08:20.000000000","message":"Yeah we discussed this already earlier in the review. It\u0027s easier to add it later when we have come up with a reasonable number through tests.","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cc3da3b3059592499c7e8f8f7309937541113560","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            self._ovn_client.update_network(n_db_obj)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @periodics.periodic(spacing\u003dDB_CONSISTENCY_CHECK_INTERVAL,"},{"line_number":108,"context_line":"                        run_immediately\u003dTrue)"},{"line_number":109,"context_line":"    def check_for_inconsistencies(self):"},{"line_number":110,"context_line":"        # Only the worker holding a valid lock within OVSDB will run"},{"line_number":111,"context_line":"        # this periodic"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_cb68b422","line":108,"updated":"2018-01-05 10:33:51.000000000","message":"just as an idea for future improvement, It\u0027d be great if we had a way to trigger an inconsistency check on conditions we know could have lead to an inconsistency.","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"fbc5d5cd28972884a3515b5b873f50072046ecc5","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            self._ovn_client.update_network(n_db_obj)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @periodics.periodic(spacing\u003dDB_CONSISTENCY_CHECK_INTERVAL,"},{"line_number":108,"context_line":"                        run_immediately\u003dTrue)"},{"line_number":109,"context_line":"    def check_for_inconsistencies(self):"},{"line_number":110,"context_line":"        # Only the worker holding a valid lock within OVSDB will run"},{"line_number":111,"context_line":"        # this periodic"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_f0a1bc47","line":108,"in_reply_to":"9f91af0f_069d3358","updated":"2018-01-08 13:16:20.000000000","message":"Yeah, I was thinking about something like that :)","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"05072c35c7f90a0a87d99a9df798fb94c216d540","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            self._ovn_client.update_network(n_db_obj)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @periodics.periodic(spacing\u003dDB_CONSISTENCY_CHECK_INTERVAL,"},{"line_number":108,"context_line":"                        run_immediately\u003dTrue)"},{"line_number":109,"context_line":"    def check_for_inconsistencies(self):"},{"line_number":110,"context_line":"        # Only the worker holding a valid lock within OVSDB will run"},{"line_number":111,"context_line":"        # this periodic"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_069d3358","line":108,"in_reply_to":"9f91af0f_cb68b422","updated":"2018-01-05 11:08:20.000000000","message":"Good idea, not sure how can we do this though. Imagine a CRUD operation failed (it was retried and so), so checking the consistency right away won\u0027t be much helpful no? Maybe shorten the time till the next recheck?","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cc3da3b3059592499c7e8f8f7309937541113560","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        for row in delete_inconsistencies:"},{"line_number":134,"context_line":"            try:"},{"line_number":135,"context_line":"                if row.resource_type \u003d\u003d ovn_const.TYPE_NETWORKS:"},{"line_number":136,"context_line":"                    self._ovn_client.delete_network(row.resource_uuid)"},{"line_number":137,"context_line":"            except Exception:"},{"line_number":138,"context_line":"                LOG.exception(\u0027Failed to fix deleted resource %(res_uuid)s \u0027"},{"line_number":139,"context_line":"                              \u0027(type: %(res_type)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_2b328011","line":136,"updated":"2018-01-05 10:33:51.000000000","message":"important one: shouldn\u0027t we cleanup the \"deleted_inconsistencies\" entries from DB? so we don\u0027t keep piling them up?","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d9872939f428d03c565273dd3caa918f5a0a3060","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        for row in delete_inconsistencies:"},{"line_number":134,"context_line":"            try:"},{"line_number":135,"context_line":"                if row.resource_type \u003d\u003d ovn_const.TYPE_NETWORKS:"},{"line_number":136,"context_line":"                    self._ovn_client.delete_network(row.resource_uuid)"},{"line_number":137,"context_line":"            except Exception:"},{"line_number":138,"context_line":"                LOG.exception(\u0027Failed to fix deleted resource %(res_uuid)s \u0027"},{"line_number":139,"context_line":"                              \u0027(type: %(res_type)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_e65ccf7f","line":136,"in_reply_to":"9f91af0f_2b328011","updated":"2018-01-05 11:17:15.000000000","message":"Yeah, the delete_network() call should already clean that up for us [0]\n\n[0] https://github.com/openstack/networking-ovn/blob/0d37e8bc9804072abed6253a50c32bf119d5a9af/networking_ovn/common/ovn_client.py#L979","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"fbc5d5cd28972884a3515b5b873f50072046ecc5","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        if not ovn_net:"},{"line_number":107,"context_line":"            # If the resource doesn\u0027t exist in the OVN DB, create it."},{"line_number":108,"context_line":"            self._ovn_client.create_network(n_db_obj)"},{"line_number":109,"context_line":"        else:"},{"line_number":110,"context_line":"            ext_ids \u003d getattr(ovn_net, \u0027external_ids\u0027, {})"},{"line_number":111,"context_line":"            ovn_revision \u003d int(ext_ids.get("}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_10330016","line":108,"updated":"2018-01-08 13:16:20.000000000","message":"Hey, wouldn\u0027t we still need some sort of  \"may_exist\"?\n\nFor example: \n\nhttps://github.com/openstack/networking-ovn/blob/0d37e8bc9804072abed6253a50c32bf119d5a9af/networking_ovn/common/ovn_client.py#L954\n\nWhat happens if the the LS got to be created, but not the provnet_port, or not the metadata_port ? \n\nOn next fix attempt, the create_network will fail on the first item, and exit on exception, so provnet-port / metadata-port won\u0027t be ever fixed.\n\nMay be I\u0027m missing something.\n\nI can work on that as a follow up to this patch if you want/and it makes sense.","commit_id":"cea18015408e52d93f056aeeb8b5aecd4b475e90"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"acbec31bbeecf53d4a7f4307cd65d5a8a55c0bc0","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        if not ovn_net:"},{"line_number":107,"context_line":"            # If the resource doesn\u0027t exist in the OVN DB, create it."},{"line_number":108,"context_line":"            self._ovn_client.create_network(n_db_obj)"},{"line_number":109,"context_line":"        else:"},{"line_number":110,"context_line":"            ext_ids \u003d getattr(ovn_net, \u0027external_ids\u0027, {})"},{"line_number":111,"context_line":"            ovn_revision \u003d int(ext_ids.get("}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_db6db9cc","line":108,"in_reply_to":"9f91af0f_10330016","updated":"2018-01-08 13:24:21.000000000","message":"Good point.\n\nThe provnet-port shouldn\u0027t be a problem because both the logical switch and the provnet-port are going to be created within the same transaction, but, the metadata port may be a concern here.\n\nPerhaps we can add a may_exist parameter to it and move the \"bump_revision()\" call [0] to be invoked after the create_metadata_port() method. That should cover it, what you think ?\n\n[0] https://github.com/openstack/networking-ovn/blob/0d37e8bc9804072abed6253a50c32bf119d5a9af/networking_ovn/common/ovn_client.py#L966-L967","commit_id":"cea18015408e52d93f056aeeb8b5aecd4b475e90"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"b15046bd7daf2b8f508cf46c38a64a9047fc327d","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        if not ovn_net:"},{"line_number":107,"context_line":"            # If the resource doesn\u0027t exist in the OVN DB, create it."},{"line_number":108,"context_line":"            self._ovn_client.create_network(n_db_obj)"},{"line_number":109,"context_line":"        else:"},{"line_number":110,"context_line":"            ext_ids \u003d getattr(ovn_net, \u0027external_ids\u0027, {})"},{"line_number":111,"context_line":"            ovn_revision \u003d int(ext_ids.get("}],"source_content_type":"text/x-python","patch_set":15,"id":"9f91af0f_b6ca4c4b","line":108,"in_reply_to":"9f91af0f_db6db9cc","updated":"2018-01-08 14:13:33.000000000","message":"Btw, was thinking out of loud here... Should may_exist be a parameter for the create_*() methods or perhaps we should just set it by default in OVNClient ?\n\nNot sure I can think about any drawbacks of having may_exist\u003dTrue set by default.","commit_id":"cea18015408e52d93f056aeeb8b5aecd4b475e90"}],"networking_ovn/db/maintenance.py":[{"author":{"_account_id":23458,"name":"Dong Jun","email":"dongjun.01@bytedance.com","username":"Dongj"},"change_message_id":"58da30dc212ebd8793ab14695f8567870365aef3","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    session \u003d db_api.get_reader_session()"},{"line_number":29,"context_line":"    with session.begin():"},{"line_number":30,"context_line":"        return (session.query(models.OVNRevisionNumbers)."},{"line_number":31,"context_line":"                outerjoin("},{"line_number":32,"context_line":"                    standard_attr.StandardAttribute,"},{"line_number":33,"context_line":"                    models.OVNRevisionNumbers.standard_attr_id \u003d\u003d"},{"line_number":34,"context_line":"                    standard_attr.StandardAttribute.id)."}],"source_content_type":"text/x-python","patch_set":7,"id":"bf8cb3f7_514c909d","line":31,"range":{"start_line":31,"start_character":16,"end_line":31,"end_character":25},"updated":"2017-12-22 11:07:44.000000000","message":"I am not sure whether join is better than outerjoin, but I think getting intersection of two tables is enough, It\u0027s not need to get total OVNRevisionNumbers rows by outerjoin.","commit_id":"404bd8812c452728c84565388418a1b92e13a43f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"95046bb2d02aed409245c89b1e186fbfe553905f","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    session \u003d db_api.get_reader_session()"},{"line_number":29,"context_line":"    with session.begin():"},{"line_number":30,"context_line":"        return (session.query(models.OVNRevisionNumbers)."},{"line_number":31,"context_line":"                outerjoin("},{"line_number":32,"context_line":"                    standard_attr.StandardAttribute,"},{"line_number":33,"context_line":"                    models.OVNRevisionNumbers.standard_attr_id \u003d\u003d"},{"line_number":34,"context_line":"                    standard_attr.StandardAttribute.id)."}],"source_content_type":"text/x-python","patch_set":7,"id":"bf8cb3f7_9421a6fe","line":31,"range":{"start_line":31,"start_character":16,"end_line":31,"end_character":25},"in_reply_to":"bf8cb3f7_514c909d","updated":"2017-12-22 11:59:17.000000000","message":"Oh that\u0027s right, an inner join should be enough here.","commit_id":"404bd8812c452728c84565388418a1b92e13a43f"}],"networking_ovn/ml2/mech_driver.py":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8d2680ad457a2133a08ca06899c0fd7007b5fa22","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                maintenance.DBInconsistenciesPeriodics(self._ovn_client))"},{"line_number":196,"context_line":"            self._maintenance_thread.start()"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        self._post_fork_event.set()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def _create_security_group(self, resource, event, trigger,"},{"line_number":201,"context_line":"                               security_group, **kwargs):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_f7911454","line":198,"range":{"start_line":198,"start_character":8,"end_line":198,"end_character":35},"updated":"2017-12-22 13:11:04.000000000","message":"We\u0027re setting this event in L171 already. I think that after L169 we can safely use the IDL connections already.","commit_id":"2bf318dbfbc3c4288fda9dd2338ab0e6dc5fc9d5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f68c164efe4d9c83a5770061ebc56b4a039763db","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                maintenance.DBInconsistenciesPeriodics(self._ovn_client))"},{"line_number":196,"context_line":"            self._maintenance_thread.start()"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        self._post_fork_event.set()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def _create_security_group(self, resource, event, trigger,"},{"line_number":201,"context_line":"                               security_group, **kwargs):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_972e88a3","line":198,"range":{"start_line":198,"start_character":8,"end_line":198,"end_character":35},"in_reply_to":"bf8cb3f7_f7911454","updated":"2017-12-22 15:23:07.000000000","message":"Oh probably this is a rebase leftover","commit_id":"2bf318dbfbc3c4288fda9dd2338ab0e6dc5fc9d5"}],"networking_ovn/tests/unit/db/test_maintenance.py":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8d2680ad457a2133a08ca06899c0fd7007b5fa22","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        self.session \u003d db_api.get_writer_session()"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def test_get_inconsistent_resources(self):"},{"line_number":34,"context_line":"        # Set the intial reviewion to -1 to force it to be incosistent"},{"line_number":35,"context_line":"        db_rev.create_initial_revision("},{"line_number":36,"context_line":"            self.net[\u0027id\u0027], constants.TYPE_NETWORKS, self.session,"},{"line_number":37,"context_line":"            revision_number\u003d-1)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_1784b80b","line":34,"range":{"start_line":34,"start_character":25,"end_line":34,"end_character":34},"updated":"2017-12-22 13:11:04.000000000","message":"snit: revision","commit_id":"2bf318dbfbc3c4288fda9dd2338ab0e6dc5fc9d5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f68c164efe4d9c83a5770061ebc56b4a039763db","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        self.session \u003d db_api.get_writer_session()"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def test_get_inconsistent_resources(self):"},{"line_number":34,"context_line":"        # Set the intial reviewion to -1 to force it to be incosistent"},{"line_number":35,"context_line":"        db_rev.create_initial_revision("},{"line_number":36,"context_line":"            self.net[\u0027id\u0027], constants.TYPE_NETWORKS, self.session,"},{"line_number":37,"context_line":"            revision_number\u003d-1)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_571a407d","line":34,"range":{"start_line":34,"start_character":25,"end_line":34,"end_character":34},"in_reply_to":"bf8cb3f7_1784b80b","updated":"2017-12-22 15:23:07.000000000","message":"done","commit_id":"2bf318dbfbc3c4288fda9dd2338ab0e6dc5fc9d5"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"14996938fcef2040f3e0dfc95bcaba2a3e1dcf8d","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from networking_ovn.tests.unit.db import base as db_base"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class TestMaintenance(db_base.DBTestCase, test_plugin.Ml2PluginV2TestCase):"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def setUp(self):"},{"line_number":28,"context_line":"        super(TestMaintenance, self).setUp()"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_3ccbac1e","line":25,"range":{"start_line":25,"start_character":6,"end_line":25,"end_character":21},"updated":"2018-01-05 05:16:13.000000000","message":"doesn\u0027t this need RevisionPlugin?","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"589a49af7fdfb9319eba699dc90c95df60c7c568","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from networking_ovn.tests.unit.db import base as db_base"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class TestMaintenance(db_base.DBTestCase, test_plugin.Ml2PluginV2TestCase):"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def setUp(self):"},{"line_number":28,"context_line":"        super(TestMaintenance, self).setUp()"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_065e93e5","line":25,"range":{"start_line":25,"start_character":6,"end_line":25,"end_character":21},"in_reply_to":"9f91af0f_304e6d6f","updated":"2018-01-05 10:56:25.000000000","message":"i couldn\u0027t find where the revision plugin is pulled in for this test.  if it relies on the side effect of something, it can break in future, eg. when the order of test execution is changed.","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"25549a5adff03875a83a0ca19d6d30d1d488339b","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from networking_ovn.tests.unit.db import base as db_base"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class TestMaintenance(db_base.DBTestCase, test_plugin.Ml2PluginV2TestCase):"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def setUp(self):"},{"line_number":28,"context_line":"        super(TestMaintenance, self).setUp()"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_6b55e8c5","line":25,"range":{"start_line":25,"start_character":6,"end_line":25,"end_character":21},"in_reply_to":"9f91af0f_3ccbac1e","updated":"2018-01-05 09:57:31.000000000","message":"I can explicitly add it (in case something change in the base class?) but it should be grand","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cc3da3b3059592499c7e8f8f7309937541113560","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from networking_ovn.tests.unit.db import base as db_base"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class TestMaintenance(db_base.DBTestCase, test_plugin.Ml2PluginV2TestCase):"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def setUp(self):"},{"line_number":28,"context_line":"        super(TestMaintenance, self).setUp()"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_304e6d6f","line":25,"range":{"start_line":25,"start_character":6,"end_line":25,"end_character":21},"in_reply_to":"9f91af0f_3ccbac1e","updated":"2018-01-05 10:33:51.000000000","message":"What do you mean?, test is passing.","commit_id":"cf939afbdcabc9d99467a96de8ff09387821b259"}]}