)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"3fc716b50fe6758f983b8a818d45dbe811d93e08","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Slawek Kaplonski \u003cskaplons@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-07-08 16:27:04 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Use normalized remote prefix IPs in OVN driver"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"OVN firewall driver can\u0027t silently normalize CIRDs given in"},{"line_number":10,"context_line":"the security group rule\u0027s \"remote_ip_prefix\"."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"bf51134e_e33cb294","line":7,"updated":"2020-07-09 14:12:43.000000000","message":"nit: [ovn] ?","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"4a5c82447647bedbc5e16e9c5b8f866ced8f6a56","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Slawek Kaplonski \u003cskaplons@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-07-08 16:27:04 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Use normalized remote prefix IPs in OVN driver"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"OVN firewall driver can\u0027t silently normalize CIRDs given in"},{"line_number":10,"context_line":"the security group rule\u0027s \"remote_ip_prefix\"."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9f560f44_8b1bb419","line":7,"in_reply_to":"bf51134e_e33cb294","updated":"2020-07-28 13:58:14.000000000","message":"Done","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"3fc716b50fe6758f983b8a818d45dbe811d93e08","unresolved":false,"context_lines":[{"line_number":10,"context_line":"the security group rule\u0027s \"remote_ip_prefix\"."},{"line_number":11,"context_line":"Because of that if user created rule with not normalized CIDR, it"},{"line_number":12,"context_line":"wasn\u0027t applied by the OVN driver."},{"line_number":13,"context_line":"Now OVN driver will normalize such rules before applyinng them."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"The OVN driver will now also check if SG rules with same normalized"},{"line_number":16,"context_line":"and same direction, port range, protocol and ethertype already exists in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"bf51134e_c3bc0e08","line":13,"range":{"start_line":13,"start_character":48,"end_line":13,"end_character":57},"updated":"2020-07-09 14:12:43.000000000","message":"nit: applying","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"4a5c82447647bedbc5e16e9c5b8f866ced8f6a56","unresolved":false,"context_lines":[{"line_number":10,"context_line":"the security group rule\u0027s \"remote_ip_prefix\"."},{"line_number":11,"context_line":"Because of that if user created rule with not normalized CIDR, it"},{"line_number":12,"context_line":"wasn\u0027t applied by the OVN driver."},{"line_number":13,"context_line":"Now OVN driver will normalize such rules before applyinng them."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"The OVN driver will now also check if SG rules with same normalized"},{"line_number":16,"context_line":"and same direction, port range, protocol and ethertype already exists in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9f560f44_2b1ee80b","line":13,"range":{"start_line":13,"start_character":48,"end_line":13,"end_character":57},"in_reply_to":"bf51134e_c3bc0e08","updated":"2020-07-28 13:58:14.000000000","message":"Done","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"1fb69b06c0a1801dbf054667a6504e5f986bd9dd","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":3,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":4,"context_line":"#    a copy of the License at"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_5b6cc107","line":1,"updated":"2020-07-03 00:13:06.000000000","message":"pep8: E902 TokenError: EOF in multi-line statement","commit_id":"d50abd766aa6b7bb72d69221bee4391366b0577f"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"1fb69b06c0a1801dbf054667a6504e5f986bd9dd","unresolved":false,"context_lines":[{"line_number":357,"context_line":"            sg_rule \u003d self._plugin.get_security_group_rule("},{"line_number":358,"context_line":"                kwargs[\u0027context\u0027], kwargs.get(\u0027security_group_rule_id\u0027))"},{"line_number":359,"context_line":"            if sg_rule[\u0027remote_ip_prefix\u0027] is not None:"},{"line_number":360,"context_line":"                if self._group_has_other_rules_with_same_normalized_cidr(sg_rule):"},{"line_number":361,"context_line":"                    return"},{"line_number":362,"context_line":"            # Here I need to get all rules from SG and compare if there is no"},{"line_number":363,"context_line":"            # the same rule after normalize cidrs"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_3b710de2","line":360,"updated":"2020-07-03 00:13:06.000000000","message":"pep8: E501 line too long (82 \u003e 79 characters)","commit_id":"d50abd766aa6b7bb72d69221bee4391366b0577f"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"cf88d97f21022748c27039c6f06154832769a3ed","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import datetime"},{"line_number":18,"context_line":"import functools"},{"line_number":19,"context_line":"import netaddr"},{"line_number":20,"context_line":"import operator"},{"line_number":21,"context_line":"import signal"},{"line_number":22,"context_line":"import threading"},{"line_number":23,"context_line":"import types"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_b4e4d2ad","line":20,"updated":"2020-07-08 02:47:14.000000000","message":"pep8: I100 Import statements are in the wrong order. import operator should be before import netaddr","commit_id":"8ccf6ce34242e4ca1a3d7515d1aca2fc889490cb"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"3fc716b50fe6758f983b8a818d45dbe811d93e08","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        if event \u003d\u003d events.AFTER_CREATE:"},{"line_number":359,"context_line":"            sg_rule \u003d kwargs.get(\u0027security_group_rule\u0027)"},{"line_number":360,"context_line":"            if sg_rule.get(\u0027remote_ip_prefix\u0027) is not None:"},{"line_number":361,"context_line":"                if self._sg_has_rules_with_same_normalized_cidr(sg_rule):"},{"line_number":362,"context_line":"                    return"},{"line_number":363,"context_line":"            self._ovn_client.create_security_group_rule("},{"line_number":364,"context_line":"                kwargs[\u0027context\u0027], kwargs.get(\u0027security_group_rule\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_43a2befd","line":361,"range":{"start_line":361,"start_character":24,"end_line":361,"end_character":63},"updated":"2020-07-09 14:12:43.000000000","message":"I wonder if this has any perf hit as per each SGR added we\u0027ll be querying the DB and pull all the SGRs that belong to that SG and compare our rule with all of them.","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"4a5c82447647bedbc5e16e9c5b8f866ced8f6a56","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        if event \u003d\u003d events.AFTER_CREATE:"},{"line_number":359,"context_line":"            sg_rule \u003d kwargs.get(\u0027security_group_rule\u0027)"},{"line_number":360,"context_line":"            if sg_rule.get(\u0027remote_ip_prefix\u0027) is not None:"},{"line_number":361,"context_line":"                if self._sg_has_rules_with_same_normalized_cidr(sg_rule):"},{"line_number":362,"context_line":"                    return"},{"line_number":363,"context_line":"            self._ovn_client.create_security_group_rule("},{"line_number":364,"context_line":"                kwargs[\u0027context\u0027], kwargs.get(\u0027security_group_rule\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_4bc51c8e","line":361,"range":{"start_line":361,"start_character":24,"end_line":361,"end_character":63},"in_reply_to":"bf51134e_43a2befd","updated":"2020-07-28 13:58:14.000000000","message":"changed","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"74afbfa9ebe96877152e8561ade1885f942d2aa8","unresolved":false,"context_lines":[{"line_number":356,"context_line":"    def _process_sg_rule_notification("},{"line_number":357,"context_line":"            self, resource, event, trigger, **kwargs):"},{"line_number":358,"context_line":"        if event \u003d\u003d events.AFTER_CREATE:"},{"line_number":359,"context_line":"            sg_rule \u003d kwargs.get(\u0027security_group_rule\u0027)"},{"line_number":360,"context_line":"            if sg_rule.get(\u0027remote_ip_prefix\u0027) is not None:"},{"line_number":361,"context_line":"                if self._sg_has_rules_with_same_normalized_cidr(sg_rule):"},{"line_number":362,"context_line":"                    return"},{"line_number":363,"context_line":"            self._ovn_client.create_security_group_rule("},{"line_number":364,"context_line":"                kwargs[\u0027context\u0027], kwargs.get(\u0027security_group_rule\u0027))"},{"line_number":365,"context_line":"        elif event \u003d\u003d events.BEFORE_DELETE:"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_74fcddb4","line":362,"range":{"start_line":359,"start_character":0,"end_line":362,"end_character":26},"updated":"2020-07-09 10:56:53.000000000","message":"nit: we could have this check outside the ifs conditions since we only invoke this method for these two events anyway (AFTER_CREATE, BEFORE_DELETE).\n\n[0] https://review.opendev.org/#/c/736386/4/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py@203","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"284055e85232d508e44de196963c3aac12cff838","unresolved":false,"context_lines":[{"line_number":356,"context_line":"    def _process_sg_rule_notification("},{"line_number":357,"context_line":"            self, resource, event, trigger, **kwargs):"},{"line_number":358,"context_line":"        if event \u003d\u003d events.AFTER_CREATE:"},{"line_number":359,"context_line":"            sg_rule \u003d kwargs.get(\u0027security_group_rule\u0027)"},{"line_number":360,"context_line":"            if sg_rule.get(\u0027remote_ip_prefix\u0027) is not None:"},{"line_number":361,"context_line":"                if self._sg_has_rules_with_same_normalized_cidr(sg_rule):"},{"line_number":362,"context_line":"                    return"},{"line_number":363,"context_line":"            self._ovn_client.create_security_group_rule("},{"line_number":364,"context_line":"                kwargs[\u0027context\u0027], kwargs.get(\u0027security_group_rule\u0027))"},{"line_number":365,"context_line":"        elif event \u003d\u003d events.BEFORE_DELETE:"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_40963440","line":362,"range":{"start_line":359,"start_character":0,"end_line":362,"end_character":26},"in_reply_to":"bf51134e_74fcddb4","updated":"2020-07-09 13:40:47.000000000","message":"yes, but that wouldn\u0027t change anything in fact. In case of BEFORE_DELETE, there is no full rule given never in kwargs so it will always need to retrieve it from db in such case.","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"3fc716b50fe6758f983b8a818d45dbe811d93e08","unresolved":false,"context_lines":[{"line_number":372,"context_line":"                kwargs[\u0027context\u0027],"},{"line_number":373,"context_line":"                sg_rule)"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"    def _sg_has_rules_with_same_normalized_cidr(self, sg_rule):"},{"line_number":376,"context_line":"        compare_keys \u003d ["},{"line_number":377,"context_line":"            \u0027ethertype\u0027, \u0027direction\u0027, \u0027protocol\u0027,"},{"line_number":378,"context_line":"            \u0027port_range_min\u0027, \u0027port_range_max\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf51134e_e3895275","line":375,"range":{"start_line":375,"start_character":54,"end_line":375,"end_character":61},"updated":"2020-07-09 14:12:43.000000000","message":"nit: the footprint is a bit confusing maybe. You want to determine of the SG has rules with same normalized cidr and you pass in a sg_rule. Maybe it\u0027s clearer if you pass in the sg_id and the remote_ip_prefix ?","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"b365897009db410768129a9994bebd31523ebe12","unresolved":false,"context_lines":[{"line_number":372,"context_line":"                kwargs[\u0027context\u0027],"},{"line_number":373,"context_line":"                sg_rule)"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"    def _sg_has_rules_with_same_normalized_cidr(self, sg_rule):"},{"line_number":376,"context_line":"        compare_keys \u003d ["},{"line_number":377,"context_line":"            \u0027ethertype\u0027, \u0027direction\u0027, \u0027protocol\u0027,"},{"line_number":378,"context_line":"            \u0027port_range_min\u0027, \u0027port_range_max\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_26d73fc2","line":375,"range":{"start_line":375,"start_character":54,"end_line":375,"end_character":61},"in_reply_to":"9f560f44_cb2dac47","updated":"2020-07-28 14:02:07.000000000","message":"gotcha! Thanks :)","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"4a5c82447647bedbc5e16e9c5b8f866ced8f6a56","unresolved":false,"context_lines":[{"line_number":372,"context_line":"                kwargs[\u0027context\u0027],"},{"line_number":373,"context_line":"                sg_rule)"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"    def _sg_has_rules_with_same_normalized_cidr(self, sg_rule):"},{"line_number":376,"context_line":"        compare_keys \u003d ["},{"line_number":377,"context_line":"            \u0027ethertype\u0027, \u0027direction\u0027, \u0027protocol\u0027,"},{"line_number":378,"context_line":"            \u0027port_range_min\u0027, \u0027port_range_max\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_cb2dac47","line":375,"range":{"start_line":375,"start_character":54,"end_line":375,"end_character":61},"in_reply_to":"bf51134e_e3895275","updated":"2020-07-28 13:58:14.000000000","message":"In fact I need sg_rule to be passed here as I need to compare also other keys, like direction, port range, protocol, etc. if normalized_cidr is the same.","commit_id":"bbe1b3bc8431ba439243ffec578459312f79ae3a"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"dff1214930a12017a7d4754cd7fe436155bc9895","unresolved":false,"context_lines":[{"line_number":361,"context_line":"        elif event \u003d\u003d events.BEFORE_DELETE:"},{"line_number":362,"context_line":"            sg_rule \u003d self._plugin.get_security_group_rule("},{"line_number":363,"context_line":"                kwargs[\u0027context\u0027], kwargs.get(\u0027security_group_rule_id\u0027))"},{"line_number":364,"context_line":"            if sg_rule.get(\u0027remote_ip_prefix\u0027) is not None:"},{"line_number":365,"context_line":"                if self._sg_has_rules_with_same_normalized_cidr(sg_rule):"},{"line_number":366,"context_line":"                    return"},{"line_number":367,"context_line":"            self._ovn_client.delete_security_group_rule("}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_4814fdf5","line":364,"updated":"2020-07-28 16:23:57.000000000","message":"I realize that this is one of the stated goals of the patch, but if we are trying to delete a rule, and the SG has rules that have different, but equivalent remote_ip_prefix, why don\u0027t we want to go ahead and delete it? Am I reading that right?","commit_id":"326058a295e62dc642531ac545d43861c26af543"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ab8b376c6a95ece2397caf1f68fecda1d9e6c3b3","unresolved":false,"context_lines":[{"line_number":361,"context_line":"        elif event \u003d\u003d events.BEFORE_DELETE:"},{"line_number":362,"context_line":"            sg_rule \u003d self._plugin.get_security_group_rule("},{"line_number":363,"context_line":"                kwargs[\u0027context\u0027], kwargs.get(\u0027security_group_rule_id\u0027))"},{"line_number":364,"context_line":"            if sg_rule.get(\u0027remote_ip_prefix\u0027) is not None:"},{"line_number":365,"context_line":"                if self._sg_has_rules_with_same_normalized_cidr(sg_rule):"},{"line_number":366,"context_line":"                    return"},{"line_number":367,"context_line":"            self._ovn_client.delete_security_group_rule("}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_df771d3a","line":364,"in_reply_to":"9f560f44_4814fdf5","updated":"2020-07-28 21:20:01.000000000","message":"imagine the case when user adds 2 rules to SG, one with e.g. cidr 10.10.10.175/26 and second with same attributes, like e.g. protocol, ethertype and others but with cidr 10.10.10.128/26. After this patch OF rules for those 2 rules will effectively be added only once on the host and when one of them will be removed, Neutron will build acl for this rule and will remove flows. So without this check it would remove OF for both those rules and there would be \"mismatch\" between what is in Neutron DB and on the backend as there would be rule to allow traffic from e.g. 10.10.10.128/26 in db but it wouldn\u0027t be on the compute.\n\nSo that\u0027s why there is this check here.","commit_id":"326058a295e62dc642531ac545d43861c26af543"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"cf620271156c55620509a5d8e7ef610e97db74ba","unresolved":false,"context_lines":[{"line_number":361,"context_line":"        elif event \u003d\u003d events.BEFORE_DELETE:"},{"line_number":362,"context_line":"            sg_rule \u003d self._plugin.get_security_group_rule("},{"line_number":363,"context_line":"                kwargs[\u0027context\u0027], kwargs.get(\u0027security_group_rule_id\u0027))"},{"line_number":364,"context_line":"            if sg_rule.get(\u0027remote_ip_prefix\u0027) is not None:"},{"line_number":365,"context_line":"                if self._sg_has_rules_with_same_normalized_cidr(sg_rule):"},{"line_number":366,"context_line":"                    return"},{"line_number":367,"context_line":"            self._ovn_client.delete_security_group_rule("}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_cedda343","line":364,"in_reply_to":"9f560f44_df771d3a","updated":"2020-07-29 14:08:36.000000000","message":"Ok, got it. Thanks! I\u0027d like to put my vote in for neutron not allowing the creation of *both* rules. I get that for backward compatibility we can\u0027t just do the no host bits thing. But I don\u0027t see any good reason to allow the creation of 2 identical rules.","commit_id":"326058a295e62dc642531ac545d43861c26af543"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"dff1214930a12017a7d4754cd7fe436155bc9895","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                                               cidr_sg_rule.prefixlen)"},{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        def _rules_equal(rule1, rule2):"},{"line_number":383,"context_line":"            for key in compare_keys:"},{"line_number":384,"context_line":"                if rule1.get(key) !\u003d rule2.get(key):"},{"line_number":385,"context_line":"                    return False"},{"line_number":386,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_65b80c76","line":383,"updated":"2020-07-28 16:23:57.000000000","message":"nit:\n\nreturn not any(rule1.get(key) !\u003d rule2.get(key) for key in compare_keys)","commit_id":"326058a295e62dc642531ac545d43861c26af543"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ab8b376c6a95ece2397caf1f68fecda1d9e6c3b3","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                                               cidr_sg_rule.prefixlen)"},{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        def _rules_equal(rule1, rule2):"},{"line_number":383,"context_line":"            for key in compare_keys:"},{"line_number":384,"context_line":"                if rule1.get(key) !\u003d rule2.get(key):"},{"line_number":385,"context_line":"                    return False"},{"line_number":386,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_bfa9897c","line":383,"in_reply_to":"9f560f44_65b80c76","updated":"2020-07-28 21:20:01.000000000","message":"Done","commit_id":"326058a295e62dc642531ac545d43861c26af543"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"dff1214930a12017a7d4754cd7fe436155bc9895","unresolved":false,"context_lines":[{"line_number":386,"context_line":"            return True"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"        for rule in sg_rules:"},{"line_number":389,"context_line":"            if not rule.get(\u0027remote_ip_prefix\u0027) or rule[\u0027id\u0027] \u003d\u003d sg_rule[\u0027id\u0027]:"},{"line_number":390,"context_line":"                continue"},{"line_number":391,"context_line":"            cidr_rule \u003d netaddr.IPNetwork(rule[\u0027remote_ip_prefix\u0027])"},{"line_number":392,"context_line":"            normalized_rule_prefix \u003d \"%s/%s\" % (cidr_rule.network,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_c84c8dc9","line":389,"range":{"start_line":389,"start_character":19,"end_line":389,"end_character":47},"updated":"2020-07-28 16:23:57.000000000","message":"*super picky nit that I don\u0027t even know why I\u0027m writing*\n\nif we don\u0027t need the return value, can just do:\n\n if not \u0027remote_ip_prefix\u0027 in rule ...:","commit_id":"326058a295e62dc642531ac545d43861c26af543"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"cf620271156c55620509a5d8e7ef610e97db74ba","unresolved":false,"context_lines":[{"line_number":386,"context_line":"            return True"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"        for rule in sg_rules:"},{"line_number":389,"context_line":"            if not rule.get(\u0027remote_ip_prefix\u0027) or rule[\u0027id\u0027] \u003d\u003d sg_rule[\u0027id\u0027]:"},{"line_number":390,"context_line":"                continue"},{"line_number":391,"context_line":"            cidr_rule \u003d netaddr.IPNetwork(rule[\u0027remote_ip_prefix\u0027])"},{"line_number":392,"context_line":"            normalized_rule_prefix \u003d \"%s/%s\" % (cidr_rule.network,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_4ef193cd","line":389,"range":{"start_line":389,"start_character":19,"end_line":389,"end_character":47},"in_reply_to":"9f560f44_1fb875c6","updated":"2020-07-29 14:08:36.000000000","message":"ack","commit_id":"326058a295e62dc642531ac545d43861c26af543"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ab8b376c6a95ece2397caf1f68fecda1d9e6c3b3","unresolved":false,"context_lines":[{"line_number":386,"context_line":"            return True"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"        for rule in sg_rules:"},{"line_number":389,"context_line":"            if not rule.get(\u0027remote_ip_prefix\u0027) or rule[\u0027id\u0027] \u003d\u003d sg_rule[\u0027id\u0027]:"},{"line_number":390,"context_line":"                continue"},{"line_number":391,"context_line":"            cidr_rule \u003d netaddr.IPNetwork(rule[\u0027remote_ip_prefix\u0027])"},{"line_number":392,"context_line":"            normalized_rule_prefix \u003d \"%s/%s\" % (cidr_rule.network,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_1fb875c6","line":389,"range":{"start_line":389,"start_character":19,"end_line":389,"end_character":47},"in_reply_to":"9f560f44_c84c8dc9","updated":"2020-07-28 21:20:01.000000000","message":"value is kind of important because it may happen that rule has \u0027remote_ip_prefix\u0027 but with value None or empty string and in such case we don\u0027t need to process it more.","commit_id":"326058a295e62dc642531ac545d43861c26af543"}],"neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"cf88d97f21022748c27039c6f06154832769a3ed","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                self.mech_driver,"},{"line_number":188,"context_line":"                \u0027_sg_has_rules_with_same_normalized_cidr\u0027) as has_same_rules, \\"},{"line_number":189,"context_line":"                mock.patch.object(ovn_acl, \u0027update_acls_for_security_group\u0027) \\"},{"line_number":190,"context_line":"                    as ovn_acl_up:"},{"line_number":191,"context_line":"            rule \u003d {\u0027security_group_id\u0027: \u0027sg_id\u0027}"},{"line_number":192,"context_line":"            self.mech_driver._process_sg_rule_notification("},{"line_number":193,"context_line":"                resources.SECURITY_GROUP_RULE, events.AFTER_CREATE, {},"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_34d1e2ce","line":190,"updated":"2020-07-08 02:47:14.000000000","message":"pep8: E131 continuation line unaligned for hanging indent","commit_id":"8ccf6ce34242e4ca1a3d7515d1aca2fc889490cb"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"cf88d97f21022748c27039c6f06154832769a3ed","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                self.mech_driver,"},{"line_number":207,"context_line":"                \u0027_sg_has_rules_with_same_normalized_cidr\u0027) as has_same_rules, \\"},{"line_number":208,"context_line":"                mock.patch.object(ovn_acl, \u0027update_acls_for_security_group\u0027) \\"},{"line_number":209,"context_line":"                    as ovn_acl_up:"},{"line_number":210,"context_line":"            rule \u003d {\u0027security_group_id\u0027: \u0027sg_id\u0027,"},{"line_number":211,"context_line":"                    \u0027remote_ip_prefix\u0027: \u00271.0.0.0/24\u0027}"},{"line_number":212,"context_line":"            has_same_rules.return_value \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_14d49ebc","line":209,"updated":"2020-07-08 02:47:14.000000000","message":"pep8: E131 continuation line unaligned for hanging indent","commit_id":"8ccf6ce34242e4ca1a3d7515d1aca2fc889490cb"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"cf88d97f21022748c27039c6f06154832769a3ed","unresolved":false,"context_lines":[{"line_number":227,"context_line":"                self.mech_driver,"},{"line_number":228,"context_line":"                \u0027_sg_has_rules_with_same_normalized_cidr\u0027) as has_same_rules, \\"},{"line_number":229,"context_line":"                mock.patch.object(ovn_acl, \u0027update_acls_for_security_group\u0027) \\"},{"line_number":230,"context_line":"                    as ovn_acl_up:"},{"line_number":231,"context_line":"            rule \u003d {\u0027security_group_id\u0027: \u0027sg_id\u0027,"},{"line_number":232,"context_line":"                    \u0027remote_ip_prefix\u0027: \u00271.0.0.0/24\u0027}"},{"line_number":233,"context_line":"            has_same_rules.return_value \u003d True"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_74da3ae9","line":230,"updated":"2020-07-08 02:47:14.000000000","message":"pep8: E131 continuation line unaligned for hanging indent","commit_id":"8ccf6ce34242e4ca1a3d7515d1aca2fc889490cb"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"cf88d97f21022748c27039c6f06154832769a3ed","unresolved":false,"context_lines":[{"line_number":285,"context_line":"              \u0027protocol\u0027: \u0027tcp\u0027,"},{"line_number":286,"context_line":"              \u0027port_range_min\u0027: \u00272000\u0027, \u0027port_range_max\u0027: \u00273000\u0027,"},{"line_number":287,"context_line":"              \u0027direction\u0027: \u0027ingress\u0027}, True)]"},{"line_number":288,"context_line":"        "},{"line_number":289,"context_line":"        rules \u003d ["},{"line_number":290,"context_line":"            {"},{"line_number":291,"context_line":"                \u0027id\u0027: \u0027rule-1-id\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_54d576b6","line":288,"updated":"2020-07-08 02:47:14.000000000","message":"pep8: W293 blank line contains whitespace","commit_id":"8ccf6ce34242e4ca1a3d7515d1aca2fc889490cb"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"dff1214930a12017a7d4754cd7fe436155bc9895","unresolved":false,"context_lines":[{"line_number":200,"context_line":"                mock.ANY, rule, ovn_const.TYPE_SECURITY_GROUP_RULES)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    @mock.patch.object(ovn_revision_numbers_db, \u0027bump_revision\u0027)"},{"line_number":203,"context_line":"    def test__process_sg_rule_notifications_sgr_create_with_remote_ip_prefix("},{"line_number":204,"context_line":"            self, mock_bump):"},{"line_number":205,"context_line":"        with mock.patch.object("},{"line_number":206,"context_line":"                self.mech_driver,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_28b8a9c6","line":203,"updated":"2020-07-28 16:23:57.000000000","message":"These two tests differ in only one line, seems like a helper function could be used.\n\nThis might just be because I write far more functional tests than I do unit tests and so don\u0027t use mock a whole lot, but I\u0027m confused about that one line that changes:\n\n _has_same_rules.return_value \u003d False/True\n\nthen followed by\n\n has_same_rules.assert_not_called()\n\nIf the only difference is the return value of has_same_rules, and we are asserting that it isn\u0027t called in both tests, how does that work?","commit_id":"326058a295e62dc642531ac545d43861c26af543"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ab8b376c6a95ece2397caf1f68fecda1d9e6c3b3","unresolved":false,"context_lines":[{"line_number":200,"context_line":"                mock.ANY, rule, ovn_const.TYPE_SECURITY_GROUP_RULES)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    @mock.patch.object(ovn_revision_numbers_db, \u0027bump_revision\u0027)"},{"line_number":203,"context_line":"    def test__process_sg_rule_notifications_sgr_create_with_remote_ip_prefix("},{"line_number":204,"context_line":"            self, mock_bump):"},{"line_number":205,"context_line":"        with mock.patch.object("},{"line_number":206,"context_line":"                self.mech_driver,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_9aae53e5","line":203,"in_reply_to":"9f560f44_28b8a9c6","updated":"2020-07-28 21:20:01.000000000","message":"Those tests were added in previous PS where I was checking for duplicate rules during create sg rule events (in same way as it\u0027s done during deletion).\nBut as Daniel told me, it\u0027s not needed when \"may_exist\u003dTrue\" is used so I don\u0027t need 2 tests here. One is enough.","commit_id":"326058a295e62dc642531ac545d43861c26af543"}]}