)]}'
{"openstack_dashboard/dashboards/project/security_groups/forms.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"71036618223c9a0b405d1ff69b29e46e401efaa1","unresolved":false,"context_lines":[{"line_number":273,"context_line":"        custom_rules \u003d [(\u0027tcp\u0027, _(\u0027Custom TCP Rule\u0027)),"},{"line_number":274,"context_line":"                        (\u0027udp\u0027, _(\u0027Custom UDP Rule\u0027)),"},{"line_number":275,"context_line":"                        (\u0027icmp\u0027, _(\u0027Custom ICMP Rule\u0027)),"},{"line_number":276,"context_line":"                        (\u0027icmpv6\u0027, _(\u0027Custom ICMPV6 Rule\u0027)),"},{"line_number":277,"context_line":"                        (\u0027custom\u0027, _(\u0027Other Protocol\u0027))]"},{"line_number":278,"context_line":"        self.fields[\u0027rule_menu\u0027].choices \u003d custom_rules + common_rules"},{"line_number":279,"context_line":"        self.rules \u003d rules_dict"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f231b9d_821729a4","line":276,"range":{"start_line":276,"start_character":45,"end_line":276,"end_character":51},"updated":"2017-06-13 15:27:35.000000000","message":"The usual spelling is \u0027ICMPv6\u0027 (\u0027v\u0027 is lower)","commit_id":"117c4c7cc3b72a0a408e5756a0d7cdd46238683c"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"71036618223c9a0b405d1ff69b29e46e401efaa1","unresolved":false,"context_lines":[{"line_number":373,"context_line":""},{"line_number":374,"context_line":"    def _clean_rule_menu(self, cleaned_data):"},{"line_number":375,"context_line":"        rule_menu \u003d cleaned_data.get(\u0027rule_menu\u0027)"},{"line_number":376,"context_line":"        if rule_menu in (\u0027icmp\u0027, \u0027icmpv6\u0027):"},{"line_number":377,"context_line":"            self._clean_rule_icmp(cleaned_data, rule_menu)"},{"line_number":378,"context_line":"        elif rule_menu \u003d\u003d \u0027tcp\u0027 or rule_menu \u003d\u003d \u0027udp\u0027:"},{"line_number":379,"context_line":"            self._clean_rule_tcp_udp(cleaned_data, rule_menu)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f231b9d_226df531","line":376,"range":{"start_line":376,"start_character":34,"end_line":376,"end_character":40},"updated":"2017-06-13 15:27:35.000000000","message":"In Neutron API, ipv6-icmp is preferred for ICMPv6. \"icmpv6\" is a legacy though we don\u0027t have a plan to drop it soon, but for a new code, I would suggest to use \u0027ipv6-icmp\u0027.","commit_id":"117c4c7cc3b72a0a408e5756a0d7cdd46238683c"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"043920c68515a0b9db1536d319220d7c825667ee","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                # version. It is used only when Neutron is enabled."},{"line_number":409,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":410,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":411,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":412,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_1790e142","line":411,"range":{"start_line":411,"start_character":24,"end_line":411,"end_character":56},"updated":"2017-06-14 09:10:54.000000000","message":"Is there a case where cleaned_data[\u0027ip_protocol\u0027] is not a string?","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"f46256971a1d9539358002cc0a1e9cc23f93ef1f","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                # version. It is used only when Neutron is enabled."},{"line_number":409,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":410,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":411,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":412,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_d714a950","line":411,"range":{"start_line":411,"start_character":24,"end_line":411,"end_character":56},"in_reply_to":"7f231b9d_1790e142","updated":"2017-06-14 09:26:56.000000000","message":"Yes, see the failed test case on the previuos patch.","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"a991500d5e59ebd14dff6f1cc5919a78e698d695","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                # version. It is used only when Neutron is enabled."},{"line_number":409,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":410,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":411,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":412,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_5a50c05d","line":411,"range":{"start_line":411,"start_character":24,"end_line":411,"end_character":56},"in_reply_to":"7f231b9d_d714a950","updated":"2017-06-14 09:54:23.000000000","message":"I see. ip_protocol is Integer field, so perhaps the cleaned data is an integer. Thanks.","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"043920c68515a0b9db1536d319220d7c825667ee","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":410,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":411,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":412,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"},{"line_number":415,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_f7bb25c1","line":412,"range":{"start_line":412,"start_character":28,"end_line":412,"end_character":41},"updated":"2017-06-14 09:10:54.000000000","message":"This is a constant, so I think tuple() is enough. This is just my preference.","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"a991500d5e59ebd14dff6f1cc5919a78e698d695","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":410,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":411,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":412,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"},{"line_number":415,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_3aa74452","line":412,"range":{"start_line":412,"start_character":28,"end_line":412,"end_character":41},"in_reply_to":"7f231b9d_1a292889","updated":"2017-06-14 09:54:23.000000000","message":"I didn\u0027t know set is faster.\n\nsemantically I am not sure what is correct as it is a constant and developers take care of the values.\n\nI am okay with it.","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"f46256971a1d9539358002cc0a1e9cc23f93ef1f","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":410,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":411,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":412,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"},{"line_number":415,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_1a292889","line":412,"range":{"start_line":412,"start_character":28,"end_line":412,"end_character":41},"in_reply_to":"7f231b9d_f7bb25c1","updated":"2017-06-14 09:26:56.000000000","message":"Set is faster and more correct semantically.","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0a3201abbf041e92f80fd6b35657197755818320","unresolved":false,"context_lines":[{"line_number":410,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":411,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":412,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"},{"line_number":415,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"},{"line_number":416,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_40174ba5","line":413,"updated":"2019-05-29 18:10:46.000000000","message":"This is fine, but in an almost-merged change to neutron master branch, \u0027icmp\u0027, \u0027icmpv6\u0027 and \u0027ipv6-icmp\u0027 are all accepted for IPv6, and will result in a rule with \u0027ipv6-icmp\u0027 as the protocol.  We decide to standardize on that to not cause confusion.","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"043920c68515a0b9db1536d319220d7c825667ee","unresolved":false,"context_lines":[{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"},{"line_number":415,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"},{"line_number":416,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":417,"context_line":"                            {\u0027ipv6-icmp\u0027, \u0027icmpv6\u0027, \u002758\u0027}):"},{"line_number":418,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027icmp\u0027"},{"line_number":419,"context_line":"        return cleaned_data"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    def handle(self, request, data):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_7779552d","line":418,"range":{"start_line":416,"start_character":20,"end_line":418,"end_character":60},"updated":"2017-06-14 09:10:54.000000000","message":"I don\u0027t think this conversion is needed.","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"f46256971a1d9539358002cc0a1e9cc23f93ef1f","unresolved":false,"context_lines":[{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"},{"line_number":415,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"},{"line_number":416,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":417,"context_line":"                            {\u0027ipv6-icmp\u0027, \u0027icmpv6\u0027, \u002758\u0027}):"},{"line_number":418,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027icmp\u0027"},{"line_number":419,"context_line":"        return cleaned_data"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    def handle(self, request, data):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_9a41b83c","line":418,"range":{"start_line":416,"start_character":20,"end_line":418,"end_character":60},"in_reply_to":"7f231b9d_7779552d","updated":"2017-06-14 09:26:56.000000000","message":"There is more than one way to add an icmp rule, not just the \"all icmp\" option in horizon. I think it\u0027s nice to be consistent and always do the right thing.\n\n(I also think that this should actually be handled on the Neutron side, but that\u0027s beside the point.)","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0a3201abbf041e92f80fd6b35657197755818320","unresolved":false,"context_lines":[{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"},{"line_number":415,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"},{"line_number":416,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":417,"context_line":"                            {\u0027ipv6-icmp\u0027, \u0027icmpv6\u0027, \u002758\u0027}):"},{"line_number":418,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027icmp\u0027"},{"line_number":419,"context_line":"        return cleaned_data"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    def handle(self, request, data):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_000dd38e","line":418,"range":{"start_line":416,"start_character":20,"end_line":418,"end_character":60},"in_reply_to":"7f231b9d_8df980c9","updated":"2019-05-29 18:10:46.000000000","message":"For IPv4, only \u0027icmp\u0027 is valid, the others aren\u0027t, and IMHO should raise an error as they are invalid.","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"a991500d5e59ebd14dff6f1cc5919a78e698d695","unresolved":false,"context_lines":[{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"},{"line_number":415,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"},{"line_number":416,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":417,"context_line":"                            {\u0027ipv6-icmp\u0027, \u0027icmpv6\u0027, \u002758\u0027}):"},{"line_number":418,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027icmp\u0027"},{"line_number":419,"context_line":"        return cleaned_data"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    def handle(self, request, data):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_fa4f8c8f","line":418,"range":{"start_line":416,"start_character":20,"end_line":418,"end_character":60},"in_reply_to":"7f231b9d_9a41b83c","updated":"2017-06-14 09:54:23.000000000","message":"I haven\u0027t understood what is consistent.\n\nIf a user specify some IPv6 related protocol like ipv6-icmp, icmpv6, and 58, I wonder the user tries to create IPv6 rule. There may be a case where the user just forget to set IPv6 CIDR rather than specifying IPv4 CIDR before clicking the submit button.\n\nIMHO it is better to show an error message that the protocol and IP version do not match.","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"4d2223f630adfb60b3513e9b7f1c970bb80e8c67","unresolved":false,"context_lines":[{"line_number":413,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":414,"context_line":"                else:"},{"line_number":415,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"},{"line_number":416,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":417,"context_line":"                            {\u0027ipv6-icmp\u0027, \u0027icmpv6\u0027, \u002758\u0027}):"},{"line_number":418,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027icmp\u0027"},{"line_number":419,"context_line":"        return cleaned_data"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    def handle(self, request, data):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_8df980c9","line":418,"range":{"start_line":416,"start_character":20,"end_line":418,"end_character":60},"in_reply_to":"7f231b9d_fa4f8c8f","updated":"2017-06-14 10:11:50.000000000","message":"But that\u0027s inconsistent with what we do in the IPv6 case then. The way I did it, it\u0027s symmetric -- no matter which kind of icmp you specify, it will always be changed to the correct one for the CIDR you specified. Why should IPv4 icmp be \"special\" and be automatically changed to ipv6-icmp, and not the other way around? Why should one raise and error, and not the other?","commit_id":"548286d0a55310fc20751e787a16131f67aff3b2"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"8e7bc3fdf56c3d3c7c6a31695edac1b16c7ee2c7","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":433,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":434,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":435,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":436,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":437,"context_line":"                else:"},{"line_number":438,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_6168e6f4","line":435,"range":{"start_line":435,"start_character":37,"end_line":435,"end_character":40},"updated":"2019-07-08 09:27:45.000000000","message":"question: Would we like to convert a numeric IP protocol into a symbol? We don\u0027t do this for other protocol like tcp,  so I wonder we don\u0027t need to do this for consistency.","commit_id":"36dcd5ab2bb1740272f96c6a1a8cd5eb9a070665"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"16be8fb1ab35b132c6936ae69cc37770b0bc878c","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":433,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":434,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":435,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":436,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":437,"context_line":"                else:"},{"line_number":438,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_41869515","line":435,"range":{"start_line":435,"start_character":37,"end_line":435,"end_character":40},"in_reply_to":"7faddb67_1986e967","updated":"2019-07-15 16:25:46.000000000","message":"We could easily do this in a way that did not change from string form to int form.  For example:\n\n    if netaddr.IPNetwork(cidr).version \u003d\u003d 6:\n      if (str(cleaned_data[\u0027ip_protocol\u0027]) \u003d\u003d \u00271\u0027:\n        cleaned_data[\u0027ip_protocol\u0027] \u003d 58\n      elif str(cleaned_data[\u0027ip_protocol\u0027]).lower() \u003d\u003d \u0027icmp\u0027:\n        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027icmpv6\u0027\n\nWould that alleviate your concern?  I don\u0027t see the need for a general text-to-int conversion table, but if we do decide we need one here perhaps we can leverage the socket library instead of neutron-lib.\n\n    import socket\n    table \u003d {num:name[8:] for name,num \n             in vars(socket).items() \n             if name.startswith(\"IPPROTO\")}\n    \nThe contents of \u0027table\u0027 are now:\n\n{0: \u0027HOPOPTS\u0027,\n 1: \u0027ICMP\u0027,\n 2: \u0027IGMP\u0027,\n 4: \u0027IPIP\u0027,\n 6: \u0027TCP\u0027,\n 8: \u0027EGP\u0027,\n 12: \u0027PUP\u0027,\n 17: \u0027UDP\u0027,\n 22: \u0027IDP\u0027,\n 29: \u0027TP\u0027,\n 41: \u0027IPV6\u0027,\n 43: \u0027ROUTING\u0027,\n 44: \u0027FRAGMENT\u0027,\n 46: \u0027RSVP\u0027,\n 47: \u0027GRE\u0027,\n 50: \u0027ESP\u0027,\n 51: \u0027AH\u0027,\n 58: \u0027ICMPV6\u0027,\n 59: \u0027NONE\u0027,\n 60: \u0027DSTOPTS\u0027,\n 103: \u0027PIM\u0027,\n 255: \u0027RAW\u0027}\n \n\nAs a general rule I think we should definitely not try autoconverting for all protocols.  I say this because there is an unassigned range and (outside this ICMP use case) someone might need to assert a rule for a numeric IP protocol that has no text representation, because it was invented by some vendor.","commit_id":"36dcd5ab2bb1740272f96c6a1a8cd5eb9a070665"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"307341770c954efab0e347409d09b8c1e70b8c9f","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":433,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":434,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":435,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":436,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":437,"context_line":"                else:"},{"line_number":438,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_4f93105c","line":435,"range":{"start_line":435,"start_character":37,"end_line":435,"end_character":40},"in_reply_to":"7faddb67_1df8f546","updated":"2019-07-16 13:44:13.000000000","message":"BTW, don\u0027t we need to move the logic added here outside of the if clause? The new logic is also needed for remote group case.","commit_id":"36dcd5ab2bb1740272f96c6a1a8cd5eb9a070665"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"acf5b1ee96575d29a21dbb00bd0b8404ed2c1516","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":433,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":434,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":435,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":436,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":437,"context_line":"                else:"},{"line_number":438,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_f5af3be0","line":435,"range":{"start_line":435,"start_character":37,"end_line":435,"end_character":40},"in_reply_to":"7faddb67_41869515","updated":"2019-07-15 19:46:06.000000000","message":"I\u0027m fine with Nate\u0027s suggestion, and agree we should only do it for ICMP as it\u0027s a special case that we\u0027ve had to work around in the Neutron API.  We just need to make sure we use the correct values:\n\n\u00271\u0027/\u0027icmp\u0027 for IPv4\n\u002758\u0027/\u0027ipv6-icmp\u0027 for IPv6\n\nThe Neutron API can handle mismatches, but might as well do it right.","commit_id":"36dcd5ab2bb1740272f96c6a1a8cd5eb9a070665"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"23cbea6935008b71b924fd3c7e4acafe3a29752b","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":433,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":434,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":435,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":436,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":437,"context_line":"                else:"},{"line_number":438,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_feb94c14","line":435,"range":{"start_line":435,"start_character":37,"end_line":435,"end_character":40},"in_reply_to":"7faddb67_4f93105c","updated":"2019-07-16 17:38:00.000000000","message":"After looking at the implementation of this form in detail, the following cases should happen:\n\n- When \"Custom ICMP Rule\" or \"All ICMP\" is selected, \u0027icmp\u0027 is populated as ip_protocol here, so it needs to be converted into \u0027ipv6-icmp\u0027 if IPv6. (This is required to support both of IPv4 and IPv6 in a same menu.)\n- When \"Other protocol\" is selected, a user can specify an integer value as ip_protocol. In this case, a warning to users looks better rather than converting it automatically.\n\nNote that a user cannot specify string representations as ip_protocol in the current horizon implementation. This is different from the neutron API behavior. Someone can improve the horizon implementation future but this never happens now.","commit_id":"36dcd5ab2bb1740272f96c6a1a8cd5eb9a070665"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"2f23318912093c7f866aa8d12d4a2496f22527a1","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":433,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":434,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":435,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":436,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":437,"context_line":"                else:"},{"line_number":438,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_870fd935","line":435,"range":{"start_line":435,"start_character":37,"end_line":435,"end_character":40},"in_reply_to":"7faddb67_6168e6f4","updated":"2019-07-12 15:57:47.000000000","message":"Akihiro, when you said \"convert into a symbol\" did you mean make a constant for it?  I am unfamiliar with \"symbol\" in the Python context.\n\nIf you did mean \"make a constant\" then I agree it\u0027s unnecessary.","commit_id":"36dcd5ab2bb1740272f96c6a1a8cd5eb9a070665"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e015ac997af64e3849173ec650d81141f62c81dc","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":433,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":434,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":435,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":436,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":437,"context_line":"                else:"},{"line_number":438,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_1c07e684","line":435,"range":{"start_line":435,"start_character":37,"end_line":435,"end_character":40},"in_reply_to":"7faddb67_6168e6f4","updated":"2019-07-10 18:57:06.000000000","message":"So I think in essence you\u0027re saying we don\u0027t need this \"cleaning\"?  If so I would agree with that, I don\u0027t think it\u0027s required just seems to be trying to make a better user experience by not raising an error.\n\nDepending on that we can then add tests, since otherwise an invalid call will just get cleaned.","commit_id":"36dcd5ab2bb1740272f96c6a1a8cd5eb9a070665"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"8179be66215256d68043fed730e41a22c6934476","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":433,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":434,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":435,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":436,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":437,"context_line":"                else:"},{"line_number":438,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_1986e967","line":435,"range":{"start_line":435,"start_character":37,"end_line":435,"end_character":40},"in_reply_to":"7faddb67_870fd935","updated":"2019-07-13 12:37:51.000000000","message":"Thanks Brain and Nate.\n\n\"symbol\" I mean is a string representation like \"tcp\", \"icmp\". In other word, \"string\" (tcp) vs \"integer\" (6).\n\nThis patch introduces protocol number auto-conversion to string representation only for \u0027icmp\u0027. If we introduce it only for icmp, it will bring inconsistency. This is my concern.\n\nI see two routes on this:\n(1) to introduce auto-conversion for all known IP protocol.  (I am not sure it is a good idea to have a copy of neutron-lib constants for this conversion.)\n(2) not to convert protocol numbers to strings.\n\nIf we go to (1), this patch can go as-is and other stuffs can be added as a follow-up patch. If (2), the conversion in this patch needs to be dropped.\n\nThought?","commit_id":"36dcd5ab2bb1740272f96c6a1a8cd5eb9a070665"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"6b87fcda7f9aaa8df13c6a1939087c8d719e1866","unresolved":false,"context_lines":[{"line_number":432,"context_line":"                if netaddr.IPNetwork(cidr).version \u003d\u003d 6:"},{"line_number":433,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv6\u0027"},{"line_number":434,"context_line":"                    if (str(cleaned_data[\u0027ip_protocol\u0027]).lower() in"},{"line_number":435,"context_line":"                            {\u0027icmp\u0027, \u00271\u0027}):"},{"line_number":436,"context_line":"                        cleaned_data[\u0027ip_protocol\u0027] \u003d \u0027ipv6-icmp\u0027"},{"line_number":437,"context_line":"                else:"},{"line_number":438,"context_line":"                    cleaned_data[\u0027ethertype\u0027] \u003d \u0027IPv4\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_1df8f546","line":435,"range":{"start_line":435,"start_character":37,"end_line":435,"end_character":40},"in_reply_to":"7faddb67_f5af3be0","updated":"2019-07-16 13:09:59.000000000","message":"Thanks for sharing your thoughts. I agree Nate\u0027s suggestion.\n\nI forgot the main point of handling ICMPv6 properly and seems to concern the auto-conversion from integer to string too much.","commit_id":"36dcd5ab2bb1740272f96c6a1a8cd5eb9a070665"},{"author":{"_account_id":27822,"name":"pengyuesheng","email":"pengyuesheng@gohighsec.com","username":"pengyuesheng"},"change_message_id":"2c58751c665d240ab18a9ca2ad318bc640ae2cae","unresolved":false,"context_lines":[{"line_number":421,"context_line":"                msg \u003d _(\u00271 (icmp) should be specified for IPv4 \u0027"},{"line_number":422,"context_line":"                        \u0027instead of 58.\u0027)"},{"line_number":423,"context_line":"                self._errors[\u0027ip_protocol\u0027] \u003d self.error_class([msg])"},{"line_number":424,"context_line":"            if msg:"},{"line_number":425,"context_line":"                messages.info(self.request, msg)"},{"line_number":426,"context_line":"        else:"},{"line_number":427,"context_line":"            # ICMPv6 uses different an IP protocol name and number."},{"line_number":428,"context_line":"            # To allow \u0027icmp\u0027 for both IPv4 and IPv6 in the form,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_400cf23a","line":425,"range":{"start_line":424,"start_character":12,"end_line":425,"end_character":48},"updated":"2019-07-22 08:54:34.000000000","message":"I think this is superfluous,the error message has been processed above.","commit_id":"e1b2531ebc1dea0831db4cd7099ad378efc356e3"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"72e26243057d9b6b1ee544db996b034254ccdcb1","unresolved":false,"context_lines":[{"line_number":421,"context_line":"                msg \u003d _(\u00271 (icmp) should be specified for IPv4 \u0027"},{"line_number":422,"context_line":"                        \u0027instead of 58.\u0027)"},{"line_number":423,"context_line":"                self._errors[\u0027ip_protocol\u0027] \u003d self.error_class([msg])"},{"line_number":424,"context_line":"            if msg:"},{"line_number":425,"context_line":"                messages.info(self.request, msg)"},{"line_number":426,"context_line":"        else:"},{"line_number":427,"context_line":"            # ICMPv6 uses different an IP protocol name and number."},{"line_number":428,"context_line":"            # To allow \u0027icmp\u0027 for both IPv4 and IPv6 in the form,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_7b53b9e3","line":425,"range":{"start_line":424,"start_character":12,"end_line":425,"end_character":48},"in_reply_to":"7faddb67_400cf23a","updated":"2019-07-24 11:32:48.000000000","message":"Done.\n\nI noticed this after pushing patch set 10, but I didn\u0027t update the patch soon.  I would like to hear opinions on the preferred approach from reviewers.\n\nIt seems we have a consensus on the direction, so I updated the patch.","commit_id":"e1b2531ebc1dea0831db4cd7099ad378efc356e3"}]}
