)]}'
{"networking_ovn/common/maintenance.py":[{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"95a019c6da5554f95b267031381f93bbee603b85","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            admin_context, row.resource_uuid)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        if row.revision_number \u003d\u003d ovn_const.INITIAL_REV_NUM:"},{"line_number":114,"context_line":"            self._ovn_client.create_port(p_db_obj)"},{"line_number":115,"context_line":"        else:"},{"line_number":116,"context_line":"            self._ovn_client.update_port(p_db_obj)"},{"line_number":117,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"9f91af0f_66caff6d","line":114,"updated":"2018-01-05 12:48:12.000000000","message":"may_exist\u003dTrue ? :)","commit_id":"0c91e6131f869854baf7175c148e8c173ee3fb61"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0367d0c55fe98d2f6d61a691a32635ca5d5edc7e","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            admin_context, row.resource_uuid)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        if row.revision_number \u003d\u003d ovn_const.INITIAL_REV_NUM:"},{"line_number":114,"context_line":"            self._ovn_client.create_port(p_db_obj)"},{"line_number":115,"context_line":"        else:"},{"line_number":116,"context_line":"            self._ovn_client.update_port(p_db_obj)"},{"line_number":117,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"9f91af0f_e8a819dc","line":114,"in_reply_to":"9f91af0f_66caff6d","updated":"2018-01-05 13:40:15.000000000","message":"++\n\nI\u0027m working on fixing all the patches in the series to take that into account! Thanks for pointing it out","commit_id":"0c91e6131f869854baf7175c148e8c173ee3fb61"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"92b1b8e7b38ac9d5c90c1b096cbc00cee8fdb225","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            try:"},{"line_number":180,"context_line":"                if row.resource_type \u003d\u003d ovn_const.TYPE_NETWORKS:"},{"line_number":181,"context_line":"                    self._fix_create_update_network(row)"},{"line_number":182,"context_line":"                elif row.resource_type \u003d\u003d ovn_const.TYPE_PORTS:"},{"line_number":183,"context_line":"                    self._fix_create_update_port(row)"},{"line_number":184,"context_line":"            except Exception:"},{"line_number":185,"context_line":"                LOG.exception(\u0027Failed to fix resource %(res_uuid)s \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f91af0f_62ebabdb","line":182,"updated":"2018-01-08 06:45:00.000000000","message":"Since the if, elif.... will only going to increase in subsequent patches, I would suggest maintaining a map of resource type - corresponding create_update function pointer in __init__ function given that _fix_create_update_(resource) is only taking a \u0027row\u0027 param.\n\nFor example here the code would be something like:\nself._resource_create_update_handler[row.resource_type](row)\n\nSame for the delete inconsistencies below.","commit_id":"bf003a1142999ecc899356cf20237aa1af93ee86"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"a423b5cf66f0d3aca65ac9d853f7f4bfa290b0a6","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            try:"},{"line_number":180,"context_line":"                if row.resource_type \u003d\u003d ovn_const.TYPE_NETWORKS:"},{"line_number":181,"context_line":"                    self._fix_create_update_network(row)"},{"line_number":182,"context_line":"                elif row.resource_type \u003d\u003d ovn_const.TYPE_PORTS:"},{"line_number":183,"context_line":"                    self._fix_create_update_port(row)"},{"line_number":184,"context_line":"            except Exception:"},{"line_number":185,"context_line":"                LOG.exception(\u0027Failed to fix resource %(res_uuid)s \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f91af0f_f3a3d3fe","line":182,"in_reply_to":"9f91af0f_09b3ba4e","updated":"2018-01-10 05:00:04.000000000","message":"+1. I am fine with this :)","commit_id":"bf003a1142999ecc899356cf20237aa1af93ee86"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ba10703e1f6f2c75dcd82112d8db67afc93f879e","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            try:"},{"line_number":180,"context_line":"                if row.resource_type \u003d\u003d ovn_const.TYPE_NETWORKS:"},{"line_number":181,"context_line":"                    self._fix_create_update_network(row)"},{"line_number":182,"context_line":"                elif row.resource_type \u003d\u003d ovn_const.TYPE_PORTS:"},{"line_number":183,"context_line":"                    self._fix_create_update_port(row)"},{"line_number":184,"context_line":"            except Exception:"},{"line_number":185,"context_line":"                LOG.exception(\u0027Failed to fix resource %(res_uuid)s \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f91af0f_09b3ba4e","line":182,"in_reply_to":"9f91af0f_4f531e40","updated":"2018-01-09 15:07:14.000000000","message":"@Numan, right.\n\nWhat we can do, if you agree with, is to refactor this bit at the end of the chain. That way we will have all resources covered already.","commit_id":"bf003a1142999ecc899356cf20237aa1af93ee86"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"b7f0df9265a247b5587a52baa28c4bffe9e52bfb","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            try:"},{"line_number":180,"context_line":"                if row.resource_type \u003d\u003d ovn_const.TYPE_NETWORKS:"},{"line_number":181,"context_line":"                    self._fix_create_update_network(row)"},{"line_number":182,"context_line":"                elif row.resource_type \u003d\u003d ovn_const.TYPE_PORTS:"},{"line_number":183,"context_line":"                    self._fix_create_update_port(row)"},{"line_number":184,"context_line":"            except Exception:"},{"line_number":185,"context_line":"                LOG.exception(\u0027Failed to fix resource %(res_uuid)s \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f91af0f_9575c266","line":182,"in_reply_to":"9f91af0f_62ebabdb","updated":"2018-01-08 11:52:11.000000000","message":"++\n\nYeah I had this type of refactor in mind, I just didn\u0027t know whether all fixes would look the same or if it would vary from resource to resource.\n\nMostly will be very similar, some will have some differnces (e.g SG and SG rules have no updates, just creation or deletion).\n\nBut anyway, I think we can benefit from refactoring it in a certain way to avoid most of the code duplication.","commit_id":"bf003a1142999ecc899356cf20237aa1af93ee86"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"3c1fdad069de1134c646b2c39f32e5a139d5231a","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            try:"},{"line_number":180,"context_line":"                if row.resource_type \u003d\u003d ovn_const.TYPE_NETWORKS:"},{"line_number":181,"context_line":"                    self._fix_create_update_network(row)"},{"line_number":182,"context_line":"                elif row.resource_type \u003d\u003d ovn_const.TYPE_PORTS:"},{"line_number":183,"context_line":"                    self._fix_create_update_port(row)"},{"line_number":184,"context_line":"            except Exception:"},{"line_number":185,"context_line":"                LOG.exception(\u0027Failed to fix resource %(res_uuid)s \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f91af0f_b55ec641","line":182,"in_reply_to":"9f91af0f_9575c266","updated":"2018-01-08 12:35:31.000000000","message":"I think it\u0027s not very critical, this is the way python has to express a case/switch. The maps are great for dynamic stuff, but this is fine too in this case :)\n\nHereby I express my IDC (I don\u0027t care) about this O:-)","commit_id":"bf003a1142999ecc899356cf20237aa1af93ee86"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"444846f8427c52d1035c8897a42a72f5a1a95f67","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            try:"},{"line_number":180,"context_line":"                if row.resource_type \u003d\u003d ovn_const.TYPE_NETWORKS:"},{"line_number":181,"context_line":"                    self._fix_create_update_network(row)"},{"line_number":182,"context_line":"                elif row.resource_type \u003d\u003d ovn_const.TYPE_PORTS:"},{"line_number":183,"context_line":"                    self._fix_create_update_port(row)"},{"line_number":184,"context_line":"            except Exception:"},{"line_number":185,"context_line":"                LOG.exception(\u0027Failed to fix resource %(res_uuid)s \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f91af0f_4f531e40","line":182,"in_reply_to":"9f91af0f_b55ec641","updated":"2018-01-08 18:41:20.000000000","message":"I think I will remove -1 :).\nlookup tables in C use array of function pointers . otherwise switch/if-else-if are generally used.\n\nEven I hereby express by IDC.\n@Lucas - Please leave as or modify what you think is best :)","commit_id":"bf003a1142999ecc899356cf20237aa1af93ee86"}],"networking_ovn/common/ovn_client.py":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"40a65d57d5e1c756735a0565cf40a89018f971d9","unresolved":false,"context_lines":[{"line_number":316,"context_line":"                        ovn_const.OVN_DEVID_EXT_ID_KEY: port[\u0027device_id\u0027],"},{"line_number":317,"context_line":"                        ovn_const.OVN_PROJID_EXT_ID_KEY: port[\u0027project_id\u0027],"},{"line_number":318,"context_line":"                        ovn_const.OVN_CIDRS_EXT_ID_KEY: port_info.cidrs,"},{"line_number":319,"context_line":"                        ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY:"},{"line_number":320,"context_line":"                            port_info.device_owner,"},{"line_number":321,"context_line":"                        ovn_const.OVN_SG_IDS_EXT_ID_KEY:"},{"line_number":322,"context_line":"                            port_info.security_group_ids,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_4b6a82b3","line":319,"updated":"2017-11-22 11:45:13.000000000","message":"I dislike having most of the information as external ids. Also new fields (if any, which is unlikely) should be in here. As I don\u0027t know of a better way I just wanted to highlight it should anyone comes up with a better idea.\nThese are not indexable and there\u0027s not a proper format definition like for example how do we represent the security groups? comma separated list? whitespace separated list? Anyways, I\u0027m okay with this since as I said,  I can\u0027t see another option.","commit_id":"128489939e403475e887f3712af7aec04280e250"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0156a5a6c78fc794763fa5e479a2d87ac9ec33f2","unresolved":false,"context_lines":[{"line_number":316,"context_line":"                        ovn_const.OVN_DEVID_EXT_ID_KEY: port[\u0027device_id\u0027],"},{"line_number":317,"context_line":"                        ovn_const.OVN_PROJID_EXT_ID_KEY: port[\u0027project_id\u0027],"},{"line_number":318,"context_line":"                        ovn_const.OVN_CIDRS_EXT_ID_KEY: port_info.cidrs,"},{"line_number":319,"context_line":"                        ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY:"},{"line_number":320,"context_line":"                            port_info.device_owner,"},{"line_number":321,"context_line":"                        ovn_const.OVN_SG_IDS_EXT_ID_KEY:"},{"line_number":322,"context_line":"                            port_info.security_group_ids,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_fc6ebb94","line":319,"in_reply_to":"ff82abbf_4b6a82b3","updated":"2017-11-22 15:28:51.000000000","message":"I agree with your statement. And a better option is to change the OVNDB schema to better match our needs e.g. Having the revision_number field to be indexable. This will take time and will need more consensus with the OVN community upstream to remodel the DB and add the fields to make OVN work better with one of the solutions that uses it as a backend (maybe others maybe reuse such fields, or maybe not).\n\nI think it\u0027s a fair thing to push forward this ideas in the OVN upstream community since they will help us optimizing our solution here but, as it\u0027s an optimization/tidying, we could do that in parallel or after we solve the sync problem. Is that OK?","commit_id":"128489939e403475e887f3712af7aec04280e250"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"53991cd02b68f412d9d88bcf6b20b7b29cc08441","unresolved":false,"context_lines":[{"line_number":316,"context_line":"                        ovn_const.OVN_DEVID_EXT_ID_KEY: port[\u0027device_id\u0027],"},{"line_number":317,"context_line":"                        ovn_const.OVN_PROJID_EXT_ID_KEY: port[\u0027project_id\u0027],"},{"line_number":318,"context_line":"                        ovn_const.OVN_CIDRS_EXT_ID_KEY: port_info.cidrs,"},{"line_number":319,"context_line":"                        ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY:"},{"line_number":320,"context_line":"                            port_info.device_owner,"},{"line_number":321,"context_line":"                        ovn_const.OVN_SG_IDS_EXT_ID_KEY:"},{"line_number":322,"context_line":"                            port_info.security_group_ids,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_37ae7c98","line":319,"in_reply_to":"ff82abbf_fc6ebb94","updated":"2017-11-22 15:56:57.000000000","message":"Great, thanks Lucas!","commit_id":"128489939e403475e887f3712af7aec04280e250"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"40a65d57d5e1c756735a0565cf40a89018f971d9","unresolved":false,"context_lines":[{"line_number":448,"context_line":"                                        addrs_remove\u003daddr_remove))"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"        if check_rev_cmd.result \u003d\u003d ovn_const.TXN_COMMITTED:"},{"line_number":451,"context_line":"            db_rev.bump_revision(port, ovn_const.TYPE_PORTS)"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    def delete_port(self, port):"},{"line_number":454,"context_line":"        with self._nb_idl.transaction(check_error\u003dTrue) as txn:"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_7009752f","line":451,"updated":"2017-11-22 11:45:13.000000000","message":"At this point, transaction\u0027s been already commited and we\u0027re bumping the rev number in neutron. This is racey as you already pointed out in the other review but as you also clarified there, you\u0027re relying on the mechanism thread to sync dbs periodically taking OVN DB as the source of truth.\nIs that correct?","commit_id":"128489939e403475e887f3712af7aec04280e250"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0156a5a6c78fc794763fa5e479a2d87ac9ec33f2","unresolved":false,"context_lines":[{"line_number":448,"context_line":"                                        addrs_remove\u003daddr_remove))"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"        if check_rev_cmd.result \u003d\u003d ovn_const.TXN_COMMITTED:"},{"line_number":451,"context_line":"            db_rev.bump_revision(port, ovn_const.TYPE_PORTS)"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    def delete_port(self, port):"},{"line_number":454,"context_line":"        with self._nb_idl.transaction(check_error\u003dTrue) as txn:"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_912e78f8","line":451,"in_reply_to":"ff82abbf_7009752f","updated":"2017-11-22 15:28:51.000000000","message":"Hi,\n\nYes for both assumptions.\n\n1) \n\nThe \"result\" attribute will only contain TXN_COMMITTED if the transaction has been already committed (it happens in the post_commit() of the OVS command [0])\n\n2)\n\nRegarding the race that can happen when bump_revision() is called. That\u0027s true, it\u0027s important to note that OVNDB is the source of truth and the revision number will also be set in the external_ids of the resource within the same update transaction.\n\nThis was something that we discussed in the spec [1], basically since the new approach does not have any lock to link these two transactions (OVNDB and NeutronDB with bump_revision()) it can get out of sync.\n\nThat\u0027s expected tho, since the new table in the NeutronDB - ovn_revision_numbers - is just an optimization (again, OVNDB is the source of truth) and if it gets out of sync, the periodic mechanism that detects inconsistencies will look at both databases and see that the revision in OVN matches the standardattributes table in neutron but not the ovn_revision_numbers one, so instead of sync\u0027ing the resource it will just fix the table. I think it\u0027s a fair and cheap cost for not needing to add an external lock to the solution.\n\n[0] https://review.openstack.org/#/c/517049/8/networking_ovn/ovsdb/commands.py@1046\n\n[1] https://review.openstack.org/#/c/490834/8/doc/source/contributor/design/database_consistency.rst@122 See the comment by Han on Oct 25 11:56 PM.","commit_id":"128489939e403475e887f3712af7aec04280e250"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"53991cd02b68f412d9d88bcf6b20b7b29cc08441","unresolved":false,"context_lines":[{"line_number":448,"context_line":"                                        addrs_remove\u003daddr_remove))"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"        if check_rev_cmd.result \u003d\u003d ovn_const.TXN_COMMITTED:"},{"line_number":451,"context_line":"            db_rev.bump_revision(port, ovn_const.TYPE_PORTS)"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    def delete_port(self, port):"},{"line_number":454,"context_line":"        with self._nb_idl.transaction(check_error\u003dTrue) as txn:"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_f79d7457","line":451,"in_reply_to":"ff82abbf_912e78f8","updated":"2017-11-22 15:56:57.000000000","message":"Thanks for clarifying further","commit_id":"128489939e403475e887f3712af7aec04280e250"}],"networking_ovn/common/utils.py":[{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"92b1b8e7b38ac9d5c90c1b096cbc00cee8fdb225","unresolved":false,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"def is_lsp_router_port(port):"},{"line_number":273,"context_line":"    return port.get(\u0027device_owner\u0027) in [const.DEVICE_OWNER_ROUTER_INTF,"},{"line_number":274,"context_line":"                                        const.DEVICE_OWNER_ROUTER_GW]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f91af0f_82deef6a","line":273,"updated":"2018-01-08 06:45:00.000000000","message":"@Miguel @Daniel - For migration scenarios, the router port device owner could be different. Migration agent should probably change the device owner to DEVICE_OWNER_ROUTER_INTF/DEVICE_OWNER_ROUTER_GW.\n\nThought of commenting here so that we don\u0027t forget :).","commit_id":"bf003a1142999ecc899356cf20237aa1af93ee86"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"3c1fdad069de1134c646b2c39f32e5a139d5231a","unresolved":false,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"def is_lsp_router_port(port):"},{"line_number":273,"context_line":"    return port.get(\u0027device_owner\u0027) in [const.DEVICE_OWNER_ROUTER_INTF,"},{"line_number":274,"context_line":"                                        const.DEVICE_OWNER_ROUTER_GW]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f91af0f_352156b1","line":273,"in_reply_to":"9f91af0f_82deef6a","updated":"2018-01-08 12:35:31.000000000","message":"Ack, ack!! :) Where can we make a note not to forget about it?","commit_id":"bf003a1142999ecc899356cf20237aa1af93ee86"}]}