)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"22122088df0400db4cb2eae2b241d1877798088c","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Since HealthMonitors are a not a part of OVN Octavia LB Driver,"},{"line_number":10,"context_line":"there are some changes with which Pool and Members behave. Also"},{"line_number":11,"context_line":"Pools are generally OFFLINE till a member is not added to it since"},{"line_number":12,"context_line":"the observed status of the pool is inoperatable till it has members"},{"line_number":13,"context_line":"associated to it."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fdfeff1_7e3b0b37","line":11,"range":{"start_line":11,"start_character":10,"end_line":11,"end_character":19},"updated":"2019-02-15 10:01:48.000000000","message":"s/generally//\n\nit\u0027s always OFFLINE if it has no members right ?","commit_id":"748d5f6ab0815e421131ff40e9628beb76d1233d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"da4ea18c59255065617d7c377eeb7adfd0066f5f","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Since HealthMonitors are a not a part of OVN Octavia LB Driver,"},{"line_number":10,"context_line":"there are some changes with which Pool and Members behave. Also"},{"line_number":11,"context_line":"Pools are OFFLINE till a member is not added to it since"},{"line_number":12,"context_line":"the observed status of the pool is inoperatable till it has members"},{"line_number":13,"context_line":"associated to it."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"5fc1f717_46587455","line":11,"range":{"start_line":11,"start_character":18,"end_line":11,"end_character":44},"updated":"2019-03-20 13:20:48.000000000","message":"nit: member is added ?","commit_id":"325c8ad15af60e8e019265ca595889074bdc5d4f"}],"networking_ovn/octavia/ovn_driver.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"22122088df0400db4cb2eae2b241d1877798088c","unresolved":false,"context_lines":[{"line_number":860,"context_line":"            self.ovn_nbdb_api.db_set("},{"line_number":861,"context_line":"                \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":862,"context_line":"                (\u0027external_ids\u0027, external_ids)).execute(check_error\u003dTrue)"},{"line_number":863,"context_line":"            # Pool status is Offline, till a member is not added to it."},{"line_number":864,"context_line":"            operating_status \u003d constants.OFFLINE"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"            status \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_9e96ef3d","line":863,"range":{"start_line":863,"start_character":52,"end_line":863,"end_character":64},"updated":"2019-02-15 10:01:48.000000000","message":"I think you mean, \"is added to it\" ?","commit_id":"748d5f6ab0815e421131ff40e9628beb76d1233d"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"e2667c2d1712c25d7d84bebafb05540717f63931","unresolved":false,"context_lines":[{"line_number":860,"context_line":"            self.ovn_nbdb_api.db_set("},{"line_number":861,"context_line":"                \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":862,"context_line":"                (\u0027external_ids\u0027, external_ids)).execute(check_error\u003dTrue)"},{"line_number":863,"context_line":"            # Pool status is Offline, till a member is not added to it."},{"line_number":864,"context_line":"            operating_status \u003d constants.OFFLINE"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"            status \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_d57bf577","line":863,"range":{"start_line":863,"start_character":52,"end_line":863,"end_character":64},"in_reply_to":"9fdfeff1_9e96ef3d","updated":"2019-02-15 11:39:02.000000000","message":"Till a member is not added to a pool  , it remains offline.","commit_id":"748d5f6ab0815e421131ff40e9628beb76d1233d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"22122088df0400db4cb2eae2b241d1877798088c","unresolved":false,"context_lines":[{"line_number":1110,"context_line":"            self._execute_commands(commands)"},{"line_number":1111,"context_line":"            return pool_status"},{"line_number":1112,"context_line":"        else:"},{"line_number":1113,"context_line":"            raise Exception(_(\"Member not found in the pool\"))"},{"line_number":1114,"context_line":""},{"line_number":1115,"context_line":"    def member_delete(self, member):"},{"line_number":1116,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_beb2d3e3","line":1113,"range":{"start_line":1113,"start_character":30,"end_line":1113,"end_character":60},"updated":"2019-02-15 10:01:48.000000000","message":"Let\u0027s add the member ID here so we know which member we are talking about","commit_id":"748d5f6ab0815e421131ff40e9628beb76d1233d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"22122088df0400db4cb2eae2b241d1877798088c","unresolved":false,"context_lines":[{"line_number":1166,"context_line":"                                              is_enabled\u003dFalse)"},{"line_number":1167,"context_line":"                ovn_lb \u003d self._find_ovn_lb_with_pool_key(pool_key)"},{"line_number":1168,"context_line":""},{"line_number":1169,"context_line":"            # Update member information. This can be further improved"},{"line_number":1170,"context_line":"            self._remove_member(member, ovn_lb, pool_key)"},{"line_number":1171,"context_line":"            self._add_member(member, ovn_lb, pool_key)"},{"line_number":1172,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_be009367","line":1169,"updated":"2019-02-15 10:01:48.000000000","message":"++ big TODO here :D","commit_id":"748d5f6ab0815e421131ff40e9628beb76d1233d"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"6b30eaa6fc750b972107a4bccd068aaad029f3a6","unresolved":false,"context_lines":[{"line_number":1364,"context_line":"                   \u0027info\u0027: request_info}"},{"line_number":1365,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":1366,"context_line":""},{"line_number":1367,"context_line":"    def member_batch_update(self, members):"},{"line_number":1368,"context_line":"        for member in members:"},{"line_number":1369,"context_line":"            self.member_update(member, member)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_3f3ec18a","side":"PARENT","line":1367,"updated":"2019-02-18 06:21:35.000000000","message":"Any particular reason why batch_update is removed from this patch ?","commit_id":"abc68fbeb3a4f433937233ee3cfb7a2ef99f40c0"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"d2a4ee968f1517b4667432d57290b6c4887e3032","unresolved":false,"context_lines":[{"line_number":1364,"context_line":"                   \u0027info\u0027: request_info}"},{"line_number":1365,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":1366,"context_line":""},{"line_number":1367,"context_line":"    def member_batch_update(self, members):"},{"line_number":1368,"context_line":"        for member in members:"},{"line_number":1369,"context_line":"            self.member_update(member, member)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_51ab7986","side":"PARENT","line":1367,"in_reply_to":"9fdfeff1_3f3ec18a","updated":"2019-02-18 10:01:04.000000000","message":"I changed a lot of the stuff in https://review.openstack.org/605343. I didnt want to mess around here so removed this function for now. I will make the batch member update dependent on this patch so that the status changes in the pool can be reflected in that patch.","commit_id":"abc68fbeb3a4f433937233ee3cfb7a2ef99f40c0"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"6b30eaa6fc750b972107a4bccd068aaad029f3a6","unresolved":false,"context_lines":[{"line_number":1020,"context_line":"        external_ids \u003d copy.deepcopy(ovn_lb.external_ids)"},{"line_number":1021,"context_line":"        existing_members \u003d external_ids[pool_key]"},{"line_number":1022,"context_line":"        member_info \u003d self._get_member_key(member)"},{"line_number":1023,"context_line":"        pool_opr_status \u003d None"},{"line_number":1024,"context_line":""},{"line_number":1025,"context_line":"        if existing_members:"},{"line_number":1026,"context_line":"            pool_data \u003d {pool_key: existing_members + \",\" + member_info}"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_3fc9e1aa","line":1023,"updated":"2019-02-18 06:21:35.000000000","message":"how about naming it to - \u0027pool_oper_status\u0027 instead ?","commit_id":"58d84216622df8b597bbd74b6b10510bba97c280"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"d2a4ee968f1517b4667432d57290b6c4887e3032","unresolved":false,"context_lines":[{"line_number":1020,"context_line":"        external_ids \u003d copy.deepcopy(ovn_lb.external_ids)"},{"line_number":1021,"context_line":"        existing_members \u003d external_ids[pool_key]"},{"line_number":1022,"context_line":"        member_info \u003d self._get_member_key(member)"},{"line_number":1023,"context_line":"        pool_opr_status \u003d None"},{"line_number":1024,"context_line":""},{"line_number":1025,"context_line":"        if existing_members:"},{"line_number":1026,"context_line":"            pool_data \u003d {pool_key: existing_members + \",\" + member_info}"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_f17e2d17","line":1023,"in_reply_to":"9fdfeff1_3fc9e1aa","updated":"2019-02-18 10:01:04.000000000","message":"I guess that its a nit, I can change it","commit_id":"58d84216622df8b597bbd74b6b10510bba97c280"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"6b30eaa6fc750b972107a4bccd068aaad029f3a6","unresolved":false,"context_lines":[{"line_number":1053,"context_line":"                pool_key \u003d self._get_pool_key(member[\u0027pool_id\u0027],"},{"line_number":1054,"context_line":"                                              is_enabled\u003dFalse)"},{"line_number":1055,"context_line":"                ovn_lb \u003d self._find_ovn_lb_with_pool_key(pool_key)"},{"line_number":1056,"context_line":"            pool_opr_status \u003d self._add_member(member, ovn_lb, pool_key)"},{"line_number":1057,"context_line":"            pool \u003d {\"id\": member[\u0027pool_id\u0027],"},{"line_number":1058,"context_line":"                    \"provisioning_status\": constants.ACTIVE}"},{"line_number":1059,"context_line":"            if pool_opr_status:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_1ff5fdca","line":1056,"updated":"2019-02-18 06:21:35.000000000","message":"Will there be a problem if the operation status of pool is set o ONLINE for every member create ?","commit_id":"58d84216622df8b597bbd74b6b10510bba97c280"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"d2a4ee968f1517b4667432d57290b6c4887e3032","unresolved":false,"context_lines":[{"line_number":1053,"context_line":"                pool_key \u003d self._get_pool_key(member[\u0027pool_id\u0027],"},{"line_number":1054,"context_line":"                                              is_enabled\u003dFalse)"},{"line_number":1055,"context_line":"                ovn_lb \u003d self._find_ovn_lb_with_pool_key(pool_key)"},{"line_number":1056,"context_line":"            pool_opr_status \u003d self._add_member(member, ovn_lb, pool_key)"},{"line_number":1057,"context_line":"            pool \u003d {\"id\": member[\u0027pool_id\u0027],"},{"line_number":1058,"context_line":"                    \"provisioning_status\": constants.ACTIVE}"},{"line_number":1059,"context_line":"            if pool_opr_status:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_318115fc","line":1056,"in_reply_to":"9fdfeff1_1ff5fdca","updated":"2019-02-18 10:01:04.000000000","message":"Complexity wise, it would be easier to handle. and wont mess anything. So yeah, I can probably change the work so that it is online whenever a member is added to it","commit_id":"58d84216622df8b597bbd74b6b10510bba97c280"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"6b30eaa6fc750b972107a4bccd068aaad029f3a6","unresolved":false,"context_lines":[{"line_number":1157,"context_line":"        existing_members \u003d external_ids[pool_key].split(\",\")"},{"line_number":1158,"context_line":"        member_info \u003d self._get_member_key(member)"},{"line_number":1159,"context_line":"        for mem in existing_members:"},{"line_number":1160,"context_line":"            if member_info.split(\u0027_\u0027)[1] \u003d\u003d mem.split("},{"line_number":1161,"context_line":"                \u0027_\u0027)[1] and mem !\u003d member_info:"},{"line_number":1162,"context_line":"                existing_members.remove(mem)"},{"line_number":1163,"context_line":"                existing_members.append(member_info)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_7f0689d0","line":1160,"updated":"2019-02-18 06:21:35.000000000","message":"The indentation here seems to be inconsistent with the rest of  the file or rest of the code base.\nIt is better if we have 4 space indentation.\nOr may be I am confused :).","commit_id":"58d84216622df8b597bbd74b6b10510bba97c280"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"d2a4ee968f1517b4667432d57290b6c4887e3032","unresolved":false,"context_lines":[{"line_number":1157,"context_line":"        existing_members \u003d external_ids[pool_key].split(\",\")"},{"line_number":1158,"context_line":"        member_info \u003d self._get_member_key(member)"},{"line_number":1159,"context_line":"        for mem in existing_members:"},{"line_number":1160,"context_line":"            if member_info.split(\u0027_\u0027)[1] \u003d\u003d mem.split("},{"line_number":1161,"context_line":"                \u0027_\u0027)[1] and mem !\u003d member_info:"},{"line_number":1162,"context_line":"                existing_members.remove(mem)"},{"line_number":1163,"context_line":"                existing_members.append(member_info)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_5199396a","line":1160,"in_reply_to":"9fdfeff1_7f0689d0","updated":"2019-02-18 10:01:04.000000000","message":"Its because of the length of the if loop","commit_id":"58d84216622df8b597bbd74b6b10510bba97c280"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"6b30eaa6fc750b972107a4bccd068aaad029f3a6","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"                commands.extend("},{"line_number":1171,"context_line":"                    self._refresh_lb_vips(ovn_lb.uuid, external_ids)"},{"line_number":1172,"context_line":"                )"},{"line_number":1173,"context_line":"                self._execute_commands(commands)"},{"line_number":1174,"context_line":""},{"line_number":1175,"context_line":"    def member_update(self, member):"},{"line_number":1176,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_7f61a9a8","line":1173,"range":{"start_line":1173,"start_character":16,"end_line":1173,"end_character":48},"updated":"2019-02-18 06:21:35.000000000","message":"Shouldn\u0027t this line be after for ? so that you execute commands after the commands are added to the \u0027commands\u0027 list ?","commit_id":"58d84216622df8b597bbd74b6b10510bba97c280"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"0fc15f58ec29416179b0a076def67d196f583245","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"                commands.extend("},{"line_number":1171,"context_line":"                    self._refresh_lb_vips(ovn_lb.uuid, external_ids)"},{"line_number":1172,"context_line":"                )"},{"line_number":1173,"context_line":"                self._execute_commands(commands)"},{"line_number":1174,"context_line":""},{"line_number":1175,"context_line":"    def member_update(self, member):"},{"line_number":1176,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_09229d02","line":1173,"range":{"start_line":1173,"start_character":16,"end_line":1173,"end_character":48},"in_reply_to":"9fdfeff1_71b9ddc9","updated":"2019-02-18 19:50:44.000000000","message":"You can do something like\n\n****\nif commands:\n    self._execute_commands(commands)\n****\n\nEvery call to _execute_commands will result in a transaction to the ovsdb-server.\nSo if there are 5 members in \u0027existing_members\u0027 then there would be 5 txns.\nAdding all the commands to a list and calling _exeucte_commands() will result in only 1 transaction to ovsdb-server.","commit_id":"58d84216622df8b597bbd74b6b10510bba97c280"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"d2a4ee968f1517b4667432d57290b6c4887e3032","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"                commands.extend("},{"line_number":1171,"context_line":"                    self._refresh_lb_vips(ovn_lb.uuid, external_ids)"},{"line_number":1172,"context_line":"                )"},{"line_number":1173,"context_line":"                self._execute_commands(commands)"},{"line_number":1174,"context_line":""},{"line_number":1175,"context_line":"    def member_update(self, member):"},{"line_number":1176,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_71b9ddc9","line":1173,"range":{"start_line":1173,"start_character":16,"end_line":1173,"end_character":48},"in_reply_to":"9fdfeff1_7f61a9a8","updated":"2019-02-18 10:01:04.000000000","message":"there are no commands in case the if loop is not executed. So I thought of keeping it in the if-loop and save one function call.","commit_id":"58d84216622df8b597bbd74b6b10510bba97c280"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"da4ea18c59255065617d7c377eeb7adfd0066f5f","unresolved":false,"context_lines":[{"line_number":1458,"context_line":"                   \u0027info\u0027: request_info}"},{"line_number":1459,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"    def member_batch_update(self, members):"},{"line_number":1462,"context_line":"        # Note(rbanerje): all members belong to the same pool."},{"line_number":1463,"context_line":"        request_list \u003d []"},{"line_number":1464,"context_line":"        skipped_members \u003d []"}],"source_content_type":"text/x-python","patch_set":13,"id":"5fc1f717_da2678bb","line":1461,"updated":"2019-03-20 13:20:48.000000000","message":"Question:\n\nSince we are checking for the monitor options when updating/creating a single member, wouldn\u0027t it make sense to also validate it for batch updates ?","commit_id":"325c8ad15af60e8e019265ca595889074bdc5d4f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"3fd66c601751432433cbcc0536bbeaa0144bd532","unresolved":false,"context_lines":[{"line_number":1458,"context_line":"                   \u0027info\u0027: request_info}"},{"line_number":1459,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"    def member_batch_update(self, members):"},{"line_number":1462,"context_line":"        # Note(rbanerje): all members belong to the same pool."},{"line_number":1463,"context_line":"        request_list \u003d []"},{"line_number":1464,"context_line":"        skipped_members \u003d []"}],"source_content_type":"text/x-python","patch_set":14,"id":"5fc1f717_757b4e27","line":1461,"updated":"2019-03-26 11:45:16.000000000","message":"(Question from patch-set #13)\n\nSince we are checking for the monitor options when updating/creating a single member, wouldn\u0027t it make sense to also validate it for batch updates ?","commit_id":"b019e4c2229de5d577d7f09d843724db82d86985"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"e53e14f7e06fc330cb696a54fa4847fae5db53ee","unresolved":false,"context_lines":[{"line_number":1458,"context_line":"                   \u0027info\u0027: request_info}"},{"line_number":1459,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"    def member_batch_update(self, members):"},{"line_number":1462,"context_line":"        # Note(rbanerje): all members belong to the same pool."},{"line_number":1463,"context_line":"        request_list \u003d []"},{"line_number":1464,"context_line":"        skipped_members \u003d []"}],"source_content_type":"text/x-python","patch_set":14,"id":"5fc1f717_fb6ff65c","line":1461,"in_reply_to":"5fc1f717_757b4e27","updated":"2019-03-28 14:51:03.000000000","message":"Please review Line#1516 onward :)","commit_id":"b019e4c2229de5d577d7f09d843724db82d86985"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"be3ca0e7188e8e34afc3c54639c4e62207e2be10","unresolved":false,"context_lines":[{"line_number":1031,"context_line":"        external_ids \u003d copy.deepcopy(ovn_lb.external_ids)"},{"line_number":1032,"context_line":"        existing_members \u003d external_ids[pool_key]"},{"line_number":1033,"context_line":"        member_info \u003d self._get_member_key(member)"},{"line_number":1034,"context_line":"        if member_info in existing_members.split(\u0027,\u0027):"},{"line_number":1035,"context_line":"            # member already present. No need to do anything."},{"line_number":1036,"context_line":"            return"},{"line_number":1037,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"5fc1f717_aa503133","side":"PARENT","line":1034,"range":{"start_line":1034,"start_character":8,"end_line":1034,"end_character":10},"updated":"2019-03-28 18:54:19.000000000","message":"Why remove this block? Wouldn’t it be adding it again in L1034?","commit_id":"d84ad7378bfd8f08cd3775952713a3faf9a62544"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"9ce5c12b9c431c51705a414fb21a524802d95fc7","unresolved":false,"context_lines":[{"line_number":1031,"context_line":"        external_ids \u003d copy.deepcopy(ovn_lb.external_ids)"},{"line_number":1032,"context_line":"        existing_members \u003d external_ids[pool_key]"},{"line_number":1033,"context_line":"        member_info \u003d self._get_member_key(member)"},{"line_number":1034,"context_line":"        if member_info in existing_members.split(\u0027,\u0027):"},{"line_number":1035,"context_line":"            # member already present. No need to do anything."},{"line_number":1036,"context_line":"            return"},{"line_number":1037,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"5fc1f717_fd332e12","side":"PARENT","line":1034,"range":{"start_line":1034,"start_character":8,"end_line":1034,"end_character":10},"in_reply_to":"5fc1f717_aa503133","updated":"2019-04-02 02:28:00.000000000","message":"So I was also wondering the same thing, whether this is required or not. The member information is being sent by the Octavia Provider Driver. So if a member is already added to the LoadBalancer, ideally this should be checked in the API itself, isnt it?\nBut yes, there should be no harm adding it again","commit_id":"d84ad7378bfd8f08cd3775952713a3faf9a62544"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"be3ca0e7188e8e34afc3c54639c4e62207e2be10","unresolved":false,"context_lines":[{"line_number":870,"context_line":"            self.ovn_nbdb_api.db_set("},{"line_number":871,"context_line":"                \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":872,"context_line":"                (\u0027external_ids\u0027, external_ids)).execute(check_error\u003dTrue)"},{"line_number":873,"context_line":"            # Pool status is Online after a member is added to it."},{"line_number":874,"context_line":"            operating_status \u003d constants.OFFLINE"},{"line_number":875,"context_line":""},{"line_number":876,"context_line":"            status \u003d {"}],"source_content_type":"text/x-python","patch_set":15,"id":"5fc1f717_6ab80974","line":873,"range":{"start_line":873,"start_character":26,"end_line":873,"end_character":28},"updated":"2019-03-28 18:54:19.000000000","message":"s/is/will become","commit_id":"0a9447a615966bef2765a6bed38728f1dfc43772"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"be3ca0e7188e8e34afc3c54639c4e62207e2be10","unresolved":false,"context_lines":[{"line_number":1115,"context_line":"            self._execute_commands(commands)"},{"line_number":1116,"context_line":"            return pool_status"},{"line_number":1117,"context_line":"        else:"},{"line_number":1118,"context_line":"            raise Exception("},{"line_number":1119,"context_line":"                _(\"Member %s not found in the pool\") % member[\u0027id\u0027])"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"    def member_delete(self, member):"}],"source_content_type":"text/x-python","patch_set":15,"id":"5fc1f717_6a9fa901","line":1118,"range":{"start_line":1118,"start_character":18,"end_line":1118,"end_character":27},"updated":"2019-03-28 18:54:19.000000000","message":"What about having custom exceptions for the Member operations?\nAlso it would be nice to add a docstring","commit_id":"0a9447a615966bef2765a6bed38728f1dfc43772"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"9ce5c12b9c431c51705a414fb21a524802d95fc7","unresolved":false,"context_lines":[{"line_number":1115,"context_line":"            self._execute_commands(commands)"},{"line_number":1116,"context_line":"            return pool_status"},{"line_number":1117,"context_line":"        else:"},{"line_number":1118,"context_line":"            raise Exception("},{"line_number":1119,"context_line":"                _(\"Member %s not found in the pool\") % member[\u0027id\u0027])"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"    def member_delete(self, member):"}],"source_content_type":"text/x-python","patch_set":15,"id":"5fc1f717_9de78aad","line":1118,"range":{"start_line":1118,"start_character":18,"end_line":1118,"end_character":27},"in_reply_to":"5fc1f717_6a9fa901","updated":"2019-04-02 02:28:00.000000000","message":"Hmm, I would be creating a separate patch to handle all exceptions. May be we can handle this comment there?\nNeed to sort out all exceptions clearly","commit_id":"0a9447a615966bef2765a6bed38728f1dfc43772"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"9ad97190f211580b621eaa85ac8075bf8c5de1ee","unresolved":false,"context_lines":[{"line_number":1115,"context_line":"            self._execute_commands(commands)"},{"line_number":1116,"context_line":"            return pool_status"},{"line_number":1117,"context_line":"        else:"},{"line_number":1118,"context_line":"            raise Exception("},{"line_number":1119,"context_line":"                _(\"Member %s not found in the pool\") % member[\u0027id\u0027])"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"    def member_delete(self, member):"}],"source_content_type":"text/x-python","patch_set":15,"id":"5fc1f717_10ee21ed","line":1118,"range":{"start_line":1118,"start_character":18,"end_line":1118,"end_character":27},"in_reply_to":"5fc1f717_9de78aad","updated":"2019-04-02 03:00:19.000000000","message":"I misunderstood the comment and have updated the patch to reflect a custom exception :)","commit_id":"0a9447a615966bef2765a6bed38728f1dfc43772"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"f68436b64ac2589478369df22f1c1b1d20ea04c0","unresolved":false,"context_lines":[{"line_number":1094,"context_line":"    def _remove_member(self, member, ovn_lb, pool_key):"},{"line_number":1095,"context_line":"        external_ids \u003d copy.deepcopy(ovn_lb.external_ids)"},{"line_number":1096,"context_line":"        existing_members \u003d external_ids[pool_key].split(\",\")"},{"line_number":1097,"context_line":"        pool_status \u003d constants.ONLINE"},{"line_number":1098,"context_line":"        member_info \u003d self._get_member_key(member)"},{"line_number":1099,"context_line":"        if member_info in existing_members:"},{"line_number":1100,"context_line":"            commands \u003d []"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_68c76c66","line":1097,"updated":"2019-04-02 15:48:13.000000000","message":"I would move this line 1097 to line 1103 as \"else\"\ni.e\nif not existing_members:\n  pool_status \u003d constants.OFFLINE\nelse:\n  pool_status \u003d constants. ONLINE\n\nThe reason being, we raise exception in else at L1120 and the variable is used within the \"if\" at L1099.","commit_id":"226cc03502ead4e3f89db7403bfef43bbff68d3d"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"04b1840cedeedeb1a18c3143ddb9f002e95d551e","unresolved":false,"context_lines":[{"line_number":1094,"context_line":"    def _remove_member(self, member, ovn_lb, pool_key):"},{"line_number":1095,"context_line":"        external_ids \u003d copy.deepcopy(ovn_lb.external_ids)"},{"line_number":1096,"context_line":"        existing_members \u003d external_ids[pool_key].split(\",\")"},{"line_number":1097,"context_line":"        pool_status \u003d constants.ONLINE"},{"line_number":1098,"context_line":"        member_info \u003d self._get_member_key(member)"},{"line_number":1099,"context_line":"        if member_info in existing_members:"},{"line_number":1100,"context_line":"            commands \u003d []"}],"source_content_type":"text/x-python","patch_set":18,"id":"3fce034c_a7025c08","line":1097,"in_reply_to":"5fc1f717_68c76c66","updated":"2019-04-15 09:03:03.000000000","message":"Done","commit_id":"226cc03502ead4e3f89db7403bfef43bbff68d3d"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"f68436b64ac2589478369df22f1c1b1d20ea04c0","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"                                              is_enabled\u003dFalse)"},{"line_number":1198,"context_line":"                ovn_lb \u003d self._find_ovn_lb_with_pool_key(pool_key)"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"            self._update_member(member, ovn_lb, pool_key)"},{"line_number":1201,"context_line":""},{"line_number":1202,"context_line":"            pool_listeners \u003d self._get_pool_listeners(ovn_lb, pool_key)"},{"line_number":1203,"context_line":"            listener_status \u003d []"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_a8a0f492","line":1200,"updated":"2019-04-02 15:48:13.000000000","message":"Sorry if this was already commented before ? but why the \u0027admin_state_up\u0027 check is deleted here ?\nWas the earlier interpretation of \u0027admin_state_up\u0027 wrong ?","commit_id":"226cc03502ead4e3f89db7403bfef43bbff68d3d"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"04b1840cedeedeb1a18c3143ddb9f002e95d551e","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"                                              is_enabled\u003dFalse)"},{"line_number":1198,"context_line":"                ovn_lb \u003d self._find_ovn_lb_with_pool_key(pool_key)"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"            self._update_member(member, ovn_lb, pool_key)"},{"line_number":1201,"context_line":""},{"line_number":1202,"context_line":"            pool_listeners \u003d self._get_pool_listeners(ovn_lb, pool_key)"},{"line_number":1203,"context_line":"            listener_status \u003d []"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_9e53f26c","line":1200,"in_reply_to":"5fc1f717_a8a0f492","updated":"2019-04-15 09:03:03.000000000","message":"In the base version, based on the status of admin_state_up, the member was being added/removed\nBut yes, we can add the member operating status as the admin_state_up bool value","commit_id":"226cc03502ead4e3f89db7403bfef43bbff68d3d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"83f34ffe6fadf76c93b986b6f63515379c8acaeb","unresolved":false,"context_lines":[{"line_number":1430,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":1431,"context_line":""},{"line_number":1432,"context_line":"    # Member"},{"line_number":1433,"context_line":"    def is_monitor_enabled(self, member):"},{"line_number":1434,"context_line":"        return not (isinstance("},{"line_number":1435,"context_line":"            member.monitor_address, o_datamodels.UnsetType) or isinstance("},{"line_number":1436,"context_line":"                member.monitor_port, o_datamodels.UnsetType))"},{"line_number":1437,"context_line":""},{"line_number":1438,"context_line":"    def member_create(self, member):"},{"line_number":1439,"context_line":"        if self.is_monitor_enabled(member):"}],"source_content_type":"text/x-python","patch_set":27,"id":"ffb9cba7_e26a38eb","line":1436,"range":{"start_line":1433,"start_character":0,"end_line":1436,"end_character":61},"updated":"2019-04-29 09:56:12.000000000","message":"We\u0027re now getting this [1] on the gate. I\u0027m pretty sure we\u0027re not setting monitor_port nor monitor_address, so something seems wrong here.\n\n[1] http://logs.openstack.org/27/643327/14/check/kuryr-kubernetes-e2e-np-containerized-ovn-provider/abfc34e/controller/logs/screen-o-api.txt.gz#_Apr_26_14_18_10_686975","commit_id":"4cd72c50bcb1794453982d7f9114c305d35fa9a2"}]}